Re: [PATCH] Fix NULL dereference in subscription REFRESH on concurrent DROP

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Fix NULL dereference in subscription REFRESH on concurrent DROP
Date: 2026-08-20 01:32:00
Message-ID: CALj2ACXoJG5EVeHtG8FHLkyc0zJKC7T3Ttqz+BOs_io7SyUnvw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Tue, Aug 18, 2026 at 11:25 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Thank you for updating the patch.

Thanks for reviewing it.

> > 1/ Uses try_table_open() for all relations and gets the namespace
> > using RelationGetNamespace().
>
> I'm not sure that this patch should follow the changes commit
> 63e7a0d2c did for pg_get_publication_tables() since that function
> opened tables if the column list is not specified
>
> On the other hand, check_publications_origin_tables() doesn't do so.
>
> A plain NULL check might work better here like
> the v1 patch does.

Agreed. I complicated this a bit with the additional table_open().

> > 2/ Deduplicates the common code that quotes the subscription
> > relations' schema-qualified names for tables and sequences into a
> > helper function.
>
> While it works for v19 and master, it doesn't work for v16, v17, and
> v18. ISTM it doesn't help simplify the code that much. I guess we can
> live with such a duplication.

Agreed.

> > 3/ Adds a TAP test with an injection point in 0002 (which I don't
> > intend to be committed).
>
> Thank you for creating the test case. I agree that the test for this
> issue needs a new injection point and it would not be good for back
> branches. I still see value a bit in having the regression test only
> for HEAD. Feedback is welcome.

As this is a crash fix, having a test makes sense. +1 for HEAD only. I
tried without an injection point, but there's no table_open() or lock
in this area to hold control and concurrently drop in another session.
I moved the TAP test to the existing subscriber TAP test file to avoid
starting another full-blown server just for this. Test now covers both
concurrent table and sequence drop.

Please find the attached v3 patch. I verified the issue goes back to
PG16 where commit 8756930190 introduced the origin check. Patches for
all affected branches are attached.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v3-0001-Fix-crash-in-subscription-REFRESH-on-concurrent-r.patch application/octet-stream 6.8 KB
nocfbot-v3-0001-PG19-Fix-crash-in-subscription-REFRESH-on-concurrent-r.patch application/octet-stream 3.4 KB
nocfbot-v3-0001-PG18-PG17-PG16-Fix-crash-in-subscription-REFRESH-on-concurrent-r.patch application/octet-stream 2.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bingshuai Li 2026-08-20 01:40:44 Re: Logical Replication - revisit `is_table_publication` function implementation
Previous Message David Rowley 2026-08-20 01:07:01 Re: Test tidscan,sql is not immune to autovacuum in v14