From 63ac94b530bae358d9a781280febe5c8cfa5334f Mon Sep 17 00:00:00 2001 From: Vigneshwaran C Date: Mon, 12 Sep 2022 11:24:54 +0530 Subject: [PATCH v1] Allow creation of publication with schema and table of the same schema. Allow creation of publication with schema and table of the same schema. --- doc/src/sgml/ref/alter_publication.sgml | 16 ++-- doc/src/sgml/ref/create_publication.sgml | 10 --- src/backend/commands/publicationcmds.c | 99 +---------------------- src/backend/commands/tablecmds.c | 27 ------- src/bin/pg_dump/t/002_pg_dump.pl | 14 ++++ src/test/regress/expected/alter_table.out | 14 +++- src/test/regress/expected/publication.out | 56 ++++++++++--- src/test/regress/sql/alter_table.sql | 3 +- src/test/regress/sql/publication.sql | 19 ++++- 9 files changed, 94 insertions(+), 164 deletions(-) diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml index d8ed89ee91..cebedc4058 100644 --- a/doc/src/sgml/ref/alter_publication.sgml +++ b/doc/src/sgml/ref/alter_publication.sgml @@ -52,9 +52,11 @@ ALTER PUBLICATION name RENAME TO ALTER SUBSCRIPTION ... REFRESH PUBLICATION action on the - subscribing side in order to become effective. Note also that the combination - of DROP with a WHERE clause is not - allowed. + subscribing side in order to become effective. Note also that + DROP ALL TABLES IN SCHEMA will not drop any schema tables + that were specified using FOR TABLE/ + ADD TABLE, and the combination of DROP + with a WHERE clause is not allowed. @@ -80,14 +82,6 @@ ALTER PUBLICATION name RENAME TO publication must be a superuser. However, a superuser can change the ownership of a publication regardless of these restrictions. - - - Adding/Setting a table that is part of schema specified in - ALL TABLES IN SCHEMA, adding/setting a schema to a - publication that already has a table that is part of the specified schema or - adding/setting a table to a publication that already has a table's schema as - part of the specified schema is not supported. - diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml index f61641896a..96758af4f7 100644 --- a/doc/src/sgml/ref/create_publication.sgml +++ b/doc/src/sgml/ref/create_publication.sgml @@ -109,11 +109,6 @@ CREATE PUBLICATION name partition are also published via publications that its ancestors are part of. - - - Specifying a table that is part of a schema specified by - FOR ALL TABLES IN SCHEMA is not supported. - @@ -135,11 +130,6 @@ CREATE PUBLICATION name the specified list of schemas, including tables created in the future. - - Specifying a schema along with a table which belongs to the specified - schema using FOR TABLE is not supported. - - Only persistent base tables and partitioned tables present in the schema will be included as part of the publication. Temporary tables, unlogged diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 8b574b86c4..e36068893d 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -66,7 +66,6 @@ typedef struct rf_context Oid parentid; /* relid of the parent relation */ } rf_context; -static List *OpenRelIdList(List *relids); static List *OpenTableList(List *tables); static void CloseTableList(List *rels); static void LockSchemaList(List *schemalist); @@ -214,44 +213,6 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate, } } -/* - * Check if any of the given relation's schema is a member of the given schema - * list. - */ -static void -CheckObjSchemaNotAlreadyInPublication(List *rels, List *schemaidlist, - PublicationObjSpecType checkobjtype) -{ - ListCell *lc; - - foreach(lc, rels) - { - PublicationRelInfo *pub_rel = (PublicationRelInfo *) lfirst(lc); - Relation rel = pub_rel->relation; - Oid relSchemaId = RelationGetNamespace(rel); - - if (list_member_oid(schemaidlist, relSchemaId)) - { - if (checkobjtype == PUBLICATIONOBJ_TABLES_IN_SCHEMA) - ereport(ERROR, - errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("cannot add schema \"%s\" to publication", - get_namespace_name(relSchemaId)), - errdetail("Table \"%s\" in schema \"%s\" is already part of the publication, adding the same schema is not supported.", - RelationGetRelationName(rel), - get_namespace_name(relSchemaId))); - else if (checkobjtype == PUBLICATIONOBJ_TABLE) - ereport(ERROR, - errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("cannot add relation \"%s.%s\" to publication", - get_namespace_name(relSchemaId), - RelationGetRelationName(rel)), - errdetail("Table's schema \"%s\" is already part of the publication or part of the specified schema list.", - get_namespace_name(relSchemaId))); - } - } -} - /* * Returns true if any of the columns used in the row filter WHERE expression is * not part of REPLICA IDENTITY, false otherwise. @@ -858,9 +819,6 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt) List *rels; rels = OpenTableList(relations); - CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist, - PUBLICATIONOBJ_TABLE); - TransformPubWhereClauses(rels, pstate->p_sourcetext, publish_via_partition_root); @@ -1110,8 +1068,7 @@ InvalidatePublicationRels(List *relids) */ static void AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup, - List *tables, List *schemaidlist, - const char *queryString) + List *tables, const char *queryString) { List *rels = NIL; Form_pg_publication pubform = (Form_pg_publication) GETSTRUCT(tup); @@ -1129,16 +1086,6 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup, if (stmt->action == AP_AddObjects) { - List *schemas = NIL; - - /* - * Check if the relation is member of the existing schema in the - * publication or member of the schema list specified. - */ - schemas = list_concat_copy(schemaidlist, GetPublicationSchemas(pubid)); - CheckObjSchemaNotAlreadyInPublication(rels, schemas, - PUBLICATIONOBJ_TABLE); - TransformPubWhereClauses(rels, queryString, pubform->pubviaroot); CheckPubRelationColumnList(rels, queryString, pubform->pubviaroot); @@ -1154,9 +1101,6 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup, List *delrels = NIL; ListCell *oldlc; - CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist, - PUBLICATIONOBJ_TABLE); - TransformPubWhereClauses(rels, queryString, pubform->pubviaroot); CheckPubRelationColumnList(rels, queryString, pubform->pubviaroot); @@ -1307,19 +1251,7 @@ AlterPublicationSchemas(AlterPublicationStmt *stmt, */ LockSchemaList(schemaidlist); if (stmt->action == AP_AddObjects) - { - List *rels; - List *reloids; - - reloids = GetPublicationRelations(pubform->oid, PUBLICATION_PART_ROOT); - rels = OpenRelIdList(reloids); - - CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist, - PUBLICATIONOBJ_TABLES_IN_SCHEMA); - - CloseTableList(rels); PublicationAddSchemas(pubform->oid, schemaidlist, false, stmt); - } else if (stmt->action == AP_DropObjects) PublicationDropSchemas(pubform->oid, schemaidlist, false); else /* AP_SetObjects */ @@ -1453,8 +1385,7 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt) errmsg("publication \"%s\" does not exist", stmt->pubname)); - AlterPublicationTables(stmt, tup, relations, schemaidlist, - pstate->p_sourcetext); + AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext); AlterPublicationSchemas(stmt, tup, schemaidlist); } @@ -1569,32 +1500,6 @@ RemovePublicationSchemaById(Oid psoid) table_close(rel, RowExclusiveLock); } -/* - * Open relations specified by a relid list. - * The returned tables are locked in ShareUpdateExclusiveLock mode in order to - * add them to a publication. - */ -static List * -OpenRelIdList(List *relids) -{ - ListCell *lc; - List *rels = NIL; - - foreach(lc, relids) - { - PublicationRelInfo *pub_rel; - Oid relid = lfirst_oid(lc); - Relation rel = table_open(relid, - ShareUpdateExclusiveLock); - - pub_rel = palloc(sizeof(PublicationRelInfo)); - pub_rel->relation = rel; - rels = lappend(rels, pub_rel); - } - - return rels; -} - /* * Open relations specified by a PublicationTable list. * The returned tables are locked in ShareUpdateExclusiveLock mode in order to diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 53b0f3a9c1..ac95e18e71 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16397,33 +16397,6 @@ AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema) newrv = makeRangeVar(stmt->newschema, RelationGetRelationName(rel), -1); nspOid = RangeVarGetAndCheckCreationNamespace(newrv, NoLock, NULL); - /* - * Check that setting the relation to a different schema won't result in a - * publication having both a schema and the same schema's table, as this - * is not supported. - */ - if (stmt->objectType == OBJECT_TABLE) - { - ListCell *lc; - List *schemaPubids = GetSchemaPublications(nspOid); - List *relPubids = GetRelationPublications(RelationGetRelid(rel)); - - foreach(lc, relPubids) - { - Oid pubid = lfirst_oid(lc); - - if (list_member_oid(schemaPubids, pubid)) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move table \"%s\" to schema \"%s\"", - RelationGetRelationName(rel), stmt->newschema), - errdetail("The schema \"%s\" and same schema's table \"%s\" cannot be part of the same publication \"%s\".", - stmt->newschema, - RelationGetRelationName(rel), - get_publication_name(pubid, false))); - } - } - /* common checks on switching namespaces */ CheckSetNamespace(oldNspOid, nspOid); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 2873b662fb..714b1321ae 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2565,6 +2565,20 @@ my %tests = ( like => { %full_runs, section_post_data => 1, }, }, + 'ALTER PUBLICATION pub3 ADD TABLE test_table' => { + create_order => 51, + create_sql => + 'ALTER PUBLICATION pub3 ADD TABLE dump_test.test_table;', + regexp => qr/^ + \QALTER PUBLICATION pub3 ADD TABLE ONLY dump_test.test_table;\E + /xm, + like => { %full_runs, section_post_data => 1, }, + unlike => { + exclude_dump_test_schema => 1, + exclude_test_table => 1, + }, + }, + 'ALTER PUBLICATION pub4 ADD TABLE test_table WHERE (col1 > 0);' => { create_order => 51, create_sql => diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index d63f4f1cba..0906c080b5 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4595,10 +4595,16 @@ create table alter1.t1 (a int); set client_min_messages = 'ERROR'; create publication pub1 for table alter1.t1, all tables in schema alter2; reset client_min_messages; -alter table alter1.t1 set schema alter2; -- should fail -ERROR: cannot move table "t1" to schema "alter2" -DETAIL: The schema "alter2" and same schema's table "t1" cannot be part of the same publication "pub1". +alter table alter1.t1 set schema alter2; +\d+ alter2.t1 + Table "alter2.t1" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + a | integer | | | | plain | | +Publications: + "pub1" + drop publication pub1; drop schema alter1 cascade; -NOTICE: drop cascades to table alter1.t1 drop schema alter2 cascade; +NOTICE: drop cascades to table alter2.t1 diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index e6e082de2f..eb0c1373c9 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -118,15 +118,42 @@ Tables from schemas: SET client_min_messages = 'ERROR'; CREATE PUBLICATION testpub_forschema FOR ALL TABLES IN SCHEMA pub_test; -RESET client_min_messages; --- fail - can't create publication with schema and table of the same schema +-- should be able to create publication with schema and table of the same +-- schema CREATE PUBLICATION testpub_for_tbl_schema FOR ALL TABLES IN SCHEMA pub_test, TABLE pub_test.testpub_nopk; -ERROR: cannot add relation "pub_test.testpub_nopk" to publication -DETAIL: Table's schema "pub_test" is already part of the publication or part of the specified schema list. --- fail - can't add a table of the same schema to the schema publication +RESET client_min_messages; +\dRp+ testpub_for_tbl_schema + Publication testpub_for_tbl_schema + Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root +--------------------------+------------+---------+---------+---------+-----------+---------- + regress_publication_user | f | t | t | t | t | f +Tables: + "pub_test.testpub_nopk" +Tables from schemas: + "pub_test" + +-- should be able to add a table of the same schema to the schema publication ALTER PUBLICATION testpub_forschema ADD TABLE pub_test.testpub_nopk; -ERROR: cannot add relation "pub_test.testpub_nopk" to publication -DETAIL: Table's schema "pub_test" is already part of the publication or part of the specified schema list. +\dRp+ testpub_forschema + Publication testpub_forschema + Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root +--------------------------+------------+---------+---------+---------+-----------+---------- + regress_publication_user | f | t | t | t | t | f +Tables: + "pub_test.testpub_nopk" +Tables from schemas: + "pub_test" + +-- should be able to drop the table +ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk; +\dRp+ testpub_forschema + Publication testpub_forschema + Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root +--------------------------+------------+---------+---------+---------+-----------+---------- + regress_publication_user | f | t | t | t | t | f +Tables from schemas: + "pub_test" + -- fail - can't drop a table from the schema publication which isn't in the -- publication ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk; @@ -166,7 +193,7 @@ Publications: (1 row) DROP TABLE testpub_tbl2; -DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema; +DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema, testpub_for_tbl_schema; CREATE TABLE testpub_tbl3 (a int); CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3); SET client_min_messages = 'ERROR'; @@ -466,10 +493,19 @@ ERROR: cannot use a WHERE clause when removing a table from a publication -- fail - cannot ALTER SET table which is a member of a pre-existing schema SET client_min_messages = 'ERROR'; CREATE PUBLICATION testpub6 FOR ALL TABLES IN SCHEMA testpub_rf_schema2; +-- should be able to set publication with schema and table of the same schema ALTER PUBLICATION testpub6 SET ALL TABLES IN SCHEMA testpub_rf_schema2, TABLE testpub_rf_schema2.testpub_rf_tbl6 WHERE (i < 99); -ERROR: cannot add relation "testpub_rf_schema2.testpub_rf_tbl6" to publication -DETAIL: Table's schema "testpub_rf_schema2" is already part of the publication or part of the specified schema list. RESET client_min_messages; +\dRp+ testpub6 + Publication testpub6 + Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root +--------------------------+------------+---------+---------+---------+-----------+---------- + regress_publication_user | f | t | t | t | t | f +Tables: + "testpub_rf_schema2.testpub_rf_tbl6" WHERE (i < 99) +Tables from schemas: + "testpub_rf_schema2" + DROP TABLE testpub_rf_tbl1; DROP TABLE testpub_rf_tbl2; DROP TABLE testpub_rf_tbl3; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index e7013f5e15..3f01fdd8a8 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3031,7 +3031,8 @@ create table alter1.t1 (a int); set client_min_messages = 'ERROR'; create publication pub1 for table alter1.t1, all tables in schema alter2; reset client_min_messages; -alter table alter1.t1 set schema alter2; -- should fail +alter table alter1.t1 set schema alter2; +\d+ alter2.t1 drop publication pub1; drop schema alter1 cascade; drop schema alter2 cascade; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index a56387edee..ff1d49ec10 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -73,11 +73,20 @@ ALTER PUBLICATION testpub_fortable SET ALL TABLES IN SCHEMA pub_test; SET client_min_messages = 'ERROR'; CREATE PUBLICATION testpub_forschema FOR ALL TABLES IN SCHEMA pub_test; -RESET client_min_messages; --- fail - can't create publication with schema and table of the same schema +-- should be able to create publication with schema and table of the same +-- schema CREATE PUBLICATION testpub_for_tbl_schema FOR ALL TABLES IN SCHEMA pub_test, TABLE pub_test.testpub_nopk; --- fail - can't add a table of the same schema to the schema publication +RESET client_min_messages; +\dRp+ testpub_for_tbl_schema + +-- should be able to add a table of the same schema to the schema publication ALTER PUBLICATION testpub_forschema ADD TABLE pub_test.testpub_nopk; +\dRp+ testpub_forschema + +-- should be able to drop the table +ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk; +\dRp+ testpub_forschema + -- fail - can't drop a table from the schema publication which isn't in the -- publication ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk; @@ -90,7 +99,7 @@ SELECT pubname, puballtables FROM pg_publication WHERE pubname = 'testpub_forall \dRp+ testpub_foralltables DROP TABLE testpub_tbl2; -DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema; +DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema, testpub_for_tbl_schema; CREATE TABLE testpub_tbl3 (a int); CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3); @@ -242,8 +251,10 @@ ALTER PUBLICATION testpub5 DROP TABLE testpub_rf_tbl1 WHERE (e < 27); -- fail - cannot ALTER SET table which is a member of a pre-existing schema SET client_min_messages = 'ERROR'; CREATE PUBLICATION testpub6 FOR ALL TABLES IN SCHEMA testpub_rf_schema2; +-- should be able to set publication with schema and table of the same schema ALTER PUBLICATION testpub6 SET ALL TABLES IN SCHEMA testpub_rf_schema2, TABLE testpub_rf_schema2.testpub_rf_tbl6 WHERE (i < 99); RESET client_min_messages; +\dRp+ testpub6 DROP TABLE testpub_rf_tbl1; DROP TABLE testpub_rf_tbl2; -- 2.32.0