On Sat, 10 Feb 2024 10:19:15 +0900 Michael Paquier <mich...@paquier.xyz> wrote:
> On Fri, Feb 09, 2024 at 04:37:23PM +0800, Julien Rouhaud wrote: > > On Fri, Feb 09, 2024 at 03:38:23PM +0900, Yugo NAGATA wrote: > >> 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? > > > > -1 for renaming to avoid breaking extensions that might access it. We > > should > > simply document for compute_query_id and query_id_enabled declaration that > > one > > should instead use IsQueryIdEnabled() if they're interested in whether the > > core > > queryid are computed or not. > > Agreed. A renaming would involve more pain than gain. Improving the > comments around how to all that would be good enough, my 2c. Thank you both for your comments. I agreed with not renaming it. I attached a updated patch that adds comments noting to use IsQueryIdEnabled() instead of accessing the variables directly. 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..82f725baaa 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -42,7 +42,13 @@ /* GUC parameters */ int compute_query_id = COMPUTE_QUERY_ID_AUTO; -/* True when compute_query_id is ON, or AUTO and a module requests them */ +/* + * True when compute_query_id is ON or AUTO, and a module requests them. + * + * Note that IsQueryIdEnabled() should be used instead of checking + * query_id_enabled or compute_query_id directly when we want to know + * whether query identifiers are computed in the core or not. + */ bool query_id_enabled = false; static void AppendJumble(JumbleState *jstate,
