On Mon, Jan 30, 2023 at 05:12:27PM +0530, Nitin Jadhav wrote:
> Thanks Michael for identifying a new mistake. I am a little confused
> here. I don't understand why GUC_NO_SHOW_ALL depends on other GUCs
> like GUC_NOT_IN_SAMPLE or GUC_NO_RESET_ALL. Looks like the dependency
> between GUC_NO_RESET_ALL and GUC_NO_SHOW_ALL is removed in the above
> patch since we have these combinations now.
pg_settings would be unable to show something marked as NO_SHOW_ALL,
so the SQL check that looked after (NO_SHOW_ALL && !NO_RESET_ALL) is
a no-op. Postgres will likely gain more parameters that are kept
around for compability reasons, and forcing a NO_RESET_ALL in such
cases could impact applications using RESET on such GUCs, meaning
potential compatibility breakages.
> Similarly why can't we
> have a GUC marked as GUC_NO_SHOW_ALL but not GUC_NOT_IN_CONFIG. For me
> it makes sense if a GUC is marked as NO_SHOW_ALL and it can be present
> in a sample file. It's up to the author/developer to choose which all
> flags are applicable to the newly introduced GUCs. Please share your
> thoughts.
As also mentioned upthread by Tom, I am not sure that this combination
makes much sense, actually, because I don't see why one would never
want to know what is the effective value loaded for a parameter stored
in a file when he/she has the permission to do so. This could be
changed as of ALTER SYSTEM, postgresql.conf or even an included file,
and the value can only be read if permission to see it is given to the
role querying SHOW or pg_settings. This combination of flags is not a
practice to encourage.
--
Michael