On Wed, Mar 27, 2024 at 11:01 AM Bruce Momjian <bruce@momjian.us> wrote:
> Uh, the above is clearly wrong. I think you mean "off" on the second line.
Woops. When the name changed from externally_managed_configuration to
allow_alter_system, the sense of it was reversed, and I guess Jelte
missed flipping the documentation references around. I likely would
have made the same mistake, but it's easily fixed.
> > +
> > + <para>
> > + Note that this setting cannot be regarded as a security feature. It
> > + only disables the <literal>ALTER SYSTEM</literal> command. It does not
> > + prevent a superuser from changing the configuration remotely using
>
> Why "remotely"?
This wording was suggested upthread. I think the point here is that if
the superuser is logging in from the local machine, it's obvious that
they can do whatever they want. The point is to emphasize that a
superuser without a local login can, too.
> "its"?
Yeah, that seems like an extra word.
> > + some outside mechanism. In such environments, using <command>ALTER
> > + SYSTEM</command> to make configuration changes might appear to work,
> > + but then may be discarded at some point in the future when that outside
>
> "might"
This does not seem like a mistake to me. I'm not sure why you think it is.
> > + mechanism updates the configuration. Setting this parameter to
> > + <literal>on</literal> can help to avoid such mistakes.
> > + </para>
>
> "off"
This is another case that needs to be fixed now that the sense of the
GUC is reversed. (We'd better make sure the code has the test the
right way around, too.)
> Is this really a patch we think we can push into PG 17. I am having my
> doubts.
If the worst thing that happens in PG 17 is that we push a patch that
needs a few documentation corrections, we're going to be doing
fabulously well.
--
Robert Haas
EDB: http://www.enterprisedb.com