I understand that this is a minimal fix, and for that it seems OK, but I
think the surrounding style is rather baroque. This code can be made
simpler. Here's my take on it. I think it's also faster: we avoid
looking up pg_publication_rel entries for rels that aren't partitioned
tables.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli)
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 7aacb6b2fe..e8ef003fe5 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -945,60 +945,42 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
foreach(lc, root_relids)
{
- HeapTuple rftuple;
Oid relid = lfirst_oid(lc);
- bool has_column_list;
- bool has_row_filter;
+ char relkind;
+ HeapTuple rftuple;
+
+ relkind = get_rel_relkind(relid);
+ if (relkind != RELKIND_PARTITIONED_TABLE)
+ continue;
rftuple = SearchSysCache2(PUBLICATIONRELMAP,
ObjectIdGetDatum(relid),
ObjectIdGetDatum(pubform->oid));
+ if (!HeapTupleIsValid(rftuple))
+ continue;
- has_row_filter
- = !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
+ if (!heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set %s to false for publication \"%s\"",
+ "publish_via_partition_root",
+ stmt->pubname),
+ errdetail("The publication contains a WHERE clause for a partitioned table \"%s\" which is not allowed when %s is false.",
+ get_rel_name(relid),
+ "publish_via_partition_root")));
- has_column_list
- = !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
+ if (!heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set %s to false for publication \"%s\"",
+ "publish_via_partition_root",
+ stmt->pubname),
+ errdetail("The publication contains a column list for a partitioned table \"%s\" which is not allowed when %s is false.",
+ get_rel_name(relid),
+ "publish_via_partition_root")));
- if (HeapTupleIsValid(rftuple) &&
- (has_row_filter || has_column_list))
- {
- HeapTuple tuple;
+ ReleaseSysCache(rftuple);
- tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
- if (HeapTupleIsValid(tuple))
- {
- Form_pg_class relform = (Form_pg_class) GETSTRUCT(tuple);
-
- if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
- has_row_filter)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot set %s for publication \"%s\"",
- "publish_via_partition_root = false",
- stmt->pubname),
- errdetail("The publication contains a WHERE clause for a partitioned table \"%s\" "
- "which is not allowed when %s is false.",
- NameStr(relform->relname),
- "publish_via_partition_root")));
-
- if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
- has_column_list)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot set %s for publication \"%s\"",
- "publish_via_partition_root = false",
- stmt->pubname),
- errdetail("The publication contains a column list for a partitioned table \"%s\" "
- "which is not allowed when %s is false.",
- NameStr(relform->relname),
- "publish_via_partition_root")));
-
- ReleaseSysCache(tuple);
- }
-
- ReleaseSysCache(rftuple);
- }
}
}
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 8208f9fa0e..580cc5be7f 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -588,7 +588,7 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
-- used for partitioned table
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR: cannot set publish_via_partition_root = false for publication "testpub6"
+ERROR: cannot set publish_via_partition_root to false for publication "testpub6"
DETAIL: The publication contains a WHERE clause for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
-- Now change the root filter to use a column "b"
-- (which is not in the replica identity)
@@ -951,7 +951,7 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
-- used for partitioned table
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR: cannot set publish_via_partition_root = false for publication "testpub6"
+ERROR: cannot set publish_via_partition_root to false for publication "testpub6"
DETAIL: The publication contains a column list for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
-- Now change the root column list to use a column "b"
-- (which is not in the replica identity)
