Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e87252c

Browse files
committed
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 924bcf4 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
1 parent c72049d commit e87252c

File tree

1 file changed

+8
-1
lines changed
  • src/backend/utils/misc

1 file changed

+8
-1
lines changed

src/backend/utils/misc/guc.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3363,9 +3363,12 @@ set_config_option_ext(const char *name, const char *value,
33633363
* Other changes might need to affect other workers, so forbid them.
33643364
*/
33653365
if (IsInParallelMode() && changeVal && action != GUC_ACTION_SAVE)
3366+
{
33663367
ereport(elevel,
33673368
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
33683369
errmsg("cannot set parameters during a parallel operation")));
3370+
return -1;
3371+
}
33693372

33703373
record = find_option(name, true, false, elevel);
33713374
if (record == NULL)
@@ -3460,6 +3463,10 @@ set_config_option_ext(const char *name, const char *value,
34603463
* backends. This is a tad klugy, but necessary because we
34613464
* don't re-read the config file during backend start.
34623465
*
3466+
* However, if changeVal is false then plow ahead anyway since
3467+
* we are trying to find out if the value is potentially good,
3468+
* not actually use it.
3469+
*
34633470
* In EXEC_BACKEND builds, this works differently: we load all
34643471
* non-default settings from the CONFIG_EXEC_PARAMS file
34653472
* during backend start. In that case we must accept
@@ -3470,7 +3477,7 @@ set_config_option_ext(const char *name, const char *value,
34703477
* started it. is_reload will be true when either situation
34713478
* applies.
34723479
*/
3473-
if (IsUnderPostmaster && !is_reload)
3480+
if (IsUnderPostmaster && changeVal && !is_reload)
34743481
return -1;
34753482
}
34763483
else if (context != PGC_POSTMASTER &&

0 commit comments

Comments
 (0)