Re: Pasword expiration warning

On Thu, 08 Jan 2026 at 09:41, Gilles Darold <gilles@darold.net> wrote:
> Le 08/01/2026 à 08:43, Japin Li a écrit :
>> On Thu, 08 Jan 2026 at 07:04, Gilles Darold <gilles@darold.net> wrote:
>>> Le 08/01/2026 à 04:37, Japin Li a écrit :
>>>> On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:
>>>>> Hi, Gilles Darold
>>>>>
>>>>> First of all, thank you for your reply. This is indeed not a simple
>>>>> countdown. I did think it would be abrupt for users to see "0 days". I
>>>>> checked v4, and I think it's fine. (PS: Should we add relevant
>>>>> explanations to the SGML?) Thank you.
>>>>>
>>>> I'd like to hear more opinions on this.
>>>>
>>> Here is a new version of the patch that adds the documentation for the
>>> new GUC, fix the warning message (days(s) instead of days) and handle
>>> the 'Infinity' value for the VALID UNTIL clause.
>>>
>>>
>> Thanks for updating the patch.
>>
>> 1.
>> I noticed a warning when applying the patch.
>>
>> Applying: Add password_expire_warning GUC to warn clients
>> .git/rebase-apply/patch:31: trailing whitespace.
>>          disable this behavior. The default value is <literal>7d</literal>.
>> warning: 1 line adds whitespace errors.
>>
>> 2.
>>        <varlistentry id="guc-password-encryption" xreflabel="password_encryption">
>> -      <term><varname>password_encryption</varname> (<type>enum</type>)
>> +      <term><varname>password_encryption</varname> (<type>enum</type>
>> +)
>>
>> I think this modification isn't necessary.
>>
>> 3.
>> +        float8          result;
>> +
>> +        result = ((float8) (vuntil - GetCurrentTimestamp())) / 1000000.0; /* in seconds */
>> +
>>
>> Perhaps we could use TimestampTz for the result variable and substitute
>> USECS_PER_SEC for 1000000.0—that would avoid the unnecessary type cast.
>>
>> 4.
>> +        if ((int) result <= password_expire_warning)
>>
>> If the result exceeds INT_MAX, it triggers undefined behavior (IIRC).
>> Therefore, we should probably cast password_expire_warning itself.
>>
>> 5.
>> With this feature, GetCurrentTimestamp() might end up being called twice.
>> Perhaps we can avoid that duplication.
>>
>>
>> Attached is v6 of the patch addressing the issues above. Please take a look.
>
>
> Thanks Japin, the implementation is fully working using the
> TimestampTz cast.
>
>
> I've attached a new patch to fix documentation and comments reported
> by liu xiaohui and create a commitfest entry :
> https://commitfest.postgresql.org/patch/6381/ Every one involved in
> the review should edit the commitfest entry.
>

A minor nitpick:

1.
+     <varlistentry id="guc-password-expire-warnings" xreflabel="password_expire_warning">
+      <term><varname>password_expire_warning</varname> (<type>integer</type>)

We should probably use guc-password-expire-warning as the ID, since the GUC is
named password_expire_warning (singular).


--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.