Re: is_superuser is not documented

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