Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.
Shubham Khanna <khannashubham1197@gmail.com>
From: Shubham Khanna <khannashubham1197@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2025-02-12T09:07:08Z
Lists: pgsql-hackers
On Wed, Feb 12, 2025 at 1:15 PM Shubham Khanna <khannashubham1197@gmail.com> wrote: > > On Wed, Feb 12, 2025 at 5:29 AM Peter Smith <smithpb2250@gmail.com> wrote: > > > > On Tue, Feb 11, 2025 at 9:16 PM Shubham Khanna > > <khannashubham1197@gmail.com> wrote: > > > > > > > #13. Unanswered question "How are tests expecting this even passing?". > > > > Was a reason identified? IOW, how can we be sure the latest tests > > > > don't have a similar problem? > > > > > > > > > > In the v4-0001 patch [1], the tests were mistakenly using > > > 'command_fails' instead of 'command_fails_like' to verify failed test > > > cases. Since 'command_fails' only checks for a non-zero exit code and > > > not specific error messages, the tests were passing even when the > > > expected errors were not being triggered correctly. > > > To address this, I have updated the patch to use 'command_fails_like', > > > ensuring that the test cases now explicitly verify the correct failure > > > messages. > > > > > > > Ah, that makes sense. Thanks for sharing the reason. So in fact, it > > was a valid concern because the v5 was still carrying over the same > > flaw from v4... Anyway, it is good to know it is fixed now in v6. > > > > ===== > > > > Some general comments for the patch v6-0001: > > > > Do you need to test every possible bad option combination? It may be > > fine because the error will be immediately raised so I expect the > > execution overhead to be ~zero. > > > > BTW, your bad option combination tests are only using --all-databases > > *after* the other options. Maybe you should mix it up a bit, sometimes > > putting it *before* the others as well, because rearranging will cause > > different errors. > > > > Everything else now looks good to me. > Fixed the comment given by Shlok at [1]. The attached patch at [2] contains the suggested changes. [1] - https://www.postgresql.org/message-id/CANhcyEXGwFeQd2fuB2txm1maCC2zKyROUR5exEMGzPYdrZ4CPQ%40mail.gmail.com [2] - https://www.postgresql.org/message-id/CAHv8RjKc6LMJ86b4yCmwNTn0c65mz0BGMCF-vPJSKDMOGagVGA%40mail.gmail.com Thanks and regards, Shubham Khanna.
Commits
-
Cosmetic fixes for pg_createsubscriber's -all option.
- d438515c29c3 18.0 landed
-
pg_createsubscriber: Add '--all' option.
- fb2ea12f42b9 18.0 landed