Ilya Gladyshev <ilya.v.gladyshev@gmail.com> writes:
> [ v2-0001-check-relkind-of-subscription-target-recursively.patch ]
Hmm. I like Shi yu's way better (formal patch attached). Checking
at CREATE/ALTER SUBSCRIPTION is much more complicated, and it's really
insufficient, because what if someone adds a new partition after
setting up the subscription?
I get the argument about it being a useful check for simple mistakes,
but I don't entirely buy that argument, because I think there are
potential use-cases that it'd disallow needlessly. Imagine a
partitioned table that receives replication updates, but only into
the "current" partition; older partitions are basically static.
Now suppose you'd like to offload some of that old seldom-used data
to another server, and make those partitions into foreign tables
so you can still access it at need. Such a setup will work perfectly
fine today, but this patch would break it.
So I think what we want is to check when we identify the partition.
Maybe Shi yu missed a place or two to check, but I verified that the
attached stops the crash.
There'd still be value in refactoring to avoid premature lookup
of the namespace name, but that's just micro-optimization.
regards, tom lane
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 5250ae7f54..17b9c42ecf 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2176,6 +2176,15 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
Assert(partrelinfo != NULL);
partrel = partrelinfo->ri_RelationDesc;
+ /*
+ * Check for supported relkind. We need this since partitions might be of
+ * unsupported relkinds; and the set of partitions can change, so checking
+ * at CREATE/ALTER SUBSCRIPTION would be insufficient.
+ */
+ CheckSubscriptionRelkind(partrel->rd_rel->relkind,
+ get_namespace_name(RelationGetNamespace(partrel)),
+ RelationGetRelationName(partrel));
+
/*
* To perform any of the operations below, the tuple must match the
* partition's rowtype. Convert if needed or just copy, using a dedicated
