Hi Fujii-san, Amit,
Thanks for reviewing the patch.
On Fri, 24 Oct 2025 at 10:31, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Oct 23, 2025 at 9:08 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Thu, Oct 23, 2025 at 8:28 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > +-- Parallel worker does not throw error during startup.
> > > +SET min_parallel_table_scan_size TO 0;
> > > +SET max_parallel_workers_per_gather TO 2;
> > > +SET parallel_setup_cost TO 0;
> > > +SET parallel_tuple_cost TO 0;
> > > +CREATE TABLE t1(a int);
> > > +INSERT INTO t1 VALUES(1), (2), (3), (4);
> > > +SELECT count(*) FROM t1;
> > >
> > > Isn't it better to reset these parameters after the test?
> >
> > I think the intention of this test case is to verify that the issue seen
> > in HEAD no longer occurs with the patch applied. So in HEAD this test
> > procedure needs to be able to reproduce the problem with
> > synchronized_standby_slots and parallel workers, but does it actually do?
> > I'm afraid additional steps are needed.
> >
> > Also, I wonder if it's really worth doing this test after the fix,
> > since it seems a special case.
> >
>
> Agreed. I also don't see the need.
>
I also agree. I have removed the test in the latest version of patch.
> >
> > +-- Cannot set synchronized_standby_slots to a invalid slot name.
> > +ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> >
> > Typo: "a invalid" should be "an invalid"
> >
>
> We can keep this test as the results with and without patch will be
> different for this case.
>
> HEAD:
> postgres=# ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*"
> DETAIL: Replication slot "invalid*" does not exist.
>
> Patch:
> postgres=# ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*"
> DETAIL: replication slot name "invalid*" contains invalid character
> HINT: Replication slot names may only contain lower case letters,
> numbers, and the underscore character.
>
I have corrected the typo in the comment and also kept this test.
I have attached the patches in [1].
[1]: https://www.postgresql.org/message-id/CANhcyEXBnQs31N2zAH7m9uSNWfOBianZEMEAzi-rj_kQ3B9ihQ%40mail.gmail.com
Thanks,
Shlok Kyal
