Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(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-05-05 03:10:10
Message-ID: 9120FADD-0D8B-4A6D-B333-99FCE7AE6DCB@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On May 4, 2026, at 13:08, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Fri, May 1, 2026 at 7:00 AM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> On Wed, Apr 29, 2026 at 8:41 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2026-05-05 03:53:07 Re: Row pattern recognition
Previous Message Euler Taveira 2026-05-05 03:08:58 Re: [Patch] Omit virtual generated columns from test_decoding output