On Sun, Oct 30, 2022 9:39 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > What I'm wondering about is whether we can refactor this code > to avoid so many usually-useless catalog lookups. Pulling the > namespace name, in particular, is expensive and we generally > are not going to need the result. In the child-rel case it'd > be much better to pass the opened relation and let the error-check > subroutine work from that. Maybe we should just do it like that > at the start of the recursion, too? Or pass the relid and let > the subroutine look up the names only in the error case. > > A completely different line of thought is that this doesn't seem > like a terribly bulletproof fix, since children could get added to > a partitioned table after we look. Maybe it'd be better to check > the relkind at the last moment before we do something that depends > on a child table being a relation. >
I agree. So maybe we can add this check in apply_handle_tuple_routing(). diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 5250ae7f54..e941b68e4b 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -2176,6 +2176,10 @@ apply_handle_tuple_routing(ApplyExecutionData *edata, Assert(partrelinfo != NULL); partrel = partrelinfo->ri_RelationDesc; + /* Check for supported relkind. */ + CheckSubscriptionRelkind(partrel->rd_rel->relkind, + relmapentry->remoterel.nspname, relmapentry->remoterel.relname); + /* * To perform any of the operations below, the tuple must match the * partition's rowtype. Convert if needed or just copy, using a dedicated Regards, Shi yu