Re: Assertion failure in GetSubscriptionRelations() with concurrent DROP TABLE

Hi,

On Mon, Aug 24, 2026 at 9:52 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Aug 24, 2026 at 11:35 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > If it's not easily reproducible with direct SQL, I don't think we need
> > to fix this at all, because none of the other places (for example,
> > getOwnedSequences_internal() or GetPubPartitionOptionRelations())
> > handle a null relkind either.
>
> But they will work (as they are if checks) and as far as I can see
> they will do the right thing which means they will tolerate concurrent
> drops but if not I feel it is better to fix those as well.

Yes, it's worth exploring all of these in the logical replication code
(maybe separately though).

> > Even if we were to fix, I would just
> > remove the assertion and continue in the loop if the relation is not
> > what we expect. Would that work?
>
> As we are scanning pg_susbcription_rel unexpected relkind here (not
> '\0', something else entirely) would probably indicate a real bug
> worth surfacing, not silently swallowing. So, I suggest we can remove
> assert after Vignesh's fix and have an elog(ERROR, "unexpected
> relkind: %d", (int) relkind);. We use such an elog at other places as
> well.

Works for me, and I think both can go in one patch. After the relkind
fix, replacing the assertion with an error means that in the rare
scenario where the OID of the dropped table gets reused to create a
relation that's not expected, we catch it. I'm all in favor of
tightening this anyway.

On backpatching this fix, I think HEAD and PG19 (still in beta) are
enough, because it's not easily reproducible with direct SQL. For the
same reason, I'm fine with not having an injection point and a TAP
test for this.

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