Subject: Re: Stale references to guc.c in comments/tests

From: Tom Lane <[email-masked]>
Date: Fri, 24 Feb 2023 10:19:04 -0500

Daniel Gustafsson <[email-masked]> writes:
> I happened to notice that there were a few references to guc.c regarding
> variables, which with the recent refactoring in 0a20ff54f have become stale.
> Attached is a trivial patch to instead point to guc_tables.c.

Hmm, I think you may have done an overenthusiastic replacement here.
I agree with changes like this:

-extern char *role_string;		/* in guc.c */
+extern char *role_string;		/* in guc_tables.c */

because clearly that variable is now declared in guc_tables.c
and guc.c knows nothing of it explicitly.  However, a lot of
these places are really talking about the behavior of the GUC
mechanisms as a whole, and so a pointer to guc_tables.c doesn't
seem very on-point to me --- I find it hard to attribute behavior
to a static table.  Here for instance:

@@ -3041,7 +3041,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
                  *
                  * Variables that are not so marked should just be emitted as
                  * simple string literals.  If the variable is not known to
-                 * guc.c, we'll do that; this makes it unsafe to use
+                 * guc_tables.c, we'll do that; this makes it unsafe to use
                  * GUC_LIST_QUOTE for extension variables.
                  */
                 if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE)

An extension's GUC is by definition not known in guc_tables.c, so I think
this change is losing the point of the text.  What it's really describing
is a variable that hasn't been entered into the dynamic tables maintained
by guc.c.

Perhaps you could use "the GUC mechanisms" in these places, but it's a bit
longer than "guc.c".  Leaving such references alone seems OK too.

			regards, tom lane