Re: Fix md5_password_warnings for role/database settings

On Wed, Jun 10, 2026 at 8:58 PM Japin Li <japinli@hotmail.com> wrote:
> > PSA v2 - addressed Japin’s first comment.
> >
> Thanks for updating the patch, LGTM.

The patch basically looks good to me!
I just have three minor review comments.


+ my ($ret, $stdout, $stderr) = $node->psql(
+ 'postgres',
+ 'SELECT 1',
+ connstr => 'user=md5_role_no_warnings',
+ extra_params => ['-w'],
+ on_error_stop => 0);
+ is($ret, 0, 'md5 with warnings disabled');
+ unlike(
+ $stderr,
+ qr/authenticated with an MD5-encrypted password/,
+ 'md5 with warnings disabled: no MD5 authentication warning');

For this test, would it be better to use connect_ok() instead of
psql(), like this? That would let us verify more behavior at once:
the connection succeeds, stderr is empty (i.e., no MD5 warning is
emitted), SHOW md5_password_warnings returns off, and the server
log still shows method=md5.

$node->connect_ok(
"user=md5_role_no_warnings",
"md5 with warnings disabled",
sql => "SHOW md5_password_warnings",
expected_stdout => qr/^off$/,
log_like =>
[qr/connection authenticated: identity="md5_role_no_warnings" method=md5/]);


  * NB: Caller should ensure the strings are allocated in a long-lived context
- * like TopMemoryContext.
+ * like TopMemoryContext.  This function takes ownership of the strings, which
+ * will be freed in EmitConnectionWarnings().

Very minor comment: I'd suggest changing "allocated" to "palloc'd"
and "freed" to "pfree'd", to avoid future callers passing some non-pfreeable
memory unexpectedly.


+typedef struct ConnectionWarning

Since this patch introduces a new typedef, ConnectionWarning,
typedefs.list should probably be updated as well. Even if we forget
to do that, it will eventually get updated later, but I think it's
better to update it manually when possible so that pgindent works
correctly before then.

Regards,

-- 
Fujii Masao