Hi,
I found the comment on query_id_enabled looks inaccurate because this is
never set to true when compute_query_id is ON.
/* True when compute_query_id is ON, or AUTO and a module requests them */
bool query_id_enabled = false;
Should we fix this as following (just fixing the place of a comma) ?
/* True when compute_query_id is ON or AUTO, and a module requests them */
Also, I think the name is a bit confusing for the same reason, that is,
query_id_enabled may be false even when query id is computed in fact.
Actually, this does not matter because we use IsQueryIdEnabled to check
if query id is enabled, instead of referring to a global variable
(query_id_enabled or compute_query_id). But, just for making a code a bit
more readable, how about renaming this to query_id_required which seems to
stand for the meaning more correctly?
I attached a patch for above fixes.
Although renaming might not be acceptable since changing global variables
may affect third party tools, I think the comment should be fixed at least.
IMHO, it seems better to make this variable static not to be accessed directly.
However, I left it as is because this is used in a static inline function.
Regards,
Yugo Nagata
--
Yugo NAGATA <nag...@sraoss.co.jp>
diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index e489bfceb5..e4fbcf0d9f 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -42,8 +42,8 @@
/* GUC parameters */
int compute_query_id = COMPUTE_QUERY_ID_AUTO;
-/* True when compute_query_id is ON, or AUTO and a module requests them */
-bool query_id_enabled = false;
+/* True when compute_query_id is ON or AUTO, and a module requests them */
+bool query_id_required = false;
static void AppendJumble(JumbleState *jstate,
const unsigned char *item, Size size);
@@ -145,7 +145,7 @@ void
EnableQueryId(void)
{
if (compute_query_id != COMPUTE_QUERY_ID_OFF)
- query_id_enabled = true;
+ query_id_required = true;
}
/*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a066800a1c..4bcaf6d71c 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -532,7 +532,7 @@ typedef struct
pg_time_t first_syslogger_file_time;
bool redirection_done;
bool IsBinaryUpgrade;
- bool query_id_enabled;
+ bool query_id_required;
int max_safe_fds;
int MaxBackends;
#ifdef WIN32
@@ -6103,7 +6103,7 @@ save_backend_variables(BackendParameters *param, Port *port, BackgroundWorker *w
param->redirection_done = redirection_done;
param->IsBinaryUpgrade = IsBinaryUpgrade;
- param->query_id_enabled = query_id_enabled;
+ param->query_id_required = query_id_required;
param->max_safe_fds = max_safe_fds;
param->MaxBackends = MaxBackends;
@@ -6348,7 +6348,7 @@ restore_backend_variables(BackendParameters *param, Port **port, BackgroundWorke
redirection_done = param->redirection_done;
IsBinaryUpgrade = param->IsBinaryUpgrade;
- query_id_enabled = param->query_id_enabled;
+ query_id_required = param->query_id_required;
max_safe_fds = param->max_safe_fds;
MaxBackends = param->MaxBackends;
diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h
index f1c55c8067..3b2e1f8018 100644
--- a/src/include/nodes/queryjumble.h
+++ b/src/include/nodes/queryjumble.h
@@ -67,7 +67,7 @@ extern const char *CleanQuerytext(const char *query, int *location, int *len);
extern JumbleState *JumbleQuery(Query *query);
extern void EnableQueryId(void);
-extern PGDLLIMPORT bool query_id_enabled;
+extern PGDLLIMPORT bool query_id_required;
/*
* Returns whether query identifier computation has been enabled, either
@@ -80,7 +80,7 @@ IsQueryIdEnabled(void)
return false;
if (compute_query_id == COMPUTE_QUERY_ID_ON)
return true;
- return query_id_enabled;
+ return query_id_required;
}
#endif /* QUERYJUMBLE_H */
