Horton

Thread

Commits

  1. Remove extraneous newlines from guc_parameters.dat

  2. Fix NLS for incorrect GUC enum value hint message

  3. Clean up qsort comparison function for GUC entries

  4. Sort guc_parameters.dat alphabetically by name

  5. Reorganize GUC structs

  6. Change config_generic.vartype to be initialized at compile time

  7. Use designated initializers for guc_tables

  8. Change reset_extra into a config_generic common field

  9. Add some const qualifiers

  10. Modernize some for loops

  1. Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-10-03T06:55:10Z

    This patch series does some reorganizing of the GUC table structs 
(config_generic, config_bool, etc.) to modernize and simplify things.  I 
suspect that some changes like these would have been desirable as the 
GUC code has evolved over time, but were avoided because of the burden 
of having to adjust all the static GUC tables at the same time.  Since 
these are now generated, this is no longer a problem.

The main change is that instead of having five separate structs, one for 
each type, with the generic part contained in each of them, flip it 
around and have one common struct, with the type-specific part has a 
subfield.

The very original GUC design had type-specific structs and
type-specific lists, and the membership in one of the lists defined
the type.  But now the structs themselves know the type (from the
.vartype field), and they are all loaded into a common hash table at
run time, this original separation no longer makes sense.  It creates
a bunch of inconsistencies in the code about whether the type-specific
or the generic struct is the primary struct, and a lot of casting in
between, which makes certain assumptions about the struct layouts.

After the change, all these casts are gone and all the data is
accessed via normal field references.  Also, various code is
simplified because only one kind of struct needs to be processed.

Additionally, I have sorted guc_parameters.dat alphabetically by name, 
and have added code to enforce the sort order going forward.  (Note: The 
order is actually checked after lower-casing, to handle the likes of 
"DateStyle".)

The order in these lists was previously pretty random and had grown
organically over time.  This made it unnecessarily cumbersome to
maintain these lists, as there was no clear guidelines about where to
put new entries.  Also, after the merger of the type-specific GUC
structs, the list still reflected the previous type-specific
super-order.

By enforcing alphabetical order, the place for new entries becomes 
clear, and often related entries will be listed close together.

The patches in the series are arranged so that they can be considered 
and applied incrementally.

  2. Re: Reorganize GUC structs

    Chao Li <li.evan.chao@gmail.com> — 2025-10-09T07:15:24Z

    > On Oct 3, 2025, at 14:55, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> 
> The patches in the series are arranged so that they can be considered and applied incrementally.
> <v1-0001-Modernize-some-for-loops.patch><v1-0002-Add-some-const-qualifiers.patch><v1-0003-Change-reset_extra-into-a-config_generic-common-f.patch><v1-0004-Use-designated-initializers-for-guc_tables.patch><v1-0005-Change-config_generic.vartype-to-be-initialized-a.patch><v1-0006-Reorganize-GUC-structs.patch><v1-0007-Sort-guc_parameters.dat-alphabetically-by-name.patch><v1-0008-Enforce-alphabetical-order-in-guc_parameters.dat.patch>



0001 - looks good to me. Basically it only moves for loop’s loop variable type definition into for(), it isn’t tied to rest commits, I guess it can be pushed independently.

0002 - also looks good. It just add “const” where possible. I think it can be pushed together with 0001.

0003 - also looks good. It moves “reset_extra” from individual typed config structure to “config_generic”, which dramatically eliminates unnecessary switch-cases. Just a small comment:

```
@@ -6244,29 +6217,11 @@ RestoreGUCState(void *gucstate)
 		switch (gconf->vartype)
 		{
 			case PGC_BOOL:
-				{
-					struct config_bool *conf = (struct config_bool *) gconf;
-
-					if (conf->reset_extra && conf->reset_extra != gconf->extra)
-						guc_free(conf->reset_extra);
-					break;
-				}
 			case PGC_INT:
-				{
-					struct config_int *conf = (struct config_int *) gconf;
-
-					if (conf->reset_extra && conf->reset_extra != gconf->extra)
-						guc_free(conf->reset_extra);
-					break;
-				}
 			case PGC_REAL:
-				{
-					struct config_real *conf = (struct config_real *) gconf;
-
-					if (conf->reset_extra && conf->reset_extra != gconf->extra)
-						guc_free(conf->reset_extra);
-					break;
-				}
+			case PGC_ENUM:
+				/* no need to do anything */
+				break;
 			case PGC_STRING:
 				{
 					struct config_string *conf = (struct config_string *) gconf;
@@ -6274,19 +6229,11 @@ RestoreGUCState(void *gucstate)
 					guc_free(*conf->variable);
 					if (conf->reset_val && conf->reset_val != *conf->variable)
 						guc_free(conf->reset_val);
-					if (conf->reset_extra && conf->reset_extra != gconf->extra)
-						guc_free(conf->reset_extra);
-					break;
-				}
-			case PGC_ENUM:
-				{
-					struct config_enum *conf = (struct config_enum *) gconf;
-
-					if (conf->reset_extra && conf->reset_extra != gconf->extra)
-						guc_free(conf->reset_extra);
 					break;
 				}
 		}
```

Do we still need this switch-case? As only PGC_STRING needs to do something, why not just do:

```
If (gconf-vartype == PGC_STRING)
{
    …
}
if (gconf->reset_extra && gconf->reset_extra != gconf->extra)
    guc_free(gconf->reset_extra);
```

0004 - Also looks good to me. But I am not an expert of perl programming, so I cannot comment much.

0005 - Also looks good to me. This is a small change. It updates the perl script to set vartype so that vartype gets set at compile time, which further simplifies the c code.

0006 - Comes to the most interesting part of this patch. It moves all typed config into “config_generic” as a unnamed union, then replaces typed config with config_generic everywhere. Though this commit touches a lot of lines of code, but all changes are straightforward. Also looks good to me.

0007 needs a rebase. I guess you may split 0007 and 0008 to a separate patch once 0001-0006 are pushed. As they only re-order GUC variables without real logic change, so they can be quickly reviewed and pushed, otherwise it would be painful where every commit the touches GUC would lead to a rebase.

0008 - I don’t review it as 0007 needs a rebase.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  3. Re: Reorganize GUC structs

    John Naylor <johncnaylorls@gmail.com> — 2025-10-13T09:59:36Z

    On Fri, Oct 3, 2025 at 1:55 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> Additionally, I have sorted guc_parameters.dat alphabetically by name,
> and have added code to enforce the sort order going forward.  (Note: The

That makes sense by default. Another possibility would be to keep them
in the same order as postgresql.conf.sample. You mentioned earlier it
may be possible to generate the latter, although it's not yet clear if
that could be done easily. If not, then keeping them in sync would be
extra work.

--
John Naylor
Amazon Web Services

  4. Re: Reorganize GUC structs

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-13T11:39:09Z

    On 2025-Oct-13, John Naylor wrote:

> On Fri, Oct 3, 2025 at 1:55 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> > Additionally, I have sorted guc_parameters.dat alphabetically by name,
> > and have added code to enforce the sort order going forward.
> 
> That makes sense by default. Another possibility would be to keep them
> in the same order as postgresql.conf.sample. You mentioned earlier it
> may be possible to generate the latter, although it's not yet clear if
> that could be done easily. If not, then keeping them in sync would be
> extra work.

I agree that keeping the guc_parameters.dat file alphabetically sorted
by default would keep the maintenance cost lowest, because we won't have
to make any subjective decisions for new entries.  However, automatically
generating the .sample file sounds impractical, considering the
free-form comments that we currently have there.  For instance

#max_prepared_transactions = 0      # zero disables the feature
                    # (change requires restart)
# Caution: it is not advisable to set max_prepared_transactions nonzero unless
# you actively intend to use prepared transactions.

We'd have to store the comments together in the guc_parameters.dat table
so that the sample file knows where and how to put them, and it would
IMO look worse.  

Also, the order of entries in postgresql.conf.sample was at some point
carefully considered.  Messing that up (by making the entries match
alphabetical order, even within each section) would also be worse IMO.
So we'd have to add values for the sorting to know where in the file
each section goes, what its opening comment is, and where each entry
goes within each section.

I think instead of that mess, maybe we can simply keep the sample file
as-is, cross-check that a line for each non-hidden GUC variable exists,
and perhaps that the commented-out default value matches the data file.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Al principio era UNIX, y UNIX habló y dijo: "Hello world\n".
No dijo "Hello New Jersey\n", ni "Hello USA\n".

  5. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-10-15T12:46:55Z

    On 13.10.25 13:39, Álvaro Herrera wrote:
> I agree that keeping the guc_parameters.dat file alphabetically sorted
> by default would keep the maintenance cost lowest, because we won't have
> to make any subjective decisions for new entries.  However, automatically
> generating the .sample file sounds impractical, considering the
> free-form comments that we currently have there.

Yes, I'm not sure if it's practical in the fullest extent.  But there 
were at various points discussions about alternative layouts and 
contents for postgresql.conf.sample, with a lot of opinions.  With this 
new framework, I think it might be interesting to experiment.  Which is 
why I mentioned it.

> I think instead of that mess, maybe we can simply keep the sample file
> as-is, cross-check that a line for each non-hidden GUC variable exists,
> and perhaps that the commented-out default value matches the data file.

Those would certainly be reasonable near-term steps.

  6. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-10-15T14:05:29Z

    On 09.10.25 09:15, Chao Li wrote:
> 0001 - looks good to me. Basically it only moves for loop’s loop 
> variable type definition into for(), it isn’t tied to rest commits, I 
> guess it can be pushed independently.
> 
> 0002 - also looks good. It just add “const” where possible. I think it 
> can be pushed together with 0001.
> 
> 0003 - also looks good. It moves “reset_extra” from individual typed 
> config structure to “config_generic”, which dramatically eliminates 
> unnecessary switch-cases.

I have committed these first three.  Attached is the rest of the patch 
series rebased.

It looks like we have consensus in principle on the remaining changes, 
but I'll leave them out here a while longer in case there are any 
further thoughts.

Regarding ...

> Just a small comment:
> 
> ```
> @@ -6244,29 +6217,11 @@ RestoreGUCState(void *gucstate)
> switch (gconf->vartype)
> {
> case PGC_BOOL:
> -{
> -struct config_bool *conf = (struct config_bool *) gconf;
> -
> -if (conf->reset_extra && conf->reset_extra != gconf->extra)
> -guc_free(conf->reset_extra);
> -break;
> -}
> case PGC_INT:
> -{
> -struct config_int *conf = (struct config_int *) gconf;
> -
> -if (conf->reset_extra && conf->reset_extra != gconf->extra)
> -guc_free(conf->reset_extra);
> -break;
> -}
> case PGC_REAL:
> -{
> -struct config_real *conf = (struct config_real *) gconf;
> -
> -if (conf->reset_extra && conf->reset_extra != gconf->extra)
> -guc_free(conf->reset_extra);
> -break;
> -}
> +case PGC_ENUM:
> +/* no need to do anything */
> +break;
> case PGC_STRING:
> {
> struct config_string *conf = (struct config_string *) gconf;
> @@ -6274,19 +6229,11 @@ RestoreGUCState(void *gucstate)
> guc_free(*conf->variable);
> if (conf->reset_val && conf->reset_val != *conf->variable)
> guc_free(conf->reset_val);
> -if (conf->reset_extra && conf->reset_extra != gconf->extra)
> -guc_free(conf->reset_extra);
> -break;
> -}
> -case PGC_ENUM:
> -{
> -struct config_enum *conf = (struct config_enum *) gconf;
> -
> -if (conf->reset_extra && conf->reset_extra != gconf->extra)
> -guc_free(conf->reset_extra);
> break;
> }
> }
> ```
> 
> Do we still need this switch-case? As only PGC_STRING needs to do 
> something, why not just do:
> 
> ```
> If (gconf-vartype == PGC_STRING)
> {
>      …
> }
> if (gconf->reset_extra && gconf->reset_extra != gconf->extra)
>      guc_free(gconf->reset_extra);
> ```

I initially had it like that, but there are a couple of other places in 
the existing code that have a switch with only actual code for 
PGC_STRING, so I ended up following that pattern.

  7. Re: Reorganize GUC structs

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-10-24T12:21:17Z

    > @@ -261,15 +261,15 @@ static bool assignable_custom_variable_name(const char *name, bool skip_errors,
>  											int elevel);
>  static void do_serialize(char **destptr, Size *maxbytes,
>  						 const char *fmt,...) pg_attribute_printf(3, 4);
> -static bool call_bool_check_hook(const struct config_bool *conf, bool *newval,
> +static bool call_bool_check_hook(const struct config_generic *conf, bool *newval,
>  								 void **extra, GucSource source, int elevel);
> -static bool call_int_check_hook(const struct config_int *conf, int *newval,
> +static bool call_int_check_hook(const struct config_generic *conf, int *newval,
>  								void **extra, GucSource source, int elevel);
> -static bool call_real_check_hook(const struct config_real *conf, double *newval,
> +static bool call_real_check_hook(const struct config_generic *conf, double *newval,
>  								 void **extra, GucSource source, int elevel);
> -static bool call_string_check_hook(const struct config_string *conf, char **newval,
> +static bool call_string_check_hook(const struct config_generic *conf, char **newval,
>  								   void **extra, GucSource source, int elevel);
> -static bool call_enum_check_hook(const struct config_enum *conf, int *newval,
> +static bool call_enum_check_hook(const struct config_generic *conf, int *newval,
>  								 void **extra, GucSource source, int elevel);

The new signatures for these function are less specific than before, 
making them a little worse IMO. Overall +1 on the patches, despite that 
little drawback.

- Heikki

  8. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-10-29T09:07:02Z

    On 24.10.25 14:21, Heikki Linnakangas wrote:
>> @@ -261,15 +261,15 @@ static bool 
>> assignable_custom_variable_name(const char *name, bool skip_errors,
>>                                              int elevel);
>>  static void do_serialize(char **destptr, Size *maxbytes,
>>                           const char *fmt,...) pg_attribute_printf(3, 4);
>> -static bool call_bool_check_hook(const struct config_bool *conf, bool 
>> *newval,
>> +static bool call_bool_check_hook(const struct config_generic *conf, 
>> bool *newval,
>>                                   void **extra, GucSource source, int 
>> elevel);
>> -static bool call_int_check_hook(const struct config_int *conf, int 
>> *newval,
>> +static bool call_int_check_hook(const struct config_generic *conf, 
>> int *newval,
>>                                  void **extra, GucSource source, int 
>> elevel);
>> -static bool call_real_check_hook(const struct config_real *conf, 
>> double *newval,
>> +static bool call_real_check_hook(const struct config_generic *conf, 
>> double *newval,
>>                                   void **extra, GucSource source, int 
>> elevel);
>> -static bool call_string_check_hook(const struct config_string *conf, 
>> char **newval,
>> +static bool call_string_check_hook(const struct config_generic *conf, 
>> char **newval,
>>                                     void **extra, GucSource source, 
>> int elevel);
>> -static bool call_enum_check_hook(const struct config_enum *conf, int 
>> *newval,
>> +static bool call_enum_check_hook(const struct config_generic *conf, 
>> int *newval,
>>                                   void **extra, GucSource source, int 
>> elevel);
> 
> The new signatures for these function are less specific than before, 
> making them a little worse IMO. Overall +1 on the patches, despite that 
> little drawback.

Thanks, pushed.

  9. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-03T11:16:13Z

    On 29.10.25 10:07, Peter Eisentraut wrote:
> On 24.10.25 14:21, Heikki Linnakangas wrote:
>>> @@ -261,15 +261,15 @@ static bool 
>>> assignable_custom_variable_name(const char *name, bool skip_errors,
>>>                                              int elevel);
>>>  static void do_serialize(char **destptr, Size *maxbytes,
>>>                           const char *fmt,...) pg_attribute_printf(3, 
>>> 4);
>>> -static bool call_bool_check_hook(const struct config_bool *conf, 
>>> bool *newval,
>>> +static bool call_bool_check_hook(const struct config_generic *conf, 
>>> bool *newval,
>>>                                   void **extra, GucSource source, int 
>>> elevel);
>>> -static bool call_int_check_hook(const struct config_int *conf, int 
>>> *newval,
>>> +static bool call_int_check_hook(const struct config_generic *conf, 
>>> int *newval,
>>>                                  void **extra, GucSource source, int 
>>> elevel);
>>> -static bool call_real_check_hook(const struct config_real *conf, 
>>> double *newval,
>>> +static bool call_real_check_hook(const struct config_generic *conf, 
>>> double *newval,
>>>                                   void **extra, GucSource source, int 
>>> elevel);
>>> -static bool call_string_check_hook(const struct config_string *conf, 
>>> char **newval,
>>> +static bool call_string_check_hook(const struct config_generic 
>>> *conf, char **newval,
>>>                                     void **extra, GucSource source, 
>>> int elevel);
>>> -static bool call_enum_check_hook(const struct config_enum *conf, int 
>>> *newval,
>>> +static bool call_enum_check_hook(const struct config_generic *conf, 
>>> int *newval,
>>>                                   void **extra, GucSource source, int 
>>> elevel);
>>
>> The new signatures for these function are less specific than before, 
>> making them a little worse IMO. Overall +1 on the patches, despite 
>> that little drawback.
> 
> Thanks, pushed.

The remaining patches to sort the list alphabetically have also been pushed.

  10. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-07T08:44:22Z

    On 03.11.25 12:16, Peter Eisentraut wrote:
> The remaining patches to sort the list alphabetically have also been 
> pushed.

Here are a few more small patches to fix related things I found 
afterwards or in passing.

The first one straightens out how the qsort comparison function for GUC 
records works, and also removes the implicit requirement that the name 
field is first in the struct.

The second one fixes up an NLS build rule for the recent changes.

The third fixes some translation markers.  This is an older problem.

  11. Re: Reorganize GUC structs

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-07T09:38:45Z

    On 2025-Nov-07, Peter Eisentraut wrote:

> @@ -3135,14 +3135,14 @@ parse_and_validate_value(const struct config_generic *record,
>  					char	   *hintmsg;
>  
>  					hintmsg = config_enum_get_options(conf,
> -													  "Available values: ",
> -													  ".", ", ");
> +													  _("Available values: "),
> +													  _("."), _(", "));

Hmm, it seems unclear (from the message catalog perspective) what the
last two strings will be used for, so please add a /* translator */
comment to each.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La fuerza no está en los medios físicos
sino que reside en una voluntad indomable" (Gandhi)

  12. Re: Reorganize GUC structs

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-07T15:14:55Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> Here are a few more small patches to fix related things I found 
> afterwards or in passing.

Looks sane, except that the comparator could do with an extra "const"
if there's enough room on the line:

+    const struct config_generic *ca = *(const struct config_generic *const *) a;
+    const struct config_generic *cb = *(const struct config_generic *const *) b;

Not essential, but casting const away even transiently looks ugly.

			regards, tom lane

  13. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-07T15:44:40Z

    On 07.11.25 10:38, Álvaro Herrera wrote:
> On 2025-Nov-07, Peter Eisentraut wrote:
> 
>> @@ -3135,14 +3135,14 @@ parse_and_validate_value(const struct config_generic *record,
>>   					char	   *hintmsg;
>>   
>>   					hintmsg = config_enum_get_options(conf,
>> -													  "Available values: ",
>> -													  ".", ", ");
>> +													  _("Available values: "),
>> +													  _("."), _(", "));
> 
> Hmm, it seems unclear (from the message catalog perspective) what the
> last two strings will be used for, so please add a /* translator */
> comment to each.

Actually, the ", " already exists in the message catalog, so adding a 
comment here might confuse the other site.

  14. Re: Reorganize GUC structs

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-07T15:53:26Z

    On 2025-Nov-07, Peter Eisentraut wrote:

> Actually, the ", " already exists in the message catalog, so adding a
> comment here might confuse the other site.

This one?
#: replication/logical/relation.c:245
#, c-format
msgid ", \"%s\""
msgstr ", »%s«"

It's different.  (And warrants a comment too, IMO)

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)

  15. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-11T07:04:17Z

    On 07.11.25 16:14, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> Here are a few more small patches to fix related things I found
>> afterwards or in passing.
> 
> Looks sane, except that the comparator could do with an extra "const"
> if there's enough room on the line:
> 
> +    const struct config_generic *ca = *(const struct config_generic *const *) a;
> +    const struct config_generic *cb = *(const struct config_generic *const *) b;
> 
> Not essential, but casting const away even transiently looks ugly.

committed with that amendment

  16. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-12T08:41:07Z

    On 07.11.25 16:53, Álvaro Herrera wrote:
> On 2025-Nov-07, Peter Eisentraut wrote:
> 
>> Actually, the ", " already exists in the message catalog, so adding a
>> comment here might confuse the other site.
> 
> This one?
> #: replication/logical/relation.c:245
> #, c-format
> msgid ", \"%s\""
> msgstr ", »%s«"
> 
> It's different.  (And warrants a comment too, IMO)

Well, that's the one, but the code actually looks like this now:

     while ((i = bms_next_member(atts, i)) >= 0)
     {
         attcnt++;
         if (attcnt > 1)
             appendStringInfoString(&attsbuf, _(", "));

         appendStringInfo(&attsbuf, _("\"%s\""), remoterel->attnames[i]);
     }

The catalog entries you are showing appear to be from pre-PG18.

There are also similar instances in ExecBuildSlotValueDescription() that 
are not marked up for gettext but perhaps should be.  There might be 
others, and I would expect more like this to appear over time, as people 
like to add more detail like this to diagnostics messages.

Do you have a suggestion what kind of comment to attach there?

  17. Re: Reorganize GUC structs

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-14T11:00:46Z

    On 2025-Nov-12, Peter Eisentraut wrote:

> Well, that's the one, but the code actually looks like this now:
> 
>     while ((i = bms_next_member(atts, i)) >= 0)
>     {
>         attcnt++;
>         if (attcnt > 1)
>             appendStringInfoString(&attsbuf, _(", "));
> 
>         appendStringInfo(&attsbuf, _("\"%s\""), remoterel->attnames[i]);
>     }
> 
> The catalog entries you are showing appear to be from pre-PG18.

Ah, right.

> There are also similar instances in ExecBuildSlotValueDescription() that are
> not marked up for gettext but perhaps should be.  There might be others, and
> I would expect more like this to appear over time, as people like to add
> more detail like this to diagnostics messages.

Sure.

> Do you have a suggestion what kind of comment to attach there?

I would say "This is a separator in a list of entity names."  We don't
need to be specific as to what kind of entity it is, I think.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree.              (Don Knuth)

  18. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-19T07:57:28Z

    On 14.11.25 12:00, Álvaro Herrera wrote:
> On 2025-Nov-12, Peter Eisentraut wrote:
> 
>> Well, that's the one, but the code actually looks like this now:
>>
>>      while ((i = bms_next_member(atts, i)) >= 0)
>>      {
>>          attcnt++;
>>          if (attcnt > 1)
>>              appendStringInfoString(&attsbuf, _(", "));
>>
>>          appendStringInfo(&attsbuf, _("\"%s\""), remoterel->attnames[i]);
>>      }
>>
>> The catalog entries you are showing appear to be from pre-PG18.
> 
> Ah, right.
> 
>> There are also similar instances in ExecBuildSlotValueDescription() that are
>> not marked up for gettext but perhaps should be.  There might be others, and
>> I would expect more like this to appear over time, as people like to add
>> more detail like this to diagnostics messages.
> 
> Sure.
> 
>> Do you have a suggestion what kind of comment to attach there?
> 
> I would say "This is a separator in a list of entity names."  We don't
> need to be specific as to what kind of entity it is, I think.

Ok, committed that way.

  19. Re: Reorganize GUC structs

    Anton Voloshin <a.voloshin@postgrespro.ru> — 2026-06-22T08:15:50Z

    Hello,

On 19/11/2025 10:57, Peter Eisentraut wrote:
 > Ok, committed that way.

in fce7c73fb (Sort guc_parameters.dat alphabetically by name) two 
extraneous newlines have been kept in the guc_parameters.dat: before 
archive_command and before seq_page_cost. I suggest to remove them, 
since they don't seem to mark any important spots anyway, see the 
attached patch.

Found by accident after re-sorting guc_parameters.dat in our fork with 
sort_guc_parameters.pl (also attached just in case). This is a 
single-use script, not meant to be robust or tidy.

Best regards,
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

  20. Re: Reorganize GUC structs

    Peter Eisentraut <peter@eisentraut.org> — 2026-06-24T13:07:28Z

    On 22.06.26 10:15, Anton Voloshin wrote:
> Hello,
> 
> On 19/11/2025 10:57, Peter Eisentraut wrote:
>  > Ok, committed that way.
> 
> in fce7c73fb (Sort guc_parameters.dat alphabetically by name) two 
> extraneous newlines have been kept in the guc_parameters.dat: before 
> archive_command and before seq_page_cost. I suggest to remove them, 
> since they don't seem to mark any important spots anyway, see the 
> attached patch.

Fixed, thanks.