| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE |
| Date: | 2026-07-27 19:20:13 |
| Message-ID: | CAD21AoC_DsvqE1Ky5D7Z_iBfkKyvSwZwVrvbA7+MvPzrvHoKYw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Jul 27, 2026 at 8:39 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Jul 23, 2026 at 11:22 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > The patches look mostly good to me. I have a few minor comments:
>
> Thanks for reviewing!
>
> > +session s1
> > +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that
> > +# the pg_publication_tables query will block when it tries to open the table.
> > +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; }
> > +# Drop the table in the lock-holding session and commit, releasing the lock.
> > +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; }
> >
> > ISTM it's clearer if we write these comments to the permutation since
> > we might add more tests in the future that might reuse these steps.
>
> Agreed, and moved the bug and test description above the permutation.
> I left the spec file name as-is. It gives flexibility to add more
> concurrent-drop related tests (for example, concurrent partition table
> drops), and I don't think renaming it to a generic name is the right
> choice without knowing exactly what future tests could be added here.
>
> > ---
> > +session s2
> > +step query
> > +{
> > + SELECT tablename FROM pg_publication_tables
> > + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop'
> > + ORDER BY tablename;
> > +}
> >
> > I guess it's better to call pg_get_publication_tables() directly
> > instead of via pg_publication_tables. =
>
> Yes, using the pg_get_publication_tables function tests its output
> directly, without the join to pg_class that the pg_publication_tables
> view adds, which would otherwise filter out the concurrently dropped
> table.
>
> Please find the attached v10 patches.
Thank you for updating the patches! They look good to me.
I've updated the commit message and am going to push them (down to
v16), barring any objections.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| REL16_v11-0001-Fix-pg_get_publication_tables-failure-with-concu.patch | text/x-patch | 9.2 KB |
| REL18_v11-0001-Fix-pg_get_publication_tables-failure-with-concu.patch | text/x-patch | 9.2 KB |
| REL17_v11-0001-Fix-pg_get_publication_tables-failure-with-concu.patch | text/x-patch | 9.2 KB |
| REL19_v11-0001-Fix-pg_get_publication_tables-failure-with-concu.patch | text/x-patch | 9.3 KB |
| master_v11-0001-Fix-pg_get_publication_tables-failure-with-concu.patch | text/x-patch | 9.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | solai v | 2026-07-27 19:25:11 | Re: [PATCH v1] Add vacuum_delay_point() to GiST empty-page deletion pass |
| Previous Message | Vik Fearing | 2026-07-27 19:00:56 | Re: FROM clause before SELECT |