Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal

On Wed, Jul 10, 2024 at 11:54:38AM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I haven't tested it, but from skimming around the code, it looks like
>> ProcessConfigFileInternal() would deduplicate any previous entries in the
>> file prior to applying the values and running the check hooks.  Else,
>> reloading a configuration file with multiple startup-only GUC entries could
>> fail, even without bogus GUC check hooks.
> 
> While it's been a little while since I actually traced the logic,
> I believe the reason that case doesn't fail is this bit in
> set_config_with_handle, about line 3477 as of HEAD:
> 
>         case PGC_POSTMASTER:
>             if (context == PGC_SIGHUP)
>             {
>                 /*
>                  * We are re-reading a PGC_POSTMASTER variable from
>                  * postgresql.conf.  We can't change the setting, so we should
>                  * give a warning if the DBA tries to change it.  However,
>                  * because of variant formats, canonicalization by check
>                  * hooks, etc, we can't just compare the given string directly
>                  * to what's stored.  Set a flag to check below after we have
>                  * the final storable value.
>                  */
>                 prohibitValueChange = true;
>             }
>             else if (context != PGC_POSTMASTER)
>                 // throw "cannot be changed now" error

That's what I thought at first, but then I saw this in
ProcessConfigFileInternal():

			/* If it's already marked, then this is a duplicate entry */
			if (record->status & GUC_IS_IN_FILE)
			{
				/*
				 * Mark the earlier occurrence(s) as dead/ignorable.  We could
				 * avoid the O(N^2) behavior here with some additional state,
				 * but it seems unlikely to be worth the trouble.
				 */
				ConfigVariable *pitem;

				for (pitem = head; pitem != item; pitem = pitem->next)
				{
					if (!pitem->ignore &&
						strcmp(pitem->name, item->name) == 0)
						pitem->ignore = true;
				}
			}

-- 
nathan