Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.

From: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Subject: Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.
Date: 2025-02-12 09:07:08
Message-ID: CAHv8RjKMhefSNuxzseGYL_yorPytQNULmdreMydSBFCEbeQkdg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 12, 2025 at 1:15 PM Shubham Khanna
<khannashubham1197(at)gmail(dot)com> wrote:
>
> On Wed, Feb 12, 2025 at 5:29 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Tue, Feb 11, 2025 at 9:16 PM Shubham Khanna
> > <khannashubham1197(at)gmail(dot)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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nitin Jadhav 2025-02-12 09:59:04 Re: Address the bug in 041_checkpoint_at_promote.pl
Previous Message Andrei Lepikhov 2025-02-12 08:54:33 Re: explain analyze rows=%.0f