Re: is_superuser versus set_config_option's parallelism check

On Fri, Aug 09, 2024 at 06:50:14PM -0400, Tom Lane wrote:
> Here's a draft patch to fix it with a flag, now with regression tests.

Looks reasonable.

> Also, now that the error depends on which parameter we're talking
> about, I thought it best to include the parameter name in the error
> and to re-word it to be more like all the other can't-set-this-now
> errors just below it.  I'm half tempted to change the errcode and
> set_config_option return value to match the others too, ie
> ERRCODE_CANT_CHANGE_RUNTIME_PARAM and "return 0" not -1.
> I don't think the existing choices here are very well thought
> through, and they're certainly inconsistent with a lot of
> otherwise-similar-seeming refusals in set_config_option.

This comment for set_config_option() leads me to think we should be
returning -1 instead of 0 in many more places in set_config_with_handle():

 * Return value:
 *  +1: the value is valid and was successfully applied.
 *  0:  the name or value is invalid (but see below).
 *  -1: the value was not applied because of context, priority, or changeVal.

But I haven't thought through it, either.  At this point, maybe the comment
is wrong...

-- 
nathan