From 941c3a44ead8c0046ab17745646d448b128d3f4a Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Wed, 8 Jul 2026 15:07:22 +0530 Subject: [PATCH v19 2/5] Restrict conflicting EXCEPT lists in multi-schema publications Add ProcessSchemaExceptTables() to qualify each mention's EXCEPT entries, build a sorted signature per schema OID, and compare it against any prior mention of the same schema in the same statement, erroring on a mismatch. --- src/backend/commands/publicationcmds.c | 171 ++++++++++++++++++---- src/backend/parser/gram.y | 35 +---- src/test/regress/expected/publication.out | 61 ++++++++ src/test/regress/sql/publication.sql | 44 ++++++ 4 files changed, 248 insertions(+), 63 deletions(-) diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 226ddafda93..4289b6bd802 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -62,6 +62,18 @@ typedef struct rf_context Oid parentid; /* relid of the parent relation */ } rf_context; +/* + * Records the EXCEPT-table signature already seen for a schema mentioned in + * a TABLES IN SCHEMA / TABLES IN SCHEMA CURRENT_SCHEMA list, so that a later + * mention of the same schema OID can be checked for consistency. See + * ProcessSchemaExceptTables(). + */ +typedef struct SchemaExceptSig +{ + Oid schemaid; + List *sig; /* sorted list of qualified-name strings */ +} SchemaExceptSig; + static List *OpenTableList(List *tables); static void CloseTableList(List *rels); static void LockSchemaList(List *schemalist); @@ -73,6 +85,10 @@ static void PublicationAddSchemas(Oid pubid, List *schemas, bool if_not_exists, static void PublicationDropSchemas(Oid pubid, List *schemas, bool missing_ok); static char defGetGeneratedColsOption(DefElem *def); static void CheckExceptNotInTableList(List *except_rels, List *explicitrelids); +static void ProcessSchemaExceptTables(Oid schemaid, const char *schema_name, + List *except_tables, ParseState *pstate, + List **schema_except_seen, + List **except_pubtables); static void @@ -177,6 +193,119 @@ parse_publication_options(ParseState *pstate, } } +/* + * list_sort() comparator for ProcessSchemaExceptTables()'s signature lists. + */ +static int +pub_except_sig_cmp(const ListCell *a, const ListCell *b) +{ + return strcmp((const char *) lfirst(a), (const char *) lfirst(b)); +} + +/* + * Returns true if the two (already-sorted) EXCEPT-table signature lists + * built by ProcessSchemaExceptTables() are identical. + */ +static bool +pub_except_sig_equal(List *sig1, List *sig2) +{ + ListCell *lc1; + ListCell *lc2; + + if (list_length(sig1) != list_length(sig2)) + return false; + + forboth(lc1, sig1, lc2, sig2) + { + if (strcmp((const char *) lfirst(lc1), (const char *) lfirst(lc2)) != 0) + return false; + } + + return true; +} + +/* + * Qualify unqualified EXCEPT table names with schema_name, reject entries + * explicitly qualified with a different schema, and append the (now + * qualified) tables to *except_pubtables. + * + * Also detects the case where schemaid was already mentioned earlier in the + * same TABLES IN SCHEMA list with a different EXCEPT list: since each + * mention flattens its except_tables independently, without this check two + * (or more) mentions of the same schema with differing EXCEPT lists would + * silently union together instead of being rejected as contradictory. + * *schema_except_seen accumulates the signature (sorted list of qualified + * relation names) seen so far for each schema OID in this statement. + */ +static void +ProcessSchemaExceptTables(Oid schemaid, const char *schema_name, + List *except_tables, ParseState *pstate, + List **schema_except_seen, List **except_pubtables) +{ + List *sig = NIL; + SchemaExceptSig *seen = NULL; + + foreach_ptr(PublicationObjSpec, eobj, except_tables) + { + const char *eobj_schemaname = eobj->pubtable->relation->schemaname; + const char *eobj_relname = eobj->pubtable->relation->relname; + + if (eobj_schemaname == NULL) + eobj->pubtable->relation->schemaname = pstrdup(schema_name); + else if (strcmp(eobj_schemaname, schema_name) != 0) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("table \"%s\" in EXCEPT clause does not belong to schema \"%s\"", + quote_qualified_identifier(eobj_schemaname, eobj_relname), + schema_name), + parser_errposition(pstate, eobj->location)); + + /* + * Include the ONLY-ness of this entry in its signature. EXCEPT + * (TABLE parent) excludes parent and its inheritance children, + * while EXCEPT (TABLE ONLY parent) excludes only parent itself. + * These are different exclusion sets, so two mentions of this + * schema that differ only in ONLY must not compare equal below -- + * otherwise they'd look like a harmless repeat instead of the + * genuine conflict they are. + */ + sig = lappend(sig, psprintf("%s%s", + eobj->pubtable->relation->inh ? "" : "ONLY ", + quote_qualified_identifier(NULL, eobj_relname))); + } + + list_sort(sig, pub_except_sig_cmp); + + foreach_ptr(SchemaExceptSig, entry, *schema_except_seen) + { + if (entry->schemaid == schemaid) + { + seen = entry; + break; + } + } + + if (seen) + { + if (!pub_except_sig_equal(seen->sig, sig)) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("schema \"%s\" specified with conflicting EXCEPT lists", + schema_name)); + } + else + { + SchemaExceptSig *entry = palloc_object(SchemaExceptSig); + + entry->schemaid = schemaid; + entry->sig = sig; + *schema_except_seen = lappend(*schema_except_seen, entry); + } + + foreach_ptr(PublicationObjSpec, eobj, except_tables) + *except_pubtables = lappend(*except_pubtables, eobj->pubtable); +} + /* * Convert the PublicationObjSpecType list into schema oid list and * PublicationTable list. @@ -187,6 +316,7 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate, { ListCell *cell; PublicationObjSpec *pubobj; + List *schema_except_seen = NIL; if (!pubobjspec_list) return; @@ -194,6 +324,7 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate, foreach(cell, pubobjspec_list) { Oid schemaid; + char *schema_name; List *search_path; pubobj = (PublicationObjSpec *) lfirst(cell); @@ -210,9 +341,14 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate, break; case PUBLICATIONOBJ_TABLES_IN_SCHEMA: schemaid = get_namespace_oid(pubobj->name, false); + schema_name = pubobj->name; /* Filter out duplicates if user specifies "sch1, sch1" */ *schemas = list_append_unique_oid(*schemas, schemaid); + + ProcessSchemaExceptTables(schemaid, schema_name, + pubobj->except_tables, pstate, + &schema_except_seen, except_pubtables); break; case PUBLICATIONOBJ_TABLES_IN_CUR_SCHEMA: search_path = fetch_search_path(false); @@ -223,41 +359,14 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate, schemaid = linitial_oid(search_path); list_free(search_path); + schema_name = get_namespace_name(schemaid); /* Filter out duplicates if user specifies "sch1, sch1" */ *schemas = list_append_unique_oid(*schemas, schemaid); - /* - * Qualify unqualified EXCEPT table names with the resolved - * current schema and reject any explicitly cross-schema - * entries. This mirrors the parse-time handling done for - * TABLES_IN_SCHEMA in preprocess_pubobj_list(), deferred here - * because CURRENT_SCHEMA is not known until execution time. - */ - if (pubobj->except_tables != NIL) - { - char *cur_schema_name = get_namespace_name(schemaid); - - foreach_ptr(PublicationObjSpec, eobj, pubobj->except_tables) - { - const char *eobj_schemaname = - eobj->pubtable->relation->schemaname; - const char *eobj_relname = - eobj->pubtable->relation->relname; - - if (eobj_schemaname == NULL) - eobj->pubtable->relation->schemaname = cur_schema_name; - else if (strcmp(eobj_schemaname, cur_schema_name) != 0) - ereport(ERROR, - errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("table \"%s\" in EXCEPT clause does not belong to schema \"%s\"", - quote_qualified_identifier(eobj_schemaname, eobj_relname), - cur_schema_name)); - - *except_pubtables = lappend(*except_pubtables, - eobj->pubtable); - } - } + ProcessSchemaExceptTables(schemaid, schema_name, + pubobj->except_tables, pstate, + &schema_except_seen, except_pubtables); break; default: /* shouldn't happen */ diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 6a7a1bc5f8d..df1fa3356d9 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -20789,8 +20789,9 @@ preprocess_pub_all_objtype_list(List *all_objects_list, List **pubobjects, /* * Process pubobjspec_list to check for errors in any of the objects and * convert PUBLICATIONOBJ_CONTINUATION into appropriate PublicationObjSpecType. - * Also flattens except_tables from TABLES IN SCHEMA nodes into the list so - * that ObjectsInPublicationToOids() sees them as top-level EXCEPT_TABLE entries. + * The except_tables attached to TABLES IN SCHEMA nodes are left in place; + * ObjectsInPublicationToOids() qualifies names, validates schema membership, + * and flattens them once the schema OID is known. */ static void preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner) @@ -20874,36 +20875,6 @@ preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner) errcode(ERRCODE_SYNTAX_ERROR), errmsg("invalid schema name"), parser_errposition(pubobj->location)); - - /* - * For TABLES_IN_SCHEMA, qualify unqualified EXCEPT table names - * with the parent schema and reject cross-schema entries at parse - * time, then flatten into the top-level list. - * - * For TABLES_IN_CUR_SCHEMA the schema name is not yet known, so - * skip both steps here; ObjectsInPublicationToOids() will - * qualify names and validate schema membership at execution time. - */ - if (pubobj->pubobjtype == PUBLICATIONOBJ_TABLES_IN_SCHEMA) - { - foreach_ptr(PublicationObjSpec, eobj, pubobj->except_tables) - { - const char *eobj_schemaname = eobj->pubtable->relation->schemaname; - const char *eobj_relname = eobj->pubtable->relation->relname; - - if (eobj_schemaname == NULL) - eobj->pubtable->relation->schemaname = pubobj->name; - else if (strcmp(eobj_schemaname, pubobj->name) != 0) - ereport(ERROR, - errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("table \"%s\" in EXCEPT clause does not belong to schema \"%s\"", - quote_qualified_identifier(eobj_schemaname, eobj_relname), - pubobj->name), - parser_errposition(eobj->location)); - } - pubobjspec_list = list_concat(pubobjspec_list, pubobj->except_tables); - pubobj->except_tables = NIL; - } } prevobjtype = pubobj->pubobjtype; diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 1acdcf44932..1f26cdc2966 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -543,6 +543,34 @@ Except tables: "pub_test.testpub_tbl_s1" "public.testpub_tbl1" +-- Repeating the same schema with an identical EXCEPT list is allowed, same +-- as repeating a schema with no EXCEPT list at all ("sch1, sch1") +CREATE PUBLICATION testpub_schema_except_samerepeat + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1); +\dRp+ testpub_schema_except_samerepeat + Publication testpub_schema_except_samerepeat + 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 from schemas: + "pub_test" +Except tables: + "pub_test.testpub_tbl_s1" + +DROP PUBLICATION testpub_schema_except_samerepeat; +-- fail: same schema repeated with conflicting EXCEPT lists +CREATE PUBLICATION testpub_schema_except_conflict + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s2); +ERROR: schema "pub_test" specified with conflicting EXCEPT lists +-- fail: same schema repeated where a later mention drops the EXCEPT list +-- entirely (an empty EXCEPT list still conflicts with a non-empty one) +CREATE PUBLICATION testpub_schema_except_conflict2 + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s2), + pub_test; +ERROR: schema "pub_test" specified with conflicting EXCEPT lists -- ALTER TABLE ... SET SCHEMA on a table excluded by a schema publication: -- the schema-scoped exclusion is no longer meaningful once the table moves -- out of its schema, so the exclusion is auto-removed. @@ -2111,6 +2139,8 @@ SET search_path = pub_test1; -- qualified name from wrong schema -> error CREATE PUBLICATION testpub_cursch_except FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE pub_test2.tbl1); ERROR: table "pub_test2.tbl1" in EXCEPT clause does not belong to schema "pub_test1" +LINE 1: ...FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE pub_test2.... + ^ -- unqualified name implicitly qualified with current schema (pub_test1.tbl) SET client_min_messages = 'ERROR'; CREATE PUBLICATION testpub_cursch_except FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl); @@ -2126,6 +2156,37 @@ Except tables: "pub_test1.tbl" DROP PUBLICATION testpub_cursch_except; +-- CURRENT_SCHEMA and a named schema that resolve to the same OID must be +-- treated as the same schema for EXCEPT-list consistency checking, just +-- like two mentions of the same schema name +-- fail: CURRENT_SCHEMA and pub_test1 (same schema) have conflicting EXCEPT lists +CREATE PUBLICATION testpub_cursch_named_conflict + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + pub_test1 EXCEPT (TABLE tbl1); +ERROR: schema "pub_test1" specified with conflicting EXCEPT lists +-- succeeds: CURRENT_SCHEMA and pub_test1 (same schema) share the same EXCEPT list +SET client_min_messages = 'ERROR'; +CREATE PUBLICATION testpub_cursch_named_same + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + pub_test1 EXCEPT (TABLE tbl); +RESET client_min_messages; +\dRp+ testpub_cursch_named_same + Publication testpub_cursch_named_same + 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 from schemas: + "pub_test1" +Except tables: + "pub_test1.tbl" + +DROP PUBLICATION testpub_cursch_named_same; +-- fail: two CURRENT_SCHEMA mentions (both resolving to pub_test1) with +-- conflicting EXCEPT lists +CREATE PUBLICATION testpub_cursch_cursch_conflict + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + CURRENT_SCHEMA EXCEPT (TABLE tbl1); +ERROR: schema "pub_test1" specified with conflicting EXCEPT lists RESET search_path; -- cleanup pub_test1 schema for invalidation tests ALTER PUBLICATION testpub2_forschema DROP TABLES IN SCHEMA pub_test1; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index c0d7c2f9d82..97ef5decfb4 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -260,6 +260,26 @@ CREATE PUBLICATION testpub_schema_except_multi public EXCEPT (TABLE testpub_tbl1); \dRp+ testpub_schema_except_multi +-- Repeating the same schema with an identical EXCEPT list is allowed, same +-- as repeating a schema with no EXCEPT list at all ("sch1, sch1") +CREATE PUBLICATION testpub_schema_except_samerepeat + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1); +\dRp+ testpub_schema_except_samerepeat +DROP PUBLICATION testpub_schema_except_samerepeat; + +-- fail: same schema repeated with conflicting EXCEPT lists +CREATE PUBLICATION testpub_schema_except_conflict + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s2); + +-- fail: same schema repeated where a later mention drops the EXCEPT list +-- entirely (an empty EXCEPT list still conflicts with a non-empty one) +CREATE PUBLICATION testpub_schema_except_conflict2 + FOR TABLES IN SCHEMA pub_test EXCEPT (TABLE pub_test.testpub_tbl_s1), + pub_test EXCEPT (TABLE pub_test.testpub_tbl_s2), + pub_test; + -- ALTER TABLE ... SET SCHEMA on a table excluded by a schema publication: -- the schema-scoped exclusion is no longer meaningful once the table moves -- out of its schema, so the exclusion is auto-removed. @@ -1298,6 +1318,30 @@ CREATE PUBLICATION testpub_cursch_except FOR TABLES IN SCHEMA CURRENT_SCHEMA EXC RESET client_min_messages; \dRp+ testpub_cursch_except DROP PUBLICATION testpub_cursch_except; + +-- CURRENT_SCHEMA and a named schema that resolve to the same OID must be +-- treated as the same schema for EXCEPT-list consistency checking, just +-- like two mentions of the same schema name +-- fail: CURRENT_SCHEMA and pub_test1 (same schema) have conflicting EXCEPT lists +CREATE PUBLICATION testpub_cursch_named_conflict + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + pub_test1 EXCEPT (TABLE tbl1); + +-- succeeds: CURRENT_SCHEMA and pub_test1 (same schema) share the same EXCEPT list +SET client_min_messages = 'ERROR'; +CREATE PUBLICATION testpub_cursch_named_same + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + pub_test1 EXCEPT (TABLE tbl); +RESET client_min_messages; +\dRp+ testpub_cursch_named_same +DROP PUBLICATION testpub_cursch_named_same; + +-- fail: two CURRENT_SCHEMA mentions (both resolving to pub_test1) with +-- conflicting EXCEPT lists +CREATE PUBLICATION testpub_cursch_cursch_conflict + FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE tbl), + CURRENT_SCHEMA EXCEPT (TABLE tbl1); + RESET search_path; -- cleanup pub_test1 schema for invalidation tests -- 2.50.1 (Apple Git-155)