On Mon, Apr 3, 2023 at 10:47 AM Fujii Masao <masao.fu...@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
