From b3dca4d49b44646f31ba9481441ef87ad7880dca Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Wed, 21 Oct 2026 09:53:08 +1100 Subject: [PATCH v6] Error if same table has ONLY mismatch. Added a new error: ERROR: conflicting ONLY specifications for table "%s" Patched code now rejects conflicting ONLY specifications when the same non-partitioned table is explicitly listed more than once. e.g. CREATE PUBLICATION pub FOR TABLE parent, TABLE ONLY parent; -- ERROR, conflict CREATE PUBLICATION pub FOR TABLE ONLY parent, TABLE parent; -- ERROR, conflict CREATE PUBLICATION pub FOR TABLE plain, TABLE ONLY plain; -- ERROR, conflict CREATE PUBLICATION pub FOR TABLE ONLY plain, TABLE plain; -- ERROR, conflict ~ ONLY has "no effect" for partitioned tables, so conflicting ONLY-ness of the same partitioned table is ignored, same as before: CREATE PUBLICATION pub FOR TABLE part_root, TABLE ONLY part_root; -- OK CREATE PUBLICATION pub FOR TABLE ONLY part_root, TABLE part_root; -- OK ~ Author: Peter Smith Reviewed-by: shveta malik Reviewed-by: Takeshi Ideriha Reviewed-by: kedar anavardekar Discussion: https://www.postgresql.org/message-id/flat/19bc00e8-ed58-488d-a727-5bca2445b795%40gmail.com#b78a125644c7448a4771f261366a11f9 --- src/backend/commands/publicationcmds.c | 45 ++++++++++++ src/test/regress/expected/publication.out | 84 ++++++++++++++++++++++- src/test/regress/sql/publication.sql | 48 ++++++++++++- 3 files changed, 171 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 301050104dd..2e23eb44518 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -1847,6 +1847,8 @@ OpenTableList(List *tables) ListCell *lc; List *relids_with_rf = NIL; List *relids_with_collist = NIL; + List *relids_explicit = NIL; + List *relids_recursing = NIL; /* * Open, share-lock, and check all the explicitly-specified relations @@ -1888,6 +1890,45 @@ OpenTableList(List *tables) errmsg("conflicting or redundant column lists for table \"%s\"", RelationGetRelationName(rel)))); + /* + * Disallow duplicate tables if one mention specifies ONLY and + * another does not, since it's not clear whether descendant + * tables should be included. Plain "foo, foo" (agreeing on + * ONLY-ness) remains a silently-tolerated no-op, as before. + * + * Check is just for explicitly mentioned tables to avoid falsely + * giving errors for implicitly inherited child tables. + * + * Mismatched ONLY-ness remains silently-tolerated for + * partitioned tables since ONLY has "no effect" for them anyway. + * + * About the Lists: + * + * `relids` holds every relation added to the result so far, both + * those the user named and inheritance children added implicitly + * by recursion. It is used to filter out duplicates. + * + * `relids_explicit` holds only the relations the user actually + * named (a subset of `relids`). `relids_recursing` holds those + * explicitly named relations whose first mention did not specify + * ONLY. Together they are used to detect conflicting ONLY + * specifications for the same table. + */ + if (list_member_oid(relids_explicit, myrelid) && + recurse != list_member_oid(relids_recursing, myrelid) && + rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("conflicting ONLY specifications for table \"%s\"", + RelationGetRelationName(rel)))); + + if (!list_member_oid(relids_explicit, myrelid)) + { + relids_explicit = lappend_oid(relids_explicit, myrelid); + if (recurse) + relids_recursing = lappend_oid(relids_recursing, myrelid); + } + table_close(rel, ShareUpdateExclusiveLock); continue; } @@ -1899,6 +1940,7 @@ OpenTableList(List *tables) pub_rel->except = t->except; rels = lappend(rels, pub_rel); relids = lappend_oid(relids, myrelid); + relids_explicit = lappend_oid(relids_explicit, myrelid); if (t->whereClause) relids_with_rf = lappend_oid(relids_with_rf, myrelid); @@ -1906,6 +1948,9 @@ OpenTableList(List *tables) if (t->columns) relids_with_collist = lappend_oid(relids_with_collist, myrelid); + if (recurse) + relids_recursing = lappend_oid(relids_recursing, myrelid); + /* * Add children of this rel, if requested, so that they too are added * to the publication. A partitioned table can't have any inheritance diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index b55da39fde5..299c7d21b12 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -474,16 +474,74 @@ CREATE PUBLICATION testpub7 FOR ALL TABLES EXCEPT (TABLE ONLY testpub_tbl_parent Except tables: "public.testpub_tbl_parent" +-- Specifying same table twice is OK when the mentions agree on ONLY-ness. +CREATE TABLE tbl_plain(a int); +CREATE PUBLICATION testpub_dup1 FOR TABLE + testpub_tbl_parent, testpub_tbl_parent, + testpub_tbl_child, testpub_tbl_child, + tbl_plain, tbl_plain; +\dRp+ testpub_dup1 + Publication testpub_dup1 + Owner | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description +--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+------------- + regress_publication_user | f | f | t | t | t | t | none | f | +Tables: + "public.tbl_plain" + "public.testpub_tbl_child" + "public.testpub_tbl_parent" + +CREATE PUBLICATION testpub_dup2 FOR TABLE + ONLY testpub_tbl_parent, ONLY testpub_tbl_parent, + ONLY testpub_tbl_child, ONLY testpub_tbl_child, + ONLY tbl_plain, ONLY tbl_plain; +\dRp+ testpub_dup2 + Publication testpub_dup2 + Owner | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description +--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+------------- + regress_publication_user | f | f | t | t | t | t | none | f | +Tables: + "public.tbl_plain" + "public.testpub_tbl_child" + "public.testpub_tbl_parent" + +CREATE PUBLICATION testpub_dup3 FOR ALL TABLES EXCEPT (TABLE + testpub_tbl_parent, testpub_tbl_parent, + testpub_tbl_child, testpub_tbl_child, + tbl_plain, tbl_plain); +ALTER PUBLICATION testpub_dup3 SET ALL TABLES EXCEPT (TABLE + ONLY testpub_tbl_parent, ONLY testpub_tbl_parent, + ONLY testpub_tbl_child, ONLY testpub_tbl_child, + ONLY tbl_plain, ONLY tbl_plain); +-- Otherwise, mixing ONLY and non-ONLY mentions of the same table is a +-- contradiction that should be rejected. RESET client_min_messages; -DROP TABLE testpub_tbl_parent, testpub_tbl_child; +CREATE PUBLICATION testpub_conflict1 FOR TABLE testpub_tbl_parent, ONLY testpub_tbl_parent; +ERROR: conflicting ONLY specifications for table "testpub_tbl_parent" +CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, testpub_tbl_parent; +ERROR: conflicting ONLY specifications for table "testpub_tbl_parent" +CREATE PUBLICATION testpub_conflict3 FOR TABLE testpub_tbl_child, ONLY testpub_tbl_child; +ERROR: conflicting ONLY specifications for table "testpub_tbl_child" +CREATE PUBLICATION testpub_conflict4 FOR TABLE ONLY testpub_tbl_child, testpub_tbl_child; +ERROR: conflicting ONLY specifications for table "testpub_tbl_child" +CREATE PUBLICATION testpub_conflict5 FOR TABLE tbl_plain, ONLY tbl_plain; +ERROR: conflicting ONLY specifications for table "tbl_plain" +CREATE PUBLICATION testpub_conflict6 FOR TABLE ONLY tbl_plain, tbl_plain; +ERROR: conflicting ONLY specifications for table "tbl_plain" +ALTER PUBLICATION testpub_dup3 SET ALL TABLES EXCEPT(TABLE ONLY testpub_tbl_parent, testpub_tbl_parent); +ERROR: conflicting ONLY specifications for table "testpub_tbl_parent" +SET client_min_messages = 'ERROR'; +DROP TABLE testpub_tbl_parent, testpub_tbl_child, tbl_plain; DROP PUBLICATION testpub3, testpub4, testpub5, testpub6, testpub7; +DROP PUBLICATION testpub_dup1, testpub_dup2, testpub_dup3; --------------------------------------------- -- EXCEPT clause tests for partitioned tables --------------------------------------------- SET client_min_messages = 'ERROR'; CREATE TABLE testpub_root(a int) PARTITION BY RANGE(a); CREATE TABLE testpub_part1 PARTITION OF testpub_root FOR VALUES FROM (0) TO (100); -CREATE PUBLICATION testpub8 FOR ALL TABLES EXCEPT (TABLE testpub_root); +-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently +-- tolerated because ONLY has no effect for partitioned tables anyway. +CREATE PUBLICATION testpub8 FOR ALL TABLES EXCEPT (TABLE testpub_root, ONLY testpub_root); \dRp+ testpub8; Publication testpub8 Owner | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description @@ -656,8 +714,28 @@ HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE. ALTER PUBLICATION testpub_forparted DROP TABLE testpub_parted; -- works again, because update is no longer replicated UPDATE testpub_parted2 SET a = 2; +-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently +-- tolerated because ONLY has no effect for partitioned tables anyway. +CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_parted, TABLE ONLY testpub_parted; +\dRp+ testpub_dup1 + Publication testpub_dup1 + Owner | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description +--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+------------- + regress_publication_user | f | f | t | t | t | t | none | f | +Tables: + "public.testpub_parted" + +CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_parted, TABLE testpub_parted; +\dRp+ testpub_dup2 + Publication testpub_dup2 + Owner | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description +--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+------------- + regress_publication_user | f | f | t | t | t | t | none | f | +Tables: + "public.testpub_parted" + DROP TABLE testpub_parted1, testpub_parted2; -DROP PUBLICATION testpub_forparted, testpub_forparted1; +DROP PUBLICATION testpub_forparted, testpub_forparted1, testpub_dup1, testpub_dup2; -- Tests for row filters CREATE TABLE testpub_rf_tbl1 (a integer, b text); CREATE TABLE testpub_rf_tbl2 (c text, d integer); diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index e3dbb2bc57c..8e701ff205c 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -234,9 +234,42 @@ CREATE PUBLICATION testpub6 FOR ALL TABLES EXCEPT (TABLE testpub_tbl_parent *); CREATE PUBLICATION testpub7 FOR ALL TABLES EXCEPT (TABLE ONLY testpub_tbl_parent); \dRp+ testpub7 +-- Specifying same table twice is OK when the mentions agree on ONLY-ness. +CREATE TABLE tbl_plain(a int); +CREATE PUBLICATION testpub_dup1 FOR TABLE + testpub_tbl_parent, testpub_tbl_parent, + testpub_tbl_child, testpub_tbl_child, + tbl_plain, tbl_plain; +\dRp+ testpub_dup1 +CREATE PUBLICATION testpub_dup2 FOR TABLE + ONLY testpub_tbl_parent, ONLY testpub_tbl_parent, + ONLY testpub_tbl_child, ONLY testpub_tbl_child, + ONLY tbl_plain, ONLY tbl_plain; +\dRp+ testpub_dup2 +CREATE PUBLICATION testpub_dup3 FOR ALL TABLES EXCEPT (TABLE + testpub_tbl_parent, testpub_tbl_parent, + testpub_tbl_child, testpub_tbl_child, + tbl_plain, tbl_plain); +ALTER PUBLICATION testpub_dup3 SET ALL TABLES EXCEPT (TABLE + ONLY testpub_tbl_parent, ONLY testpub_tbl_parent, + ONLY testpub_tbl_child, ONLY testpub_tbl_child, + ONLY tbl_plain, ONLY tbl_plain); + +-- Otherwise, mixing ONLY and non-ONLY mentions of the same table is a +-- contradiction that should be rejected. RESET client_min_messages; -DROP TABLE testpub_tbl_parent, testpub_tbl_child; +CREATE PUBLICATION testpub_conflict1 FOR TABLE testpub_tbl_parent, ONLY testpub_tbl_parent; +CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, testpub_tbl_parent; +CREATE PUBLICATION testpub_conflict3 FOR TABLE testpub_tbl_child, ONLY testpub_tbl_child; +CREATE PUBLICATION testpub_conflict4 FOR TABLE ONLY testpub_tbl_child, testpub_tbl_child; +CREATE PUBLICATION testpub_conflict5 FOR TABLE tbl_plain, ONLY tbl_plain; +CREATE PUBLICATION testpub_conflict6 FOR TABLE ONLY tbl_plain, tbl_plain; +ALTER PUBLICATION testpub_dup3 SET ALL TABLES EXCEPT(TABLE ONLY testpub_tbl_parent, testpub_tbl_parent); + +SET client_min_messages = 'ERROR'; +DROP TABLE testpub_tbl_parent, testpub_tbl_child, tbl_plain; DROP PUBLICATION testpub3, testpub4, testpub5, testpub6, testpub7; +DROP PUBLICATION testpub_dup1, testpub_dup2, testpub_dup3; --------------------------------------------- -- EXCEPT clause tests for partitioned tables @@ -244,7 +277,9 @@ DROP PUBLICATION testpub3, testpub4, testpub5, testpub6, testpub7; SET client_min_messages = 'ERROR'; CREATE TABLE testpub_root(a int) PARTITION BY RANGE(a); CREATE TABLE testpub_part1 PARTITION OF testpub_root FOR VALUES FROM (0) TO (100); -CREATE PUBLICATION testpub8 FOR ALL TABLES EXCEPT (TABLE testpub_root); +-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently +-- tolerated because ONLY has no effect for partitioned tables anyway. +CREATE PUBLICATION testpub8 FOR ALL TABLES EXCEPT (TABLE testpub_root, ONLY testpub_root); \dRp+ testpub8; \d testpub_part1 \d testpub_root @@ -335,8 +370,15 @@ UPDATE testpub_parted2 SET a = 2; ALTER PUBLICATION testpub_forparted DROP TABLE testpub_parted; -- works again, because update is no longer replicated UPDATE testpub_parted2 SET a = 2; +-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently +-- tolerated because ONLY has no effect for partitioned tables anyway. +CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_parted, TABLE ONLY testpub_parted; +\dRp+ testpub_dup1 +CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_parted, TABLE testpub_parted; +\dRp+ testpub_dup2 + DROP TABLE testpub_parted1, testpub_parted2; -DROP PUBLICATION testpub_forparted, testpub_forparted1; +DROP PUBLICATION testpub_forparted, testpub_forparted1, testpub_dup1, testpub_dup2; -- Tests for row filters CREATE TABLE testpub_rf_tbl1 (a integer, b text); -- 2.47.3