On Tue, Aug 31, 2021 at 7:40 AM houzj.f...@fujitsu.com
<houzj.f...@fujitsu.com> wrote:
>
> From Tuesday, August 31, 2021 1:10 AM vignesh C <vignes...@gmail.com> wrote:
> > Hi,
> >
> > Relation invalidation was missing in case of create publication and drop
> > publication of "FOR ALL TABLES" publication, added so that the publication
> > information can be rebuilt. Without these invalidation update/delete
> > operations on the relation will be successful in the publisher which will
> > later
> > result in conflict in the subscriber.
> > Thanks to Amit for identifying the issue at [1]. Attached patch has the fix
> > for the
> > same.
> > Thoughts?
>
> I have one comment about the testcase in the patch.
>
> +-- Test cache invalidation FOR ALL TABLES publication
> +SET client_min_messages = 'ERROR';
> +CREATE TABLE testpub_tbl4(a int);
> +CREATE PUBLICATION testpub_foralltables FOR ALL TABLES;
> +RESET client_min_messages;
> +-- fail missing REPLICA IDENTITY
> +UPDATE testpub_tbl4 set a = 2;
> +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;
>
> The above testcases can pass without the code change in the patch, is it
> better
> to add a testcase which can show different results after applying the patch ?
Thanks for the comment, I have slightly modified the test case which
will fail without the patch. Attached v2 patch which has the changes
for the same.
Regards,
Vignesh
From 3b7fc6828efa979cbf8a89f47d3c4e4f1fb5041a Mon Sep 17 00:00:00 2001
From: Vignesh C <vignes...@gmail.com>
Date: Mon, 30 Aug 2021 22:29:07 +0530
Subject: [PATCH v2] Added missing invalidations for all tables publication.
Relation invalidation was missing in case of create publication and drop
publication of "FOR ALL TABLES" publication, added them so that the
publication information can be rebuilt.
---
src/backend/catalog/dependency.c | 5 +++-
src/backend/commands/publicationcmds.c | 33 +++++++++++++++++++++++
src/include/commands/publicationcmds.h | 1 +
src/test/regress/expected/publication.out | 15 +++++++++++
src/test/regress/sql/publication.sql | 14 ++++++++++
5 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 76b65e39c4..91c3e976e0 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 8487eeb7e6..54eb0b20b4 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -234,6 +234,9 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
PublicationAddTables(puboid, rels, true, NULL);
CloseTableList(rels);
}
+ /* Invalidate relcache so that publication info is rebuilt. */
+ else if (stmt->for_all_tables)
+ CacheInvalidateRelcacheAll();
table_close(rel, RowExclusiveLock);
@@ -497,6 +500,36 @@ RemovePublicationRelById(Oid proid)
table_close(rel, RowExclusiveLock);
}
+/*
+ * 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
diff --git a/src/include/commands/publicationcmds.h b/src/include/commands/publicationcmds.h
index a3fa2ac6cd..c98d519b29 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 4a5ef0bc24..366719e961 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 = 2;
+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 = 2;
+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 d844075368..285696aaf1 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 = 2;
+DROP PUBLICATION testpub_foralltables;
+-- should pass after dropping the publication
+UPDATE testpub_tbl4 set a = 2;
+DROP TABLE testpub_tbl4;
+
-- fail - view
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
SET client_min_messages = 'ERROR';
--
2.30.2
