Thread

Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-22T19:31:18Z

Hi,

I came across a race condition in pg_get_publication_tables with
concurrent DROP TABLE. pg_get_publication_tables collects table OIDs
without locks on the first call, then opens each table on later calls.
If a table is dropped in between, the function errors with "could not
open relation with OID".

This is common in environments where many tables are being created and
dropped while pg_publication_tables is queried, such as with FOR ALL
TABLES publications.

Please find the attached patch that fixes this by skipping
concurrently dropped tables instead of erroring out. Tables created
after the list is built are simply not present in the result set,
which is expected point-in-time behavior with no error. I've also
added a TAP test for this issue.

Thoughts?

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-23T11:15:11Z

On Thu, Apr 23, 2026 at 1:01 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> I came across a race condition in pg_get_publication_tables with
> concurrent DROP TABLE. pg_get_publication_tables collects table OIDs
> without locks on the first call, then opens each table on later calls.
> If a table is dropped in between, the function errors with "could not
> open relation with OID".
>

I agree with the problem statement, this is a weird error:

postgres=# select * from pg_publication_tables;
ERROR:  could not open relation with OID 16390

> This is common in environments where many tables are being created and
> dropped while pg_publication_tables is queried, such as with FOR ALL
> TABLES publications.
> Please find the attached patch that fixes this by skipping
> concurrently dropped tables instead of erroring out. Tables created
> after the list is built are simply not present in the result set,
> which is expected point-in-time behavior with no error.

I too think that this should be fixed by skipping the dropped table.
Will reveiw patch soon.

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-24T05:49:40Z

On Thu, Apr 23, 2026 at 4:45 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 1:01 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > Hi,
> >
> > I came across a race condition in pg_get_publication_tables with
> > concurrent DROP TABLE. pg_get_publication_tables collects table OIDs
> > without locks on the first call, then opens each table on later calls.
> > If a table is dropped in between, the function errors with "could not
> > open relation with OID".
> >
>
> I agree with the problem statement, this is a weird error:
>
> postgres=# select * from pg_publication_tables;
> ERROR:  could not open relation with OID 16390
>
> > This is common in environments where many tables are being created and
> > dropped while pg_publication_tables is queried, such as with FOR ALL
> > TABLES publications.
> > Please find the attached patch that fixes this by skipping
> > concurrently dropped tables instead of erroring out. Tables created
> > after the list is built are simply not present in the result set,
> > which is expected point-in-time behavior with no error.
>
> I too think that this should be fixed by skipping the dropped table.
> Will reveiw patch soon.
>

Bharath, I reviewed the patch. I personally think that manually
incrementing the call counter of SRF (funcctx->call_cntr++) in
pg_get_publication_tables() is not a good idea. I think these are
read-only for us and any changes to SRF fields must use SRF macros.

I tried to find if any other code-part does that, found one refernce
in hstore_svals():

/* ugly ugly ugly. why no macro for this? */
(funcctx)->call_cntr++;

Having said that, I could not find any other way to implement the fix
also. Did you try exploring 'SRF_RETURN_NEXT_NULL' in this case? I am
not very sure about this as well, as it will end up retruning NULL and
may impact output and its usage in tablesync too.

Let's see what others have to say on this fix.

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-24T06:21:25Z

Hi,

On Fri, Apr 24, 2026 at 11:19:40AM +0530, shveta malik wrote:
> On Thu, Apr 23, 2026 at 4:45 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Thu, Apr 23, 2026 at 1:01 AM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > >
> I tried to find if any other code-part does that, found one refernce
> in hstore_svals():
> 
> /* ugly ugly ugly. why no macro for this? */
> (funcctx)->call_cntr++;
> 
> Having said that, I could not find any other way to implement the fix
> also.

What about introducing a publication_tables_state struct stored in user_fctx
that carries both the list and a private position index? (kind of what 
pg_timezone_abbrevs_zone() is doing).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-24T07:17:03Z

On Fri, Apr 24, 2026 at 11:51 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On Fri, Apr 24, 2026 at 11:19:40AM +0530, shveta malik wrote:
> > On Thu, Apr 23, 2026 at 4:45 PM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > On Thu, Apr 23, 2026 at 1:01 AM Bharath Rupireddy
> > > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > > >
> > I tried to find if any other code-part does that, found one refernce
> > in hstore_svals():
> >
> > /* ugly ugly ugly. why no macro for this? */
> > (funcctx)->call_cntr++;
> >
> > Having said that, I could not find any other way to implement the fix
> > also.
>
> What about introducing a publication_tables_state struct stored in user_fctx
> that carries both the list and a private position index? (kind of what
> pg_timezone_abbrevs_zone() is doing).
>

Yeah, that is a good idea. Seems doable.

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-24T20:20:56Z

Hi,

On Fri, Apr 24, 2026 at 12:17 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> > What about introducing a publication_tables_state struct stored in user_fctx
> > that carries both the list and a private position index? (kind of what
> > pg_timezone_abbrevs_zone() is doing).
>
> Yeah, that is a good idea. Seems doable.

+1. Thanks for the pointer. Adding a new struct to carry both the
table_infos and the current index into it seems simple with a smaller
diff.

If I were to think of another approach (I don't prefer this approach
anyway), we could convert pg_get_publication_tables from the current
value-per-call SRF function (SRF_IS_FIRSTCALL + SRF_RETURN_NEXT) to a
materialized SRF function (InitMaterializedSRF +
tuplestore_putvalues). With a materialized SRF function, there is no
need to add a new structure or maintain per-call context - table_infos
becomes a local variable, and we skip placing anything related to
dropped tables into the tuplestore immediately in the second loop (the
first loop remains the same, preparing the table_infos list). This
approach seems more complex with a larger diff and requires use of
InitMaterializedSRF for versions >= PG16, SetSingleFuncCall for PG15,
and for PG14 requires inlining SetSingleFuncCall.

I prefer adding the new struct to carry both table_infos and the
current index into it with the current value-per-call SRF function,
unless others have better ideas. If okay, I will send a new patch
soon. Thank you!

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-27T03:45:32Z

On Sat, Apr 25, 2026 at 1:51 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Fri, Apr 24, 2026 at 12:17 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > > What about introducing a publication_tables_state struct stored in user_fctx
> > > that carries both the list and a private position index? (kind of what
> > > pg_timezone_abbrevs_zone() is doing).
> >
> > Yeah, that is a good idea. Seems doable.
>
> +1. Thanks for the pointer. Adding a new struct to carry both the
> table_infos and the current index into it seems simple with a smaller
> diff.
>
> If I were to think of another approach (I don't prefer this approach
> anyway), we could convert pg_get_publication_tables from the current
> value-per-call SRF function (SRF_IS_FIRSTCALL + SRF_RETURN_NEXT) to a
> materialized SRF function (InitMaterializedSRF +
> tuplestore_putvalues). With a materialized SRF function, there is no
> need to add a new structure or maintain per-call context - table_infos
> becomes a local variable, and we skip placing anything related to
> dropped tables into the tuplestore immediately in the second loop (the
> first loop remains the same, preparing the table_infos list). This
> approach seems more complex with a larger diff and requires use of
> InitMaterializedSRF for versions >= PG16, SetSingleFuncCall for PG15,
> and for PG14 requires inlining SetSingleFuncCall.
>
> I prefer adding the new struct to carry both table_infos and the
> current index into it with the current value-per-call SRF function,
> unless others have better ideas.

+1. It is simpler than the Materialization concept.

> If okay, I will send a new patch
> soon. Thank you!
>

Sure, Thanks!

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-27T08:36:00Z

Hi,

On Sun, Apr 26, 2026 at 8:45 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> > I prefer adding the new struct to carry both table_infos and the
> > current index into it with the current value-per-call SRF function,
> > unless others have better ideas.
>
> +1. It is simpler than the Materialization concept.
>
> > If okay, I will send a new patch
> > soon. Thank you!
>
> Sure, Thanks!

Attached v2 patch. I also refactored the test a bit. Please review. Thank you!

I added this to the current CF: https://commitfest.postgresql.org/patch/6715/.

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-27T10:01:17Z

Hi,

On Mon, Apr 27, 2026 at 01:36:00AM -0700, Bharath Rupireddy wrote:
> Hi,
> 
> On Sun, Apr 26, 2026 at 8:45 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > > I prefer adding the new struct to carry both table_infos and the
> > > current index into it with the current value-per-call SRF function,
> > > unless others have better ideas.
> >
> > +1. It is simpler than the Materialization concept.
> >
> > > If okay, I will send a new patch
> > > soon. Thank you!
> >
> > Sure, Thanks!
> 
> Attached v2 patch. I also refactored the test a bit. Please review. Thank you!

Thanks!

I've 2 comments:

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.

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?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-28T01:40:00Z

Hi,

On Mon, Apr 27, 2026 at 3:01 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> 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!

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-28T03:59:39Z

On Tue, Apr 28, 2026 at 7:11 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Mon, Apr 27, 2026 at 3:01 AM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> 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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-28T04:21:54Z

Hi,

On Mon, Apr 27, 2026 at 8:59 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> > 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.

Sure. How about pg-get-publication-tables-after-list-built? It's 42
bytes, under the INJ_NAME_MAXLEN of 64 bytes, but meaningful.

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-28T05:54:47Z

Hi,

On Mon, Apr 27, 2026 at 09:21:54PM -0700, Bharath Rupireddy wrote:
> Hi,
> 
> On Mon, Apr 27, 2026 at 8:59 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > > 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 for the new version!

> > 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.
> 
> Sure. How about pg-get-publication-tables-after-list-built? It's 42
> bytes, under the INJ_NAME_MAXLEN of 64 bytes, but meaningful.

That looks ok to me.

Just still 2 minor comments:

1/ 
+       # Drop the table while the SRF is paused.

s/the SRF/ pg_get_publication_tables()/ ?

2/

+# pg_get_publication_tables race with concurrent DROP TABLE.

What about describing a bit more the bug? The other tests in 100_bugs.pl describe
the bug they're testing.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-28T14:17:00Z

Hi,

On Mon, Apr 27, 2026 at 10:54 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> > Sure. How about pg-get-publication-tables-after-list-built? It's 42
> > bytes, under the INJ_NAME_MAXLEN of 64 bytes, but meaningful.
>
> That looks ok to me.
>
> Just still 2 minor comments:
>
> 1/
> +       # Drop the table while the SRF is paused.
>
> s/the SRF/ pg_get_publication_tables()/ ?
>
> 2/
>
> +# pg_get_publication_tables race with concurrent DROP TABLE.
>
> What about describing a bit more the bug? The other tests in 100_bugs.pl describe
> the bug they're testing.

Please find the attached v4 patch. Thank yoU!

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-28T15:28:06Z

Hi,

On Tue, Apr 28, 2026 at 07:17:00AM -0700, Bharath Rupireddy wrote:
> Hi,
> 
> On Mon, Apr 27, 2026 at 10:54 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > > Sure. How about pg-get-publication-tables-after-list-built? It's 42
> > > bytes, under the INJ_NAME_MAXLEN of 64 bytes, but meaningful.
> >
> > That looks ok to me.
> >
> > Just still 2 minor comments:
> >
> > 1/
> > +       # Drop the table while the SRF is paused.
> >
> > s/the SRF/ pg_get_publication_tables()/ ?
> >
> > 2/
> >
> > +# pg_get_publication_tables race with concurrent DROP TABLE.
> >
> > What about describing a bit more the bug? The other tests in 100_bugs.pl describe
> > the bug they're testing.
> 
> Please find the attached v4 patch. Thank yoU!

Thanks, LGTM!

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Ajin Cherian <itsajin@gmail.com> — 2026-04-29T01:42:23Z

On Wed, Apr 29, 2026 at 12:17 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
>
>
> Please find the attached v4 patch. Thank yoU!
>

One small comment. The includes need to be in alphabetical order.
injection_point.h should come after fmgroids.h

 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
+#include "utils/injection_point.h"
 #include "utils/syscache.h"

regards,
Ajin Cherian
Fujitsu Australia

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-04-29T03:27:51Z

On Wed, Apr 29, 2026 at 7:12 AM Ajin Cherian <itsajin@gmail.com> wrote:
>
> On Wed, Apr 29, 2026 at 12:17 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> >
> >
> > Please find the attached v4 patch. Thank yoU!
> >
>
> One small comment. The includes need to be in alphabetical order.
> injection_point.h should come after fmgroids.h
>
>  #include "utils/fmgroids.h"
>  #include "utils/lsyscache.h"
>  #include "utils/rel.h"
> +#include "utils/injection_point.h"
>  #include "utils/syscache.h"
>

+1

Also there is a trailing whitespace issue while applying the patch.
Other than these, the patch looks good.

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-04-29T07:15:00Z

Hi,

On Tue, Apr 28, 2026 at 8:28 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Wed, Apr 29, 2026 at 7:12 AM Ajin Cherian <itsajin@gmail.com> wrote:
>
> > One small comment. The includes need to be in alphabetical order.
> > injection_point.h should come after fmgroids.h
> >
> > +#include "utils/injection_point.h"
> >  #include "utils/syscache.h"
>
> Also there is a trailing whitespace issue while applying the patch.
> Other than these, the patch looks good.

Fixed. Please find the attached v5 patch.

The fix is needed only for PG16 and later, not PG15 or PG14. The bug
was introduced by b7ae03953690 [1] in PG16, which added a table_open()
call in pg_get_publication_tables(). PG15 and earlier only use
get_rel_namespace() and syscache lookups, both of which gracefully
handle dropped relations (returning InvalidOid/false rather than
erroring).

I verified the bug and the fix on all affected branches. Please find
the attached version-specific patches for backpatching. Thank you!

[1] b7ae03953690 - Ignore dropped and generated columns from the column list

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Chao Li <li.evan.chao@gmail.com> — 2026-04-30T03:40:20Z


> On Apr 29, 2026, at 15:15, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
> 
> Hi,
> 
> On Tue, Apr 28, 2026 at 8:28 PM shveta malik <shveta.malik@gmail.com> wrote:
>> 
>> On Wed, Apr 29, 2026 at 7:12 AM Ajin Cherian <itsajin@gmail.com> wrote:
>> 
>>> One small comment. The includes need to be in alphabetical order.
>>> injection_point.h should come after fmgroids.h
>>> 
>>> +#include "utils/injection_point.h"
>>> #include "utils/syscache.h"
>> 
>> Also there is a trailing whitespace issue while applying the patch.
>> Other than these, the patch looks good.
> 
> Fixed. Please find the attached v5 patch.
> 
> The fix is needed only for PG16 and later, not PG15 or PG14. The bug
> was introduced by b7ae03953690 [1] in PG16, which added a table_open()
> call in pg_get_publication_tables(). PG15 and earlier only use
> get_rel_namespace() and syscache lookups, both of which gracefully
> handle dropped relations (returning InvalidOid/false rather than
> erroring).
> 
> I verified the bug and the fix on all affected branches. Please find
> the attached version-specific patches for backpatching. Thank you!
> 
> [1] b7ae03953690 - Ignore dropped and generated columns from the column list
> 
> --
> Bharath Rupireddy
> Amazon Web Services: https://aws.amazon.com
> <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>

I am afraid this is only a partial fix.

```
@@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
 		/* Show all columns when the column list is not specified. */
 		if (nulls[2])
 		{
-			Relation	rel = table_open(relid, AccessShareLock);
+			Relation	rel = try_table_open(relid, AccessShareLock);
 			int			nattnums = 0;
 			int16	   *attnums;
-			TupleDesc	desc = RelationGetDescr(rel);
+			TupleDesc	desc;
 			int			i;
 
+			/* Skip if the relation has been concurrently dropped. */
+			if (rel == NULL)
+				continue;
```

This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().

So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list. It also introduces inconsistent behavior between tables published with and without column lists.

To resolve the race condition completely, I think we should try to open the table regardless of whether a column list is specified.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-05-01T01:30:00Z

Hi,

On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> > <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>
>
> I am afraid this is only a partial fix.

Thanks for reviewing it. Please find my responses below.

> ```
> @@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
>                 /* Show all columns when the column list is not specified. */
>                 if (nulls[2])
>                 {
> -                       Relation        rel = table_open(relid, AccessShareLock);
> +                       Relation        rel = try_table_open(relid, AccessShareLock);
>                         int                     nattnums = 0;
>                         int16      *attnums;
> -                       TupleDesc       desc = RelationGetDescr(rel);
> +                       TupleDesc       desc;
>                         int                     i;
>
> +                       /* Skip if the relation has been concurrently dropped. */
> +                       if (rel == NULL)
> +                               continue;
> ```
>
> This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().

Right. The try_table_open() is only needed there because that's the
only code path that actually opens the relation (to enumerate its
columns). The column list path reads from the pg_publication_rel
catalog - it never calls table_open(), so it cannot hit the ERROR.

> So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list.

IMO, no function returning table OIDs can guarantee they remain valid
- a drop can happen right after we return the OID, and accuracy is in
the caller's hands. All the callers of pg_get_publication_tables()
already handle this by JOINing with pg_class.

> It also introduces inconsistent behavior between tables published with and without column lists.

These two paths do different things - one needs the relation open, the
other doesn't. For callers, the outcome is the same: any stale OID
gets filtered out by their pg_class JOIN.

> To resolve the race condition completely, I think we should try to open the table regardless of whether a column list is specified.

IMO, that would add unnecessary locking overhead in a path that
doesn't need it. The bug is the ERROR, and this patch fixes it.

In short, when no column list is specified,
pg_get_publication_tables() needs to open the relation to enumerate
all publishable columns and return them as an int2vector in the attrs
output column. It currently uses table_open() for this, which errors
out with concurrent table drops. This patch fixes it by using
try_table_open() and skipping the relation if it's been dropped.

Thoughts?

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-05-04T02:34:00Z

Hi,

On Wed, Apr 29, 2026 at 12:15 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Fixed. Please find the attached v5 patch.
>
> The fix is needed only for PG16 and later, not PG15 or PG14. The bug
> was introduced by b7ae03953690 [1] in PG16, which added a table_open()
> call in pg_get_publication_tables(). PG15 and earlier only use
> get_rel_namespace() and syscache lookups, both of which gracefully
> handle dropped relations (returning InvalidOid/false rather than
> erroring).
>
> I verified the bug and the fix on all affected branches. Please find
> the attached version-specific patches for backpatching. Thank you!
>
> [1] b7ae03953690 - Ignore dropped and generated columns from the column list

Please find the attached v6 patch, which fixes a test failure on
FreeBSD. This variant of the build forces parallel query via
debug_parallel_query=regress, causing the pg_get_publication_tables
injection point to fire in parallel workers as well. I disabled forced
parallel query for this test case.

Thank you!


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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-05-04T05:08:10Z

On Fri, May 1, 2026 at 7:00 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> > > <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>
> >
> > I am afraid this is only a partial fix.
>
> Thanks for reviewing it. Please find my responses below.
>
> > ```
> > @@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
> >                 /* Show all columns when the column list is not specified. */
> >                 if (nulls[2])
> >                 {
> > -                       Relation        rel = table_open(relid, AccessShareLock);
> > +                       Relation        rel = try_table_open(relid, AccessShareLock);
> >                         int                     nattnums = 0;
> >                         int16      *attnums;
> > -                       TupleDesc       desc = RelationGetDescr(rel);
> > +                       TupleDesc       desc;
> >                         int                     i;
> >
> > +                       /* Skip if the relation has been concurrently dropped. */
> > +                       if (rel == NULL)
> > +                               continue;
> > ```
> >
> > This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().
>
> Right. The try_table_open() is only needed there because that's the
> only code path that actually opens the relation (to enumerate its
> columns). The column list path reads from the pg_publication_rel
> catalog - it never calls table_open(), so it cannot hit the ERROR.
>
> > So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list.
>
> IMO, no function returning table OIDs can guarantee they remain valid
> - a drop can happen right after we return the OID, and accuracy is in
> the caller's hands. All the callers of pg_get_publication_tables()
> already handle this by JOINing with pg_class.
>

I agree with Bharath. Also I would like to add one more point. We do have this:

+ /* Skip if the relation has been concurrently dropped. */
+ if (!OidIsValid(schemaid))
+ continue;

So if a table is dropped before we could access its explicitly
mentioned column-list, above should handle it right even without
'try_table_open' done? Moreover, pg_publication_rel will not return
any row for dropped table, so we should be good. For 'table_infos',
the case was different, as we saved the list in first call and are
using that in subsequent call? But that is not the case with
pg_publication_rel.

> > It also introduces inconsistent behavior between tables published with and without column lists.
>
> These two paths do different things - one needs the relation open, the
> other doesn't. For callers, the outcome is the same: any stale OID
> gets filtered out by their pg_class JOIN.
>
> > To resolve the race condition completely, I think we should try to open the table regardless of whether a column list is specified.
>
> IMO, that would add unnecessary locking overhead in a path that
> doesn't need it. The bug is the ERROR, and this patch fixes it.
>
> In short, when no column list is specified,
> pg_get_publication_tables() needs to open the relation to enumerate
> all publishable columns and return them as an int2vector in the attrs
> output column. It currently uses table_open() for this, which errors
> out with concurrent table drops. This patch fixes it by using
> try_table_open() and skipping the relation if it's been dropped.
>
> Thoughts?
>
> --
> Bharath Rupireddy
> Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Chao Li <li.evan.chao@gmail.com> — 2026-05-05T03:08:29Z


> On May 1, 2026, at 09:30, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
> 
> Hi,
> 
> On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
>> 
>>> <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>
>> 
>> I am afraid this is only a partial fix.
> 
> Thanks for reviewing it. Please find my responses below.
> 
>> ```
>> @@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
>>                /* Show all columns when the column list is not specified. */
>>                if (nulls[2])
>>                {
>> -                       Relation        rel = table_open(relid, AccessShareLock);
>> +                       Relation        rel = try_table_open(relid, AccessShareLock);
>>                        int                     nattnums = 0;
>>                        int16      *attnums;
>> -                       TupleDesc       desc = RelationGetDescr(rel);
>> +                       TupleDesc       desc;
>>                        int                     i;
>> 
>> +                       /* Skip if the relation has been concurrently dropped. */
>> +                       if (rel == NULL)
>> +                               continue;
>> ```
>> 
>> This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().
> 
> Right. The try_table_open() is only needed there because that's the
> only code path that actually opens the relation (to enumerate its
> columns). The column list path reads from the pg_publication_rel
> catalog - it never calls table_open(), so it cannot hit the ERROR.
> 
>> So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list.
> 
> IMO, no function returning table OIDs can guarantee they remain valid
> - a drop can happen right after we return the OID, and accuracy is in
> the caller's hands. All the callers of pg_get_publication_tables()
> already handle this by JOINing with pg_class.
> 

Fair.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Chao Li <li.evan.chao@gmail.com> — 2026-05-05T03:10:10Z


> On May 4, 2026, at 13:08, shveta malik <shveta.malik@gmail.com> wrote:
> 
> On Fri, May 1, 2026 at 7:00 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
>> 
>> Hi,
>> 
>> On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>> 
>>>> <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>
>>> 
>>> I am afraid this is only a partial fix.
>> 
>> Thanks for reviewing it. Please find my responses below.
>> 
>>> ```
>>> @@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
>>>                /* Show all columns when the column list is not specified. */
>>>                if (nulls[2])
>>>                {
>>> -                       Relation        rel = table_open(relid, AccessShareLock);
>>> +                       Relation        rel = try_table_open(relid, AccessShareLock);
>>>                        int                     nattnums = 0;
>>>                        int16      *attnums;
>>> -                       TupleDesc       desc = RelationGetDescr(rel);
>>> +                       TupleDesc       desc;
>>>                        int                     i;
>>> 
>>> +                       /* Skip if the relation has been concurrently dropped. */
>>> +                       if (rel == NULL)
>>> +                               continue;
>>> ```
>>> 
>>> This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().
>> 
>> Right. The try_table_open() is only needed there because that's the
>> only code path that actually opens the relation (to enumerate its
>> columns). The column list path reads from the pg_publication_rel
>> catalog - it never calls table_open(), so it cannot hit the ERROR.
>> 
>>> So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list.
>> 
>> IMO, no function returning table OIDs can guarantee they remain valid
>> - a drop can happen right after we return the OID, and accuracy is in
>> the caller's hands. All the callers of pg_get_publication_tables()
>> already handle this by JOINing with pg_class.
>> 
> 
> I agree with Bharath. Also I would like to add one more point. We do have this:
> 
> + /* Skip if the relation has been concurrently dropped. */
> + if (!OidIsValid(schemaid))
> + continue;
> 

Actually, this is the other comment I have. Why the comment says “if the relation has been dropped”, but the actual check is on schema id?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

shveta malik <shveta.malik@gmail.com> — 2026-05-05T04:02:00Z

On Tue, May 5, 2026 at 8:40 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On May 4, 2026, at 13:08, shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Fri, May 1, 2026 at 7:00 AM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >>>
> >>>> <v5-0001-PG16-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-Fix-pg_get_publication_tables-race-with-concurren.patch><v5-0001-PG18-Fix-pg_get_publication_tables-race-with-conc.txt><v5-0001-PG17-Fix-pg_get_publication_tables-race-with-conc.txt>
> >>>
> >>> I am afraid this is only a partial fix.
> >>
> >> Thanks for reviewing it. Please find my responses below.
> >>
> >>> ```
> >>> @@ -1599,12 +1621,18 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
> >>>                /* Show all columns when the column list is not specified. */
> >>>                if (nulls[2])
> >>>                {
> >>> -                       Relation        rel = table_open(relid, AccessShareLock);
> >>> +                       Relation        rel = try_table_open(relid, AccessShareLock);
> >>>                        int                     nattnums = 0;
> >>>                        int16      *attnums;
> >>> -                       TupleDesc       desc = RelationGetDescr(rel);
> >>> +                       TupleDesc       desc;
> >>>                        int                     i;
> >>>
> >>> +                       /* Skip if the relation has been concurrently dropped. */
> >>> +                       if (rel == NULL)
> >>> +                               continue;
> >>> ```
> >>>
> >>> This change uses try_table_open() to detect whether a table has been dropped, but try_table_open() is only called when the column list is not specified. If a table is included in the publication with an explicit column list, then even if it is dropped concurrently, it may still be returned by pg_get_publication_tables().
> >>
> >> Right. The try_table_open() is only needed there because that's the
> >> only code path that actually opens the relation (to enumerate its
> >> columns). The column list path reads from the pg_publication_rel
> >> catalog - it never calls table_open(), so it cannot hit the ERROR.
> >>
> >>> So this patch removes the “could not open relation with OID” error, but it does not fully ensure the accuracy of the returned table list.
> >>
> >> IMO, no function returning table OIDs can guarantee they remain valid
> >> - a drop can happen right after we return the OID, and accuracy is in
> >> the caller's hands. All the callers of pg_get_publication_tables()
> >> already handle this by JOINing with pg_class.
> >>
> >
> > I agree with Bharath. Also I would like to add one more point. We do have this:
> >
> > + /* Skip if the relation has been concurrently dropped. */
> > + if (!OidIsValid(schemaid))
> > + continue;
> >
>
> Actually, this is the other comment I have. Why the comment says “if the relation has been dropped”, but the actual check is on schema id?
>

Okay, I see your point. Shall we tweak it to the below to make it more
understandable?


Oid schemaid;

/*
* get_rel_namespace() returns InvalidOid if the relation no longer exists
* (e.g., dropped concurrently). Skip such entries.
*/
if (!OidIsValid(schemaid = get_rel_namespace(relid)))
continue;

thanks
Shveta

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Masahiko Sawada <sawada.mshk@gmail.com> — 2026-05-12T21:20:14Z

Hi,

On Sun, May 3, 2026 at 7:35 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Wed, Apr 29, 2026 at 12:15 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > Fixed. Please find the attached v5 patch.
> >
> > The fix is needed only for PG16 and later, not PG15 or PG14. The bug
> > was introduced by b7ae03953690 [1] in PG16, which added a table_open()
> > call in pg_get_publication_tables(). PG15 and earlier only use
> > get_rel_namespace() and syscache lookups, both of which gracefully
> > handle dropped relations (returning InvalidOid/false rather than
> > erroring).
> >
> > I verified the bug and the fix on all affected branches. Please find
> > the attached version-specific patches for backpatching. Thank you!
> >
> > [1] b7ae03953690 - Ignore dropped and generated columns from the column list
>
> Please find the attached v6 patch, which fixes a test failure on
> FreeBSD. This variant of the build forces parallel query via
> debug_parallel_query=regress, causing the pg_get_publication_tables
> injection point to fire in parallel workers as well. I disabled forced
> parallel query for this test case.
>

I reviewed the patch and here are some comments:

+/* State for pg_get_publication_tables SRF */
+typedef struct
+{
+ List    *table_infos; /* list of published_rel */
+ int curr_idx; /* current index into table_infos */
+} publication_tables_state;

I think we can define publication_table_state in
pg_get_publication_tables() as it's used only in that function.

---
+ /* Skip if the relation has been concurrently dropped. */
+ if (!OidIsValid(schemaid))
+ continue;

Although this check is done for all relations in table_infos, we also
check the return value of try_table_open(), and these two checks have
the same comment. I think we need more comments on why these two
checks are required.

In which case is schemaid InvalidOid and do we not call
try_table_open() (i.e., nulls[2] is false)?

It might make more sense to get-and-check the schemaid of each
relation when adding the table information to table_infos.

---
Looking at the regression tests in the patch, it tests the ALL TABLES
publication cases and the concurrently-dropped table is handled when
we call check the return value of try_table_open() but not
get_rel_namespace(). I think the test case itself is fine but we can
do the same test without adding a new injection point. For instance,

backend-1: begin;
backend-2: begin; lock table t_dropme in access exclusive mode;
backend-1: select * from pg_publication_tables;
backend-2: drop table t_dropme; commit;
backend-1: get an error "could not open relation with XXX"

---
If we use tuplestore instead of SRF, can we simplify the code as we
would not need publication_tables_state and the above check? It would
be only for the master, though.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-06-15T23:21:31Z

Hi,

On Tue, May 12, 2026 at 2:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I reviewed the patch and here are some comments:

Thank you for reviewing!

> +/* State for pg_get_publication_tables SRF */
> +typedef struct
> +{
> + List    *table_infos; /* list of published_rel */
> + int curr_idx; /* current index into table_infos */
> +} publication_tables_state;
>
> I think we can define publication_table_state in
> pg_get_publication_tables() as it's used only in that function.

Done for pre-HEAD versions. For HEAD, I used the SRF approach.

> ---
> + /* Skip if the relation has been concurrently dropped. */
> + if (!OidIsValid(schemaid))
> + continue;
>
> Although this check is done for all relations in table_infos, we also
> check the return value of try_table_open(), and these two checks have
> the same comment. I think we need more comments on why these two
> checks are required.

When get_rel_namespace() returns InvalidOid for the concurrently
dropped tables, it costs additional syscache lookups plus index scans
(cache miss). So, there's no harm from the correctness perspective.

However, I would like to optimize this part of the code by 1/ gating
it with InvalidOid checkup, 2/ moving the schemaid fetch closer to
where it's being used i.e. inside if (!pub->alltables). I would like
to do this only for HEAD, so, I attached it as a separate 0002 patch.
For pre-HEAD versions, I removed this check and retained the
try_table_open() fix.

> In which case is schemaid InvalidOid and do we not call
> try_table_open() (i.e., nulls[2] is false)?

This case is NOT possible, because the drop table ensures dependent
catalog entries (pg_publication_namespace in this case) are deleted
too.

> It might make more sense to get-and-check the schemaid of each
> relation when adding the table information to table_infos.

That won't help fix the table_open() error with concurrent table drops.

> ---
> Looking at the regression tests in the patch, it tests the ALL TABLES
> publication cases and the concurrently-dropped table is handled when
> we call check the return value of try_table_open() but not
> get_rel_namespace(). I think the test case itself is fine but we can
> do the same test without adding a new injection point. For instance,
>
> backend-1: begin;
> backend-2: begin; lock table t_dropme in access exclusive mode;
> backend-1: select * from pg_publication_tables;
> backend-2: drop table t_dropme; commit;
> backend-1: get an error "could not open relation with XXX"

Nice! It works. I used this for the test-case in the new patches.

> ---
> If we use tuplestore instead of SRF, can we simplify the code as we
> would not need publication_tables_state and the above check? It would
> be only for the master, though.

I implemented this idea for HEAD and it simplifies the code a bit.

Please find the attached v7 patches.

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Masahiko Sawada <sawada.mshk@gmail.com> — 2026-06-17T01:20:17Z

On Mon, Jun 15, 2026 at 4:21 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Tue, May 12, 2026 at 2:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I reviewed the patch and here are some comments:
>
> Thank you for reviewing!
>
> > +/* State for pg_get_publication_tables SRF */
> > +typedef struct
> > +{
> > + List    *table_infos; /* list of published_rel */
> > + int curr_idx; /* current index into table_infos */
> > +} publication_tables_state;
> >
> > I think we can define publication_table_state in
> > pg_get_publication_tables() as it's used only in that function.
>
> Done for pre-HEAD versions. For HEAD, I used the SRF approach.
>
> > ---
> > + /* Skip if the relation has been concurrently dropped. */
> > + if (!OidIsValid(schemaid))
> > + continue;
> >
> > Although this check is done for all relations in table_infos, we also
> > check the return value of try_table_open(), and these two checks have
> > the same comment. I think we need more comments on why these two
> > checks are required.
>
> When get_rel_namespace() returns InvalidOid for the concurrently
> dropped tables, it costs additional syscache lookups plus index scans
> (cache miss). So, there's no harm from the correctness perspective.
>
> However, I would like to optimize this part of the code by 1/ gating
> it with InvalidOid checkup, 2/ moving the schemaid fetch closer to
> where it's being used i.e. inside if (!pub->alltables). I would like
> to do this only for HEAD, so, I attached it as a separate 0002 patch.
> For pre-HEAD versions, I removed this check and retained the
> try_table_open() fix.

I'm not sure skipping get_rel_namespace() contributes to a visible
performance gain. Rather I'm concerned that checking
SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP) with
schemaid=InvalidOid could lead to unexpected results.

I'm concerned about the fact that the patch handles only the case
where the table doesn't have the column list. The tables in the result
not having the column list are guaranteed to be present since we do
the existence check for them but others are not. I guess it's reliable
if we could call try_table_open() for all tables in the table_infos,
and check its namespace using RelationGetNamespace(). I think it
doesn't lead to noticeable overheads in practice as we're already
calling table_open() for all tables in ALL TABLES publications or ALL
TABLE IN SCHEMA publications where users cannot specify the column
list. What do you think?

> > ---
> > If we use tuplestore instead of SRF, can we simplify the code as we
> > would not need publication_tables_state and the above check? It would
> > be only for the master, though.
>
> I implemented this idea for HEAD and it simplifies the code a bit.

Sorry, I should have mentioned that the changes to use tuplestore
instead of SRF should be PG20. I think that we should not such a
refactoring even PG19.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-06-23T22:40:12Z

Hi,

On Tue, Jun 16, 2026 at 6:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> > However, I would like to optimize this part of the code by 1/ gating
> > it with InvalidOid checkup, 2/ moving the schemaid fetch closer to
> > where it's being used i.e. inside if (!pub->alltables). I would like
> > to do this only for HEAD, so, I attached it as a separate 0002 patch.
> > For pre-HEAD versions, I removed this check and retained the
> > try_table_open() fix.
>
> I'm not sure skipping get_rel_namespace() contributes to a visible
> performance gain. Rather I'm concerned that checking
> SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP) with
> schemaid=InvalidOid could lead to unexpected results.

Thanks for reviewing!

AFAICS it won't give unexpected results. The first time it looks up a
publication with an invalid schema OID, it would cause a cache miss
and leave a negative cache entry - but that's per publication, not per
relation, so negligible.

That said, with the approach of using try_table_open for all tables
regardless and getting the schema from the open relation via
RelationGetNamespace, schemaid is always valid by the time it reaches
the syscache lookup (might be worth adding an assertion here). So the
get_rel_namespace optimization (0002) can be dropped entirely.

> I'm concerned about the fact that the patch handles only the case
> where the table doesn't have the column list. The tables in the result
> not having the column list are guaranteed to be present since we do
> the existence check for them but others are not. I guess it's reliable
> if we could call try_table_open() for all tables in the table_infos,
> and check its namespace using RelationGetNamespace(). I think it
> doesn't lead to noticeable overheads in practice as we're already
> calling table_open() for all tables in ALL TABLES publications or ALL
> TABLE IN SCHEMA publications where users cannot specify the column
> list. What do you think?

My earlier argument was that no function returning table OIDs can
guarantee they remain valid - a drop can happen right after we return
the OID, and accuracy is in the caller's hands. All the callers of
pg_get_publication_tables already handle this by JOINing with
pg_class.

However, a closer look at other functions that either build a list of
table OIDs (expand_partitioned_rtentry) or work on previously built
table OIDs (vacuum_open_relation) proves me wrong - they all account
for concurrent table drops with try_table_open. So, I'm convinced to
add try_table_open in pg_get_publication_tables for all the tables
regardless, unless I'm missing something here.

> > > If we use tuplestore instead of SRF, can we simplify the code as we
> > > would not need publication_tables_state and the above check? It would
> > > be only for the master, though.
> >
> > I implemented this idea for HEAD and it simplifies the code a bit.
>
> Sorry, I should have mentioned that the changes to use tuplestore
> instead of SRF should be PG20. I think that we should not such a
> refactoring even PG19.

I will drop the tuplestore changes for now and repost them as a
refactoring patch after the PG20 dev branch is cut.

Thoughts?

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Masahiko Sawada <sawada.mshk@gmail.com> — 2026-06-26T20:30:23Z

On Tue, Jun 23, 2026 at 3:40 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jun 16, 2026 at 6:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > > However, I would like to optimize this part of the code by 1/ gating
> > > it with InvalidOid checkup, 2/ moving the schemaid fetch closer to
> > > where it's being used i.e. inside if (!pub->alltables). I would like
> > > to do this only for HEAD, so, I attached it as a separate 0002 patch.
> > > For pre-HEAD versions, I removed this check and retained the
> > > try_table_open() fix.
> >
> > I'm not sure skipping get_rel_namespace() contributes to a visible
> > performance gain. Rather I'm concerned that checking
> > SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP) with
> > schemaid=InvalidOid could lead to unexpected results.
>
> Thanks for reviewing!
>
> AFAICS it won't give unexpected results. The first time it looks up a
> publication with an invalid schema OID, it would cause a cache miss
> and leave a negative cache entry - but that's per publication, not per
> relation, so negligible.
>
> That said, with the approach of using try_table_open for all tables
> regardless and getting the schema from the open relation via
> RelationGetNamespace, schemaid is always valid by the time it reaches
> the syscache lookup (might be worth adding an assertion here). So the
> get_rel_namespace optimization (0002) can be dropped entirely.
>
> > I'm concerned about the fact that the patch handles only the case
> > where the table doesn't have the column list. The tables in the result
> > not having the column list are guaranteed to be present since we do
> > the existence check for them but others are not. I guess it's reliable
> > if we could call try_table_open() for all tables in the table_infos,
> > and check its namespace using RelationGetNamespace(). I think it
> > doesn't lead to noticeable overheads in practice as we're already
> > calling table_open() for all tables in ALL TABLES publications or ALL
> > TABLE IN SCHEMA publications where users cannot specify the column
> > list. What do you think?
>
> My earlier argument was that no function returning table OIDs can
> guarantee they remain valid - a drop can happen right after we return
> the OID, and accuracy is in the caller's hands. All the callers of
> pg_get_publication_tables already handle this by JOINing with
> pg_class.
>
> However, a closer look at other functions that either build a list of
> table OIDs (expand_partitioned_rtentry) or work on previously built
> table OIDs (vacuum_open_relation) proves me wrong - they all account
> for concurrent table drops with try_table_open. So, I'm convinced to
> add try_table_open in pg_get_publication_tables for all the tables
> regardless, unless I'm missing something here.

+1

>
> > > > If we use tuplestore instead of SRF, can we simplify the code as we
> > > > would not need publication_tables_state and the above check? It would
> > > > be only for the master, though.
> > >
> > > I implemented this idea for HEAD and it simplifies the code a bit.
> >
> > Sorry, I should have mentioned that the changes to use tuplestore
> > instead of SRF should be PG20. I think that we should not such a
> > refactoring even PG19.
>
> I will drop the tuplestore changes for now and repost them as a
> refactoring patch after the PG20 dev branch is cut.
>
> Thoughts?

+1. We can revisit the tuplestore idea for PG20, possibly in a
separate thread. For bug fixing, let's focus on making it simple and
less invasive.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-06-27T05:49:00Z

Hi,

On Fri, Jun 26, 2026 at 1:31 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> > My earlier argument was that no function returning table OIDs can
> > guarantee they remain valid - a drop can happen right after we return
> > the OID, and accuracy is in the caller's hands. All the callers of
> > pg_get_publication_tables already handle this by JOINing with
> > pg_class.
> >
> > However, a closer look at other functions that either build a list of
> > table OIDs (expand_partitioned_rtentry) or work on previously built
> > table OIDs (vacuum_open_relation) proves me wrong - they all account
> > for concurrent table drops with try_table_open. So, I'm convinced to
> > add try_table_open in pg_get_publication_tables for all the tables
> > regardless, unless I'm missing something here.
>
> +1
>
> > I will drop the tuplestore changes for now and repost them as a
> > refactoring patch after the PG20 dev branch is cut.
> >
> > Thoughts?
>
> +1. We can revisit the tuplestore idea for PG20, possibly in a
> separate thread. For bug fixing, let's focus on making it simple and
> less invasive.

Please find the attached v8 patches. Thanks!

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

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Masahiko Sawada <sawada.mshk@gmail.com> — 2026-06-30T17:37:57Z

On Fri, Jun 26, 2026 at 10:49 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Fri, Jun 26, 2026 at 1:31 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > > My earlier argument was that no function returning table OIDs can
> > > guarantee they remain valid - a drop can happen right after we return
> > > the OID, and accuracy is in the caller's hands. All the callers of
> > > pg_get_publication_tables already handle this by JOINing with
> > > pg_class.
> > >
> > > However, a closer look at other functions that either build a list of
> > > table OIDs (expand_partitioned_rtentry) or work on previously built
> > > table OIDs (vacuum_open_relation) proves me wrong - they all account
> > > for concurrent table drops with try_table_open. So, I'm convinced to
> > > add try_table_open in pg_get_publication_tables for all the tables
> > > regardless, unless I'm missing something here.
> >
> > +1
> >
> > > I will drop the tuplestore changes for now and repost them as a
> > > refactoring patch after the PG20 dev branch is cut.
> > >
> > > Thoughts?
> >
> > +1. We can revisit the tuplestore idea for PG20, possibly in a
> > separate thread. For bug fixing, let's focus on making it simple and
> > less invasive.
>
> Please find the attached v8 patches. Thanks!

Thank you for updating the patches!

While changes to pg_get_publication_tables() look good to me, I think
the regression tests should be implemented as an isolation test rather
than a TAP test in 100_bugs.pl as it can be tested without subscribers
and the isolation test works well for this kind of race condition
tests.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2026-07-04T05:26:00Z

Hi,

On Tue, Jun 30, 2026 at 10:38 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> While changes to pg_get_publication_tables() look good to me,

Thanks for reviewing!

> I think
> the regression tests should be implemented as an isolation test rather
> than a TAP test in 100_bugs.pl as it can be tested without subscribers
> and the isolation test works well for this kind of race condition
> tests.

Agreed. The isolation test avoids the extra publisher start/stop the
TAP version needed, and it's about the same amount of code. Please
find the attached v9 patches.

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