> On Jun 10, 2026, at 23:12, Fujii Masao <[email protected]> wrote: > > On Wed, Jun 10, 2026 at 8:58 PM Japin Li <[email protected]> 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/
v3-0001-Fix-md5_password_warnings-for-role-and-database-s.patch
Description: Binary data
