On Tue, Apr 28, 2026 at 7:11 AM Bharath Rupireddy
<[email protected]> wrote:
>
> Hi,
>
> On Mon, Apr 27, 2026 at 3:01 AM Bertrand Drouvot
> <[email protected]> wrote:
> >
> > I've 2 comments:
>
> Thank you for reviewing!
>
> > 1/ What about having just one curr_idx increment? (right after list_nth(),
> > before the skip checks). I think that would be less error-prone if new skip
> > conditions are added later.
>
> Done.
>
> > 2/ I think that the test is racy and could also succeed even without the
> > fixes.
> > Indeed, I think that the drops can complete before any concurrent polling
> > happens (I can see it by adding a pg_sleep(2) before the first poll in the
> > DO
> > block). What about using an injection point to ensure a relation is removed
> > during the polling?
>
> I initially considered an injection point but chose polling since the
> TAP test reproduced the bug consistently with hundreds of tables on my
> dev system. I've now added an injection point for predictability.
>
> I adjusted the commit message a bit. Please find the attached v3 patch
> for further review. Thank you!
>
Thanks Bharath. I have just one minor comment:
+ INJECTION_POINT("pg-get-publication-tables-build-list", NULL);
Shall we name it as 'pg-get-publication-tables-list-built' to be more
meaningful, as we are pausing after list is built.
thanks
Shveta
