Added missing invalidations for all tables publication

Started by vignesh Cover 4 years ago11 messages

vignesh21@gmail.com

over 4 years ago

1 attachment(s)

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]/messages/by-id/CAA4eK1LtTXMqu-UbcByjHw+aKP38t4+r7kyKnmBQMA-__9U52A@mail.gmail.com. Attached patch has
the fix for the same.
Thoughts?

[1]: /messages/by-id/CAA4eK1LtTXMqu-UbcByjHw+aKP38t4+r7kyKnmBQMA-__9U52A@mail.gmail.com

Regards,
Vignesh

Attachments:

v1-0001-Added-missing-invalidations-for-all-tables-public.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Added-missing-invalidations-for-all-tables-public.patchDownload

From fd58e547c22723856a2f18c306b17ab6e59cddb1 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Mon, 30 Aug 2021 22:29:07 +0530
Subject: [PATCH v1] 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 | 13 +++++++++
 src/test/regress/sql/publication.sql      | 12 +++++++++
 5 files changed, 63 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..c8dc33f872 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -158,6 +158,19 @@ 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);
+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..3043c3ee25 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -93,6 +93,18 @@ 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);
+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

houzj.fnst@fujitsu.com

over 4 years ago

In reply to: vignesh C (#1)

RE: Added missing invalidations for all tables publication

From Tuesday, August 31, 2021 1:10 AM vignesh C <vignesh21@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 ?

Best regards,
Hou zj

vignesh C

vignesh21@gmail.com

over 4 years ago

In reply to: houzj.fnst@fujitsu.com (#2)

1 attachment(s)

Re: Added missing invalidations for all tables publication

On Tue, Aug 31, 2021 at 7:40 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

From Tuesday, August 31, 2021 1:10 AM vignesh C <vignesh21@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

Attachments:

v2-0001-Added-missing-invalidations-for-all-tables-public.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Added-missing-invalidations-for-all-tables-public.patchDownload

From 3b7fc6828efa979cbf8a89f47d3c4e4f1fb5041a Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@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

Kyotaro Horiguchi

horikyota.ntt@gmail.com

over 4 years ago

In reply to: vignesh C (#3)

Re: Added missing invalidations for all tables publication

At Tue, 31 Aug 2021 08:31:05 +0530, vignesh C <vignesh21@gmail.com> wrote in

On Tue, Aug 31, 2021 at 7:40 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
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.

The test works fine. The code looks fine for me except one minor
cosmetic flaw.

+	if (!HeapTupleIsValid(tup))
+		elog(ERROR, "cache lookup failed for publication %u",
+			 pubid);

The last two lines don't need to be separated. ((Almost) All other
instance of the same error is written that way.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

vignesh C

vignesh21@gmail.com

over 4 years ago

In reply to: Kyotaro Horiguchi (#4)

1 attachment(s)

Re: Added missing invalidations for all tables publication

On Tue, Aug 31, 2021 at 2:00 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

At Tue, 31 Aug 2021 08:31:05 +0530, vignesh C <vignesh21@gmail.com> wrote in

On Tue, Aug 31, 2021 at 7:40 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
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.

The test works fine. The code looks fine for me except one minor
cosmetic flaw.
+       if (!HeapTupleIsValid(tup))
+               elog(ERROR, "cache lookup failed for publication %u",
+                        pubid);
The last two lines don't need to be separated. ((Almost) All other
instance of the same error is written that way.

Thanks for the comments, the attached v3 patch has the changes for the same.

Regards,
Vignesh

Attachments:

v3-0001-Added-missing-invalidations-for-all-tables-public.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Added-missing-invalidations-for-all-tables-public.patchDownload

From f42b3af317f423296a66b09632474e82f9395240 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Mon, 30 Aug 2021 22:29:07 +0530
Subject: [PATCH v3] 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    | 32 +++++++++++++++++++++++
 src/include/commands/publicationcmds.h    |  1 +
 src/test/regress/expected/publication.out | 15 +++++++++++
 src/test/regress/sql/publication.sql      | 14 ++++++++++
 5 files changed, 66 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..2f0460c837 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,35 @@ 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

Amit Kapila

amit.kapila16@gmail.com

over 4 years ago

In reply to: vignesh C (#5)

1 attachment(s)

Re: Added missing invalidations for all tables publication

On Tue, Aug 31, 2021 at 8:54 PM vignesh C <vignesh21@gmail.com> wrote:

On Tue, Aug 31, 2021 at 2:00 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

Thanks for the comments, the attached v3 patch has the changes for the same.

I think this bug should be fixed in back branches (till v10). OTOH, as
this is not reported by any user and we have found it during code
review so it seems either users don't have an exact use case or they
haven't noticed this yet. What do you people think about
back-patching?

Attached, please find a slightly updated patch with minor changes. I
have slightly changed the test to make it more meaningful. If we
decide to back-patch this, can you please test this on back-branches?

--
With Regards,
Amit Kapila.

Attachments:

v4-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v4-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From b41dc0c8573df62c1dee8cea6ddc11663ac26e40 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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

houzj.fnst@fujitsu.com

over 4 years ago

In reply to: Amit Kapila (#6)

RE: Added missing invalidations for all tables publication

From Mon, Sep 6, 2021 1:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Aug 31, 2021 at 8:54 PM vignesh C <vignesh21@gmail.com> wrote:

Thanks for the comments, the attached v3 patch has the changes for the
same.

I think this bug should be fixed in back branches (till v10). OTOH, as this is not
reported by any user and we have found it during code review so it seems
either users don't have an exact use case or they haven't noticed this yet. What
do you people think about back-patching?

Personally, I think it's ok to back-patch.

Attached, please find a slightly updated patch with minor changes. I have
slightly changed the test to make it more meaningful.

The patch looks good to me.

Best regards,
Hou zj

houzj.fnst@fujitsu.com

over 4 years ago

In reply to: houzj.fnst@fujitsu.com (#7)

6 attachment(s)

RE: Added missing invalidations for all tables publication

From Mon, Sep 6, 2021 1:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Aug 31, 2021 at 8:54 PM vignesh C <vignesh21@gmail.com> wrote:

Thanks for the comments, the attached v3 patch has the changes for
the same.

I think this bug should be fixed in back branches (till v10). OTOH, as
this is not reported by any user and we have found it during code
review so it seems either users don't have an exact use case or they
haven't noticed this yet. What do you people think about back-patching?

Personally, I think it's ok to back-patch.

I found that the patch cannot be applied to back-branches(v10-v14) cleanly,
so, I generate the patches for back-branches. Attached, all the patches have
passed regression test.

Best regards,
Hou zj

Attachments:

v5-HEAD-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-HEAD-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From b41dc0c8573df62c1dee8cea6ddc11663ac26e40 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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

v5-PG10-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-PG10-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From 6968155330093c5840e35d878f1e94e29f15c80a Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 09:44:57 +0800
Subject: [PATCH] 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/commands/publicationcmds.c    | 13 ++++++++++++-
 src/test/regress/expected/publication.out | 15 +++++++++++++++
 src/test/regress/sql/publication.sql      | 14 ++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 675ee96b0f..5c744ef74e 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -224,6 +224,11 @@ CreatePublication(CreatePublicationStmt *stmt)
 		PublicationAddTables(puboid, rels, true, NULL);
 		CloseTableList(rels);
 	}
+	else if (stmt->for_all_tables)
+	{
+		/* Invalidate relcache so that publication info is rebuilt. */
+		CacheInvalidateRelcacheAll();
+	}
 
 	heap_close(rel, RowExclusiveLock);
 
@@ -438,14 +443,20 @@ RemovePublicationById(Oid pubid)
 {
 	Relation	rel;
 	HeapTuple	tup;
+	Form_pg_publication pubform;
 
 	rel = heap_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);
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index afbbdd543d..adaa5ac81c 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -107,6 +107,21 @@ Tables:
 
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
+-- 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:  "testpub_view" is not a table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 815410b3c5..1071334825 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -60,6 +60,20 @@ CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
 
+-- 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;
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
-- 
2.18.4

v5-PG11-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-PG11-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From 6968155330093c5840e35d878f1e94e29f15c80a Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 09:44:57 +0800
Subject: [PATCH] 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/commands/publicationcmds.c    | 13 ++++++++++++-
 src/test/regress/expected/publication.out | 15 +++++++++++++++
 src/test/regress/sql/publication.sql      | 14 ++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 675ee96b0f..5c744ef74e 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -224,6 +224,11 @@ CreatePublication(CreatePublicationStmt *stmt)
 		PublicationAddTables(puboid, rels, true, NULL);
 		CloseTableList(rels);
 	}
+	else if (stmt->for_all_tables)
+	{
+		/* Invalidate relcache so that publication info is rebuilt. */
+		CacheInvalidateRelcacheAll();
+	}
 
 	heap_close(rel, RowExclusiveLock);
 
@@ -438,14 +443,20 @@ RemovePublicationById(Oid pubid)
 {
 	Relation	rel;
 	HeapTuple	tup;
+	Form_pg_publication pubform;
 
 	rel = heap_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);
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index afbbdd543d..adaa5ac81c 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -107,6 +107,21 @@ Tables:
 
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
+-- 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:  "testpub_view" is not a table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 815410b3c5..1071334825 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -60,6 +60,20 @@ CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
 
+-- 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;
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
-- 
2.18.4

v5-PG12-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-PG12-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From 3e75338094799b1b35690608b18c7c5bd32e1087 Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 09:33:21 +0800
Subject: [PATCH] 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/commands/publicationcmds.c    | 15 +++++++++++++--
 src/test/regress/expected/publication.out | 15 +++++++++++++++
 src/test/regress/sql/publication.sql      | 14 ++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 1ac1a71bd9..33a66c2b59 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -227,6 +227,11 @@ CreatePublication(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);
 
@@ -440,21 +445,27 @@ AlterPublication(AlterPublicationStmt *stmt)
 }
 
 /*
- * Drop publication by OID
+ * 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);
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 0e5e8f2b92..06c7b157d6 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -107,6 +107,21 @@ Tables:
 
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
+-- 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:  "testpub_view" is not a table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 815410b3c5..1071334825 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -60,6 +60,20 @@ CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
 DROP TABLE testpub_tbl3, testpub_tbl3a;
 DROP PUBLICATION testpub3, testpub4;
 
+-- 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;
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
-- 
2.18.4

v5-PG13-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-PG13-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From 76f94148107e4dc42843001aab20dfa0d775735c Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 09:20:27 +0800
Subject: [PATCH] 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/commands/publicationcmds.c    | 15 +++++++++++++--
 src/test/regress/expected/publication.out | 15 +++++++++++++++
 src/test/regress/sql/publication.sql      | 14 ++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index a5e29b5a82..0b08cbddd0 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -236,6 +236,11 @@ CreatePublication(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);
 
@@ -469,21 +474,27 @@ AlterPublication(AlterPublicationStmt *stmt)
 }
 
 /*
- * Drop publication by OID
+ * 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);
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 63d6ab7a4e..c299702480 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -156,6 +156,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:  "testpub_view" is not a table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index d844075368..04b34ee299 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';
-- 
2.18.4

v5-PG14-0001-Invalidate-relcache-for-publications-defined-for-.patchapplication/octet-stream; name=v5-PG14-0001-Invalidate-relcache-for-publications-defined-for-.patchDownload

From 3ccfa9afb27784ba60b2f8a9d2a139f10b763d4a Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 09:03:59 +0800
Subject: [PATCH] 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 0c37fc1d53..85b2bbac02 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1470,6 +1470,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:
@@ -1488,7 +1492,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 95c253c8e0..3b022337e0 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -236,6 +236,11 @@ CreatePublication(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)
 	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 00e2e626e6..1a8fbd3d5b 100644
--- a/src/include/commands/publicationcmds.h
+++ b/src/include/commands/publicationcmds.h
@@ -20,6 +20,7 @@
 
 extern ObjectAddress CreatePublication(CreatePublicationStmt *stmt);
 extern void AlterPublication(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 63d6ab7a4e..c299702480 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -156,6 +156,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:  "testpub_view" is not a table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index d844075368..04b34ee299 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';
-- 
2.18.4

Amit Kapila

amit.kapila16@gmail.com

over 4 years ago

In reply to: houzj.fnst@fujitsu.com (#8)

Re: Added missing invalidations for all tables publication

On Wed, Sep 8, 2021 at 7:57 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

From Mon, Sep 6, 2021 1:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Aug 31, 2021 at 8:54 PM vignesh C <vignesh21@gmail.com> wrote:

Thanks for the comments, the attached v3 patch has the changes for
the same.

I think this bug should be fixed in back branches (till v10). OTOH, as
this is not reported by any user and we have found it during code
review so it seems either users don't have an exact use case or they
haven't noticed this yet. What do you people think about back-patching?

Personally, I think it's ok to back-patch.

I found that the patch cannot be applied to back-branches(v10-v14) cleanly,
so, I generate the patches for back-branches. Attached, all the patches have
passed regression test.

Pushed!

--
With Regards,
Amit Kapila.

#10

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: Amit Kapila (#9)

Re: Added missing invalidations for all tables publication

Amit Kapila <amit.kapila16@gmail.com> writes:

On Wed, Sep 8, 2021 at 7:57 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

I found that the patch cannot be applied to back-branches(v10-v14) cleanly,
so, I generate the patches for back-branches. Attached, all the patches have
passed regression test.

Pushed!

Shouldn't the CF entry for this be closed? [1]https://commitfest.postgresql.org/34/3311/

regards, tom lane

[1]: https://commitfest.postgresql.org/34/3311/

#11

Amit Kapila

amit.kapila16@gmail.com

over 4 years ago

In reply to: Tom Lane (#10)

Re: Added missing invalidations for all tables publication

On Sat, Sep 11, 2021 at 11:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Amit Kapila <amit.kapila16@gmail.com> writes:

On Wed, Sep 8, 2021 at 7:57 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:

I found that the patch cannot be applied to back-branches(v10-v14) cleanly,
so, I generate the patches for back-branches. Attached, all the patches have
passed regression test.

Pushed!

Shouldn't the CF entry for this be closed? [1]

Yes, and I have done that now.

--
With Regards,
Amit Kapila.