> On Jun 10, 2026, at 23:12, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> 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!
Thanks for your review.
> 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/]);
>
Agreed.
>
> * 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.
>
Okay, changed.
>
> +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.
>
Ah, completely forgot that. Added.
PFA v3:
* Addressed all Fujii’s comments
* Changed a palloc to palloc_object
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
