On Mon, Apr 3, 2023 at 10:47 AM Fujii Masao <
masao.fujii@oss.nttdata.com> wrote:
> Yes, the patch has not been committed yet because of lack of review comments.
> Do you have any review comments on this patch?
> Or you think it's ready for committer?
I'm not very familiar with this code, so I'm not sure how much my
review is worth, but maybe it will spark some discussion.
> Yes, this patch moves the descriptions of is_superuser to config.sgml
> and changes its group to PRESET_OPTIONS.
is_superuser feels a little out of place in this file. All of
the options here apply to the entire PostgreSQL server, while
is_superuser only applies to the current session. The description of
this file says :
> These options report various aspects of PostgreSQL behavior that
> might be of interest to certain applications, particularly
> administrative front-ends. Most of them are determined when
> PostgreSQL is compiled or when it is installed.
Which doesn't seem to apply to is_superuser. It doesn't affect
the behavior of PostgreSQL, only what the current session is allowed to
do. It's also not determined when PostgreSQL is compiled/installed. Is
there some update that we can make to the description that would make
is_superuser fit in better?
I'm not familiar with the origins of is_superuser and it may be too
late for this, but it seems like is_superuser would fit in much better
as a system information function [0] rather than a GUC. Particularly
in the Session Information Functions.
> - GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
> + GUC_REPORT | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
This looks good to me. The lack of is_superuser from SHOW ALL has been
a source of confusion to me in the past.
As a side note server_version, server_encoding, lc_collate, and
lc_ctype all appear in both the preset options section of config.sgml
and in show.sgml. I'm not sure what the logic is for just including
these three parameters in show.sgml, but I think we should either
include all of the preset options or none of them.
Thanks,
Joe Koshakow
[0]
https://www.postgresql.org/docs/current/functions-info.html