rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix failure to verify PGC_[SU_]BACKEND GUCs in pg_file_settings view. 

set_config_option() bails out early if it detects that the option to
be set is PGC_BACKEND or PGC_SU_BACKEND class and we're reading the
config file in a postmaster child; we don't want to apply any new
value in such a case.  That's fine as far as it goes, but it fails
to consider the requirements of the pg_file_settings view: for that,
we need to check validity of the value even though we have no
intention to apply it.  Because we didn't, even very silly values
for affected GUCs would be reported as valid by the view.  There
are only half a dozen such GUCs, which perhaps explains why this
got overlooked for so long.

Fix by continuing when changeVal is false; this parallels the logic
in some other early-exit paths.

Also, the check added by commit 924bcf4f1 to prevent GUC changes in
parallel workers seems a few bricks shy of a load: it's evidently
assuming that ereport(elevel, ...) won't return.  Make sure we
bail out if it does.  The lack of trouble reports suggests that
this is only a latent bug, i.e. parallel workers don't actually
reach here with elevel < ERROR.  (Per the code coverage report,
we never reach here at all in the regression suite.)  But we clearly
don't want to risk proceeding if that does happen.

Per report from Rıdvan Korkmaz.  These are ancient bugs, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/2089235.1703617353@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2023-12-26 17:57:48 -05:00
parent a46972e30c
commit 059de3ca47
1 changed files with 8 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/backend/utils/misc/guc.c
View File

@ -3412,9 +3412,12 @@ set_config_with_handle(const char *name, config_handle *handle,
* Other changes might need to affect other workers, so forbid them.
*/
if (IsInParallelMode() && changeVal && action != GUC_ACTION_SAVE)
{
ereport(elevel,
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
errmsg("cannot set parameters during a parallel operation")));
return -1;
}
/* if handle is specified, no need to look up option */
if (!handle)
@ -3515,6 +3518,10 @@ set_config_with_handle(const char *name, config_handle *handle,
* backends. This is a tad klugy, but necessary because we
* don't re-read the config file during backend start.
*
* However, if changeVal is false then plow ahead anyway since
* we are trying to find out if the value is potentially good,
* not actually use it.
*
* In EXEC_BACKEND builds, this works differently: we load all
* non-default settings from the CONFIG_EXEC_PARAMS file
* during backend start. In that case we must accept
@ -3525,7 +3532,7 @@ set_config_with_handle(const char *name, config_handle *handle,
* started it. is_reload will be true when either situation
* applies.
*/
if (IsUnderPostmaster && !is_reload)
if (IsUnderPostmaster && changeVal && !is_reload)
return -1;
}
else if (context != PGC_POSTMASTER &&