From 9542e982b2a5bd0732c7c14674534f86e30f3dd8 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 20 Oct 2026 08:58:46 +1100 Subject: [PATCH v3] Error if same table has ONLY mismatch. Added a new error: ERROR: conflicting ONLY specifications for table "%s" Patched so any conflicting ONLY-ness of the same table gives ERROR: 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 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 | 64 ++++++++++++++++++++++- src/test/regress/sql/publication.sql | 31 ++++++++++- 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 301050104dd..9dfc965b4fb 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 explcitly 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..8fe060ca757 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -474,9 +474,49 @@ 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, TABLE testpub_tbl_parent, + TABLE testpub_tbl_child, testpub_tbl_child, + TABLE 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, TABLE ONLY testpub_tbl_parent, + TABLE ONLY testpub_tbl_child, ONLY testpub_tbl_child, + TABLE 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" + +-- 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, TABLE ONLY testpub_tbl_parent; +ERROR: conflicting ONLY specifications for table "testpub_tbl_parent" +CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, TABLE testpub_tbl_parent; +ERROR: conflicting ONLY specifications for table "testpub_tbl_parent" +CREATE PUBLICATION testpub_conflict3 FOR TABLE ONLY testpub_tbl_child, testpub_tbl_child; +ERROR: conflicting ONLY specifications for table "testpub_tbl_child" +CREATE PUBLICATION testpub_conflict4 FOR TABLE testpub_tbl_child, ONLY testpub_tbl_child; +ERROR: conflicting ONLY specifications for table "testpub_tbl_child" +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; --------------------------------------------- -- EXCEPT clause tests for partitioned tables --------------------------------------------- @@ -656,8 +696,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..e775ceb63b2 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -234,9 +234,29 @@ 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, TABLE testpub_tbl_parent, + TABLE testpub_tbl_child, testpub_tbl_child, + TABLE tbl_plain, tbl_plain; +\dRp+ testpub_dup1 +CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_tbl_parent, TABLE ONLY testpub_tbl_parent, + TABLE ONLY testpub_tbl_child, ONLY testpub_tbl_child, + TABLE ONLY tbl_plain, ONLY tbl_plain; +\dRp+ testpub_dup2 + +-- 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, TABLE ONLY testpub_tbl_parent; +CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, TABLE testpub_tbl_parent; +CREATE PUBLICATION testpub_conflict3 FOR TABLE ONLY testpub_tbl_child, testpub_tbl_child; +CREATE PUBLICATION testpub_conflict4 FOR TABLE testpub_tbl_child, ONLY testpub_tbl_child; + +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; --------------------------------------------- -- EXCEPT clause tests for partitioned tables @@ -335,8 +355,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