Alvaro Herrera <alvhe...@alvh.no-ip.org> writes: > On 2021-Jul-27, Tom Lane wrote: >> Ugh. I think this patch is likely to create more problems than it fixes.
> I doubt that; as I said, the code already behaves in exactly that way > for closely related operations, so this patch isn't doing anything new. > Note that that loop this code is modifying only applies to lines that > are removed from the config file. Ah ... what's wrong here is some combination of -ENOCAFFEINE and a not-great explanation on your part. I misread the patch as adding "error = true" rather than the flag change. I agree that setting the GUC_PENDING_RESTART flag is fine, because set_config_option would do so if we reached it. Perhaps you should comment this along that line? Also, the cases inside set_config_option uniformly set that flag *before* the ereport not after. So maybe like if (gconf->context < PGC_SIGHUP) { + /* The removal can't be effective without a restart */ + gconf->status |= GUC_PENDING_RESTART; ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), One thing worth checking is whether the pending-restart flag gets cleared again if the DBA undoes the removal and again reloads. I think the right thing will happen, but it'd be worthwhile to check. regards, tom lane