From b41dc0c8573df62c1dee8cea6ddc11663ac26e40 Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Mon, 30 Aug 2021 22:29:07 +0530 Subject: [PATCH v4] Invalidate relcache for publications defined for all tables. Updates/Deletes on a relation were allowed even without replica identity after we define the publication for all tables. This would later lead to conflicts on subscriber. The reason was that for such publications we were not invalidating the relcache and the publication information for relations was not getting rebuilt. Author: Vignesh C Reviewed-by: Hou Zhijie, Kyotaro Horiguchi, Amit Kapila Discussion: https://postgr.es/m/CALDaNm0pF6zeWqCA8TCe2sDuwFAy8fCqba=nHampCKag-qLixg@mail.gmail.com --- src/backend/catalog/dependency.c | 5 ++++- src/backend/commands/publicationcmds.c | 34 +++++++++++++++++++++++++++++++ src/include/commands/publicationcmds.h | 1 + src/test/regress/expected/publication.out | 15 ++++++++++++++ src/test/regress/sql/publication.sql | 14 +++++++++++++ 5 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 76b65e3..91c3e97 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1460,6 +1460,10 @@ doDeletion(const ObjectAddress *object, int flags) RemovePublicationRelById(object->objectId); break; + case OCLASS_PUBLICATION: + RemovePublicationById(object->objectId); + break; + case OCLASS_CAST: case OCLASS_COLLATION: case OCLASS_CONVERSION: @@ -1478,7 +1482,6 @@ doDeletion(const ObjectAddress *object, int flags) case OCLASS_USER_MAPPING: case OCLASS_DEFACL: case OCLASS_EVENT_TRIGGER: - case OCLASS_PUBLICATION: case OCLASS_TRANSFORM: DropObjectById(object); break; diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 8487eeb..ef1026a 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -234,6 +234,11 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt) PublicationAddTables(puboid, rels, true, NULL); CloseTableList(rels); } + else if (stmt->for_all_tables) + { + /* Invalidate relcache so that publication info is rebuilt. */ + CacheInvalidateRelcacheAll(); + } table_close(rel, RowExclusiveLock); @@ -498,6 +503,35 @@ RemovePublicationRelById(Oid proid) } /* + * Remove the publication by mapping OID. + */ +void +RemovePublicationById(Oid pubid) +{ + Relation rel; + HeapTuple tup; + Form_pg_publication pubform; + + rel = table_open(PublicationRelationId, RowExclusiveLock); + + tup = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for publication %u", pubid); + + pubform = (Form_pg_publication) GETSTRUCT(tup); + + /* Invalidate relcache so that publication info is rebuilt. */ + if (pubform->puballtables) + CacheInvalidateRelcacheAll(); + + CatalogTupleDelete(rel, &tup->t_self); + + ReleaseSysCache(tup); + + table_close(rel, RowExclusiveLock); +} + +/* * Open relations specified by a RangeVar list. * The returned tables are locked in ShareUpdateExclusiveLock mode in order to * add them to a publication. diff --git a/src/include/commands/publicationcmds.h b/src/include/commands/publicationcmds.h index a3fa2ac..c98d519 100644 --- a/src/include/commands/publicationcmds.h +++ b/src/include/commands/publicationcmds.h @@ -20,6 +20,7 @@ extern ObjectAddress CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt); extern void AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt); +extern void RemovePublicationById(Oid pubid); extern void RemovePublicationRelById(Oid proid); extern ObjectAddress AlterPublicationOwner(const char *name, Oid newOwnerId); diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 4a5ef0b..cad1b37 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -158,6 +158,21 @@ Tables: DROP TABLE testpub_parted1; DROP PUBLICATION testpub_forparted, testpub_forparted1; +-- Test cache invalidation FOR ALL TABLES publication +SET client_min_messages = 'ERROR'; +CREATE TABLE testpub_tbl4(a int); +INSERT INTO testpub_tbl4 values(1); +UPDATE testpub_tbl4 set a = 2; +CREATE PUBLICATION testpub_foralltables FOR ALL TABLES; +RESET client_min_messages; +-- fail missing REPLICA IDENTITY +UPDATE testpub_tbl4 set a = 3; +ERROR: cannot update table "testpub_tbl4" because it does not have a replica identity and publishes updates +HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE. +DROP PUBLICATION testpub_foralltables; +-- should pass after dropping the publication +UPDATE testpub_tbl4 set a = 3; +DROP TABLE testpub_tbl4; -- fail - view CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view; ERROR: cannot add relation "testpub_view" to publication diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index d844075..04b34ee 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -93,6 +93,20 @@ ALTER PUBLICATION testpub_forparted SET (publish_via_partition_root = true); DROP TABLE testpub_parted1; DROP PUBLICATION testpub_forparted, testpub_forparted1; +-- Test cache invalidation FOR ALL TABLES publication +SET client_min_messages = 'ERROR'; +CREATE TABLE testpub_tbl4(a int); +INSERT INTO testpub_tbl4 values(1); +UPDATE testpub_tbl4 set a = 2; +CREATE PUBLICATION testpub_foralltables FOR ALL TABLES; +RESET client_min_messages; +-- fail missing REPLICA IDENTITY +UPDATE testpub_tbl4 set a = 3; +DROP PUBLICATION testpub_foralltables; +-- should pass after dropping the publication +UPDATE testpub_tbl4 set a = 3; +DROP TABLE testpub_tbl4; + -- fail - view CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view; SET client_min_messages = 'ERROR'; -- 1.8.3.1