Release cache tuple when no longer needed

There was a small buglet in commit 52e4f0cd472d whereby a tuple acquired
from cache was not released, giving rise to WARNING messages; fix that.

While at it, restructure the code a bit on stylistic grounds.

Author: Hou zj <[email protected]>
Reported-by: Peter Smith <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Discussion: https://postgr.es/m/CAHut+PvKTyhTBtYCQsP6Ph7=o-oWRSX+v+PXXLXp81-o2bazig@mail.gmail.com

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 7aacb6b2fec7451b4a6a1d87a01d168bf13ec47e..6df0e6670fd59ff09ce797198effd871047537c5 100644 (file)
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -925,8 +925,9 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 
        /*
         * If the publication doesn't publish changes via the root partitioned
-        * table, the partition's row filter and column list will be used. So disallow
-        * using WHERE clause and column lists on partitioned table in this case.
+        * table, the partition's row filter and column list will be used. So
+        * disallow using WHERE clause and column lists on partitioned table in
+        * this case.
         */
        if (!pubform->puballtables && publish_via_partition_root_given &&
                !publish_via_partition_root)
@@ -945,60 +946,60 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 
                foreach(lc, root_relids)
                {
-                       HeapTuple       rftuple;
                        Oid                     relid = lfirst_oid(lc);
-                       bool            has_column_list;
-                       bool            has_row_filter;
+                       HeapTuple       rftuple;
+                       char            relkind;
+                       char       *relname;
+                       bool            has_rowfilter;
+                       bool            has_collist;
+
+                       /*
+                        * Beware: we don't have lock on the relations, so cope silently
+                        * with the cache lookups returning NULL.
+                        */
 
                        rftuple = SearchSysCache2(PUBLICATIONRELMAP,
                                                                          ObjectIdGetDatum(relid),
                                                                          ObjectIdGetDatum(pubform->oid));
-
-                       has_row_filter
-                               = !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
-
-                       has_column_list
-                               = !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
-
-                       if (HeapTupleIsValid(rftuple) &&
-                               (has_row_filter || has_column_list))
+                       if (!HeapTupleIsValid(rftuple))
+                               continue;
+                       has_rowfilter = !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
+                       has_collist = !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
+                       if (!has_rowfilter && !has_collist)
                        {
-                               HeapTuple       tuple;
-
-                               tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
-                               if (HeapTupleIsValid(tuple))
-                               {
-                                       Form_pg_class relform = (Form_pg_class) GETSTRUCT(tuple);
-
-                                       if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
-                                               has_row_filter)
-                                               ereport(ERROR,
-                                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                                                errmsg("cannot set %s for publication \"%s\"",
-                                                                               "publish_via_partition_root = false",
-                                                                               stmt->pubname),
-                                                                errdetail("The publication contains a WHERE clause for a partitioned table \"%s\" "
-                                                                                  "which is not allowed when %s is false.",
-                                                                                  NameStr(relform->relname),
-                                                                                  "publish_via_partition_root")));
-
-                                       if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
-                                               has_column_list)
-                                               ereport(ERROR,
-                                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                                                errmsg("cannot set %s for publication \"%s\"",
-                                                                               "publish_via_partition_root = false",
-                                                                               stmt->pubname),
-                                                                errdetail("The publication contains a column list for a partitioned table \"%s\" "
-                                                                                  "which is not allowed when %s is false.",
-                                                                                  NameStr(relform->relname),
-                                                                                  "publish_via_partition_root")));
-
-                                       ReleaseSysCache(tuple);
-                               }
+                               ReleaseSysCache(rftuple);
+                               continue;
+                       }
 
+                       relkind = get_rel_relkind(relid);
+                       if (relkind != RELKIND_PARTITIONED_TABLE)
+                       {
+                               ReleaseSysCache(rftuple);
+                               continue;
+                       }
+                       relname = get_rel_name(relid);
+                       if (relname == NULL)    /* table concurrently dropped */
+                       {
                                ReleaseSysCache(rftuple);
+                               continue;
                        }
+
+                       if (has_rowfilter)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
+                                                               "publish_via_partition_root",
+                                                               stmt->pubname),
+                                                errdetail("The publication contains a WHERE clause for partitioned table \"%s\", which is not allowed when \"%s\" is false.",
+                                                                  relname, "publish_via_partition_root")));
+                       Assert(has_collist);
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
+                                                       "publish_via_partition_root",
+                                                       stmt->pubname),
+                                        errdetail("The publication contains a column list for partitioned table \"%s\", which is not allowed when \"%s\" is false.",
+                                                          relname, "publish_via_partition_root")));
                }
        }
 

diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 8208f9fa0e07f026ef55b3db0ada7ec80c08ad8f..398c0f38f6c6c1b6a605c1a4d6f256da41e10cfa 100644 (file)
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -588,8 +588,12 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR:  cannot set publish_via_partition_root = false for publication "testpub6"
-DETAIL:  The publication contains a WHERE clause for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
+ERROR:  cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
+DETAIL:  The publication contains a WHERE clause for partitioned table "rf_tbl_abcd_part_pk", which is not allowed when "publish_via_partition_root" is false.
+-- remove partitioned table's row filter
+ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
+-- ok - we don't have row filter for partitioned table.
+ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
 -- Now change the root filter to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 WHERE (b > 99);
@@ -951,8 +955,12 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR:  cannot set publish_via_partition_root = false for publication "testpub6"
-DETAIL:  The publication contains a column list for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
+ERROR:  cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
+DETAIL:  The publication contains a column list for partitioned table "rf_tbl_abcd_part_pk", which is not allowed when "publish_via_partition_root" is false.
+-- remove partitioned table's column list
+ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
+-- ok - we don't have column list for partitioned table.
+ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
 -- Now change the root column list to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (b);

diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 8539110025e777771444437c373d686e967ea2b5..9eb86fd54f80f99b87ad87fae27f682b7c5130ad 100644 (file)
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -352,6 +352,10 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
+-- remove partitioned table's row filter
+ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
+-- ok - we don't have row filter for partitioned table.
+ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
 -- Now change the root filter to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 WHERE (b > 99);
@@ -635,6 +639,10 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
+-- remove partitioned table's column list
+ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
+-- ok - we don't have column list for partitioned table.
+ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
 -- Now change the root column list to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (b);