Hi,
Right now it is possible to add a partitioned table with foreign tables
as its children as a target of a subscription. It can lead to an assert
(or a segfault, if compiled without asserts) on a logical replication
worker when the worker attempts to insert the data received via
replication into the foreign table. Reproduce with caution, the worker
is going to crash and restart indefinitely. The setup:
Publisher on 5432 port:
CREATE TABLE parent (id int, num int);
CREATE PUBLICATION parent_pub FOR TABLE parent;
Subscriber on 5433 port:
CREATE EXTENSION postgres_fdw;
CREATE SERVER loopback foreign data wrapper postgres_fdw options (host
'127.0.0.1', port '5433', dbname 'postgres');
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
CREATE TABLE parent (id int, num int) partition by range (id);
CREATE FOREIGN TABLE p1 PARTITION OF parent DEFAULT SERVER loopback;
CREATE TABLE p1_loc(id int, num int);
CREATE SUBSCRIPTION parent_sub CONNECTION 'host=127.0.0.1 port=5432
dbname=postgres' PUBLICATION parent_pub;
Then run an insert on the publisher: INSERT INTO parent VALUES (1, 1);
This will cause a segfault or raise an assert, because inserting into
foreign tables via logical replication is not possible. The solution I
propose is to add recursive checks of relkind for children of a target,
if the target is a partitioned table. I have attached a patch for this
and managed to reproduce this on REL_14_STABLE as well, not sure if a
patch for that version is also needed.
Kind Regards,
Ilya Gladyshev
From 96f1fce8fb50f527b958de13a60f7324dd1ef052 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Fri, 28 Oct 2022 23:25:20 +0400
Subject: [PATCH] check relkind of subscription target recursively
---
src/backend/commands/subscriptioncmds.c | 13 ++++---
src/backend/executor/execReplication.c | 45 ++++++++++++++++++----
src/backend/replication/logical/relation.c | 7 ++--
src/include/executor/executor.h | 4 +-
4 files changed, 51 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index f0cec2ad5e..e2dd6425e4 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -703,9 +703,10 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
relid = RangeVarGetRelid(rv, AccessShareLock, false);
- /* Check for supported relkind. */
- CheckSubscriptionRelkind(get_rel_relkind(relid),
- rv->schemaname, rv->relname);
+ /* Check for supported relkind recursively. */
+ CheckSubscriptionRelation(relid,
+ get_rel_relkind(relid),
+ rv->schemaname, rv->relname);
AddSubscriptionRelState(subid, relid, table_state,
InvalidXLogRecPtr);
@@ -864,9 +865,9 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
relid = RangeVarGetRelid(rv, AccessShareLock, false);
- /* Check for supported relkind. */
- CheckSubscriptionRelkind(get_rel_relkind(relid),
- rv->schemaname, rv->relname);
+ /* Check for supported relkind recursively. */
+ CheckSubscriptionRelation(relid, get_rel_relkind(relid),
+ rv->schemaname, rv->relname);
pubrel_local_oids[off++] = relid;
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 6014f2e248..98bc4a6618 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -16,9 +16,11 @@
#include "access/genam.h"
#include "access/relscan.h"
+#include "access/table.h"
#include "access/tableam.h"
#include "access/transam.h"
#include "access/xact.h"
+#include "catalog/pg_inherits.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "executor/nodeModifyTable.h"
@@ -645,13 +647,7 @@ CheckCmdReplicaIdentity(Relation rel, CmdType cmd)
errhint("To enable deleting from the table, set REPLICA IDENTITY using ALTER TABLE.")));
}
-
-/*
- * Check if we support writing into specific relkind.
- *
- * The nspname and relname are only needed for error reporting.
- */
-void
+static void
CheckSubscriptionRelkind(char relkind, const char *nspname,
const char *relname)
{
@@ -662,3 +658,38 @@ CheckSubscriptionRelkind(char relkind, const char *nspname,
nspname, relname),
errdetail_relkind_not_supported(relkind)));
}
+
+/*
+ * Recursively check if we support writing into specific relkind.
+ *
+ * The nspname and relname are only needed for error reporting.
+ */
+void
+CheckSubscriptionRelation(Oid relid, char relkind, const char *nspname,
+ const char *relname)
+{
+ CheckSubscriptionRelkind(relkind, nspname, relname);
+
+ if (relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *inheritors;
+ ListCell *lc;
+
+ inheritors = find_all_inheritors(relid,
+ AccessShareLock,
+ NULL);
+ foreach (lc, inheritors)
+ {
+ Oid child_oid = lfirst_oid(lc);
+ Relation child_rel = RelationIdGetRelation(child_oid);
+ char *relname;
+ char *nspname;
+
+ relname = RelationGetRelationName(child_rel);
+ nspname = get_namespace_name(RelationGetNamespace(child_rel));
+
+ CheckSubscriptionRelkind(child_rel->rd_rel->relkind, nspname, relname);
+ table_close(child_rel, AccessShareLock);
+ }
+ }
+}
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index e989047681..307ab0b559 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -392,9 +392,10 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
entry->localrel = table_open(relid, NoLock);
entry->localreloid = relid;
- /* Check for supported relkind. */
- CheckSubscriptionRelkind(entry->localrel->rd_rel->relkind,
- remoterel->nspname, remoterel->relname);
+ /* Check for supported relkind recursively. */
+ CheckSubscriptionRelation(entry->localrel->rd_rel->oid,
+ entry->localrel->rd_rel->relkind,
+ remoterel->nspname, remoterel->relname);
/*
* Build the mapping of local attribute numbers to remote attribute
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index ed95ed1176..757a8fa780 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -646,8 +646,8 @@ extern void ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
TupleTableSlot *searchslot);
extern void CheckCmdReplicaIdentity(Relation rel, CmdType cmd);
-extern void CheckSubscriptionRelkind(char relkind, const char *nspname,
- const char *relname);
+extern void CheckSubscriptionRelation(Oid relid, char relkind, const char *nspname,
+ const char *relname);
/*
* prototypes from functions in nodeModifyTable.c
--
2.30.2
