From: Alvaro Herrera Date: Wed, 13 Apr 2022 16:19:38 +0000 (+0200) Subject: Release cache tuple when no longer needed X-Git-Tag: REL_15_BETA1~170 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ed0fbc8e5ac995eada933250c1d5535336442b97;p=thirdparty%2Fpostgresql.git 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 Reported-by: Peter Smith Reviewed-by: Amit Kapila 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 7aacb6b2fec..6df0e6670fd 100644 --- 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 8208f9fa0e0..398c0f38f6c 100644 --- 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 8539110025e..9eb86fd54f8 100644 --- 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);