Hello!

A first question: have you looked at the GoAway patch[1]? While that
isn't exactly about the same situation, it was already considered for
password expiration checks in[2], and the same idea could be useful
for this situation too, especially in the context of my last question
in this email.

+ MyClientConnectionInfo.warning_message =
+ psprintf(_("your password will expire in %d day(s)"),
+ (int) (result / SECS_PER_DAY));

Shouldn't that use MemoryContextStrtup(TopMemoryContext, ...)?

+ /*
+ * Message to send to the client in case of connection success.
+ * When not NULL a WARNING message is sent to the client at end
+ * of the connection in src/backend/utils/init/postinit.c at
+ * enf of InitPostgres(). For example, it is use to show the
+ * password expiration warning.
+ */
+ const char *warning_message;

Handling of this new variable is missing from
EstimateClientConnectionInfoSpace and SerializeClientConnectionInfo,
which the struct explicitly asks for a few lines above this change.
Even if you think that's not necessary for some reason, it should be
explained to avoid confusing readers.

+ * Password OK, but check if rolvaliduntil is less than GUC
+ * password_expire_warning days to send a warning to the client
+ */
+ if (!isnull && password_expire_warning > 0 && vuntil < PG_INT64_MAX)

Could this use TIMESTAMP_NOT_FINITE?

And I think that "days"  should be "seconds".

+ TimestampTz result = (vuntil - now) / USECS_PER_SEC; /* in seconds */

Maybe call this variable something more descriptive, like
seconds_until_expiration?

+
+ if (result <= (TimestampTz) password_expire_warning)
+ {
+ MyClientConnectionInfo.warning_message =
+ psprintf(_("your password will expire in %d day(s)"),
+ (int) (result / SECS_PER_DAY));
+ }

This is not that useful on the last day - have you considered
displaying hours if the expiration date is within a day, or maybe
HH:MM?

There are a few typos (warnin in the commit message, disable ->
disables in the documentation, enf -> end in a comment)

I would also consider long lived connections. The warning currently
only fires when a user connects, maybe it would be useful to also do
something when the user enters the expiration period during an active
connection?

[1]: https://www.postgresql.org/message-id/DDPQ1RV5FE9U.I2WW34NGRD8Z%40jeltef.nl
[2]: 
https://www.postgresql.org/message-id/CAER375OvH3_ONmc-SgUFpA6gv_d6eNj2KdZktzo-f_uqNwwWNw%40mail.gmail.com


Reply via email to