On 2022/09/13 17:25, bt22kawamotok wrote:

Thanks for the patch!


+     <varlistentry id="guc-is-superuser" xreflabel="is_superuser">
+      <term><varname>is_superuser</varname> (<type>boolean</type>)

You need to add this entry just after that of "in_hot_standby" because
the descriptions of preset parameters should be placed in alphabetical
order in the docs.


+       <para>
+        Reports whether the user is superuser or not.

Isn't it better to add "current" before "user", e.g.,
"Reports whether the current user is a superuser"?


         /* Not for general use --- used by SET SESSION AUTHORIZATION */
-        {"is_superuser", PGC_INTERNAL, UNGROUPED,
+        {"is_superuser", PGC_INTERNAL, PRESET_OPTIONS,

This comment should be rewritten or removed because "Not for general
use" part is not true.


-            GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL |
GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+            GUC_REPORT | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | 
GUC_DISALLOW_IN_FILE

As Tom commented upthread, GUC_NO_RESET_ALL flag should be removed
because it's not necessary when PGC_INTERNAL context (i.e., in this context,
RESET ALL is prohibit by defaulted) is used.


With the patch, make check failed. You need to update
src/test/regress/expected/guc.out.


       <varlistentry>
        <term><literal>IS_SUPERUSER</literal></term>
        <listitem>
         <para>
          True if the current role has superuser privileges.
         </para>

I found that the docs of SET command has the above description about
is_superuser.
This description seems useless if we document the is_superuser GUC
itself. So isn't
it better to remove this description?

Thank you for your review.
I create new patch add_document_is_superuser_v2.
please check it.

Thanks for updating the patch!

The patch looks good to me.

-               /* Not for general use --- used by SET SESSION AUTHORIZATION */
                {"session_authorization", PGC_USERSET, UNGROUPED,

If we don't document session_authorization and do expect that
it's usually used by SET/SHOW SESSION AUTHORIZATION,
this comment doesn't need to be removed, I think.

Could you register this patch into the next Commit Fest
so that we don't forget it?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


Reply via email to