Skip to content

Commit ed0fbc8

Browse files
alvherrealvherre
committed
Release cache tuple when no longer needed
There was a small buglet in commit 52e4f0c 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 <houzj.fnst@fujitsu.com> Reported-by: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAHut+PvKTyhTBtYCQsP6Ph7=o-oWRSX+v+PXXLXp81-o2bazig@mail.gmail.com
1 parent 112fdb3 commit ed0fbc8

File tree

3 files changed

+68
-51
lines changed

3 files changed

+68
-51
lines changed

‎src/backend/commands/publicationcmds.c

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -925,8 +925,9 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
925925

926926
/*
927927
* If the publication doesn't publish changes via the root partitioned
928-
* table, the partition's row filter and column list will be used. So disallow
929-
* using WHERE clause and column lists on partitioned table in this case.
928+
* table, the partition's row filter and column list will be used. So
929+
* disallow using WHERE clause and column lists on partitioned table in
930+
* this case.
930931
*/
931932
if (!pubform->puballtables && publish_via_partition_root_given &&
932933
!publish_via_partition_root)
@@ -945,60 +946,60 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
945946

946947
foreach(lc, root_relids)
947948
{
948-
HeapTuple rftuple;
949949
Oid relid = lfirst_oid(lc);
950-
bool has_column_list;
951-
bool has_row_filter;
950+
HeapTuple rftuple;
951+
char relkind;
952+
char *relname;
953+
bool has_rowfilter;
954+
bool has_collist;
955+
956+
/*
957+
* Beware: we don't have lock on the relations, so cope silently
958+
* with the cache lookups returning NULL.
959+
*/
952960

953961
rftuple = SearchSysCache2(PUBLICATIONRELMAP,
954962
ObjectIdGetDatum(relid),
955963
ObjectIdGetDatum(pubform->oid));
956-
957-
has_row_filter
958-
= !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
959-
960-
has_column_list
961-
= !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
962-
963-
if (HeapTupleIsValid(rftuple) &&
964-
(has_row_filter || has_column_list))
964+
if (!HeapTupleIsValid(rftuple))
965+
continue;
966+
has_rowfilter = !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
967+
has_collist = !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
968+
if (!has_rowfilter && !has_collist)
965969
{
966-
HeapTuple tuple;
967-
968-
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
969-
if (HeapTupleIsValid(tuple))
970-
{
971-
Form_pg_class relform = (Form_pg_class) GETSTRUCT(tuple);
972-
973-
if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
974-
has_row_filter)
975-
ereport(ERROR,
976-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
977-
errmsg("cannot set %s for publication \"%s\"",
978-
"publish_via_partition_root = false",
979-
stmt->pubname),
980-
errdetail("The publication contains a WHERE clause for a partitioned table \"%s\" "
981-
"which is not allowed when %s is false.",
982-
NameStr(relform->relname),
983-
"publish_via_partition_root")));
984-
985-
if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
986-
has_column_list)
987-
ereport(ERROR,
988-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
989-
errmsg("cannot set %s for publication \"%s\"",
990-
"publish_via_partition_root = false",
991-
stmt->pubname),
992-
errdetail("The publication contains a column list for a partitioned table \"%s\" "
993-
"which is not allowed when %s is false.",
994-
NameStr(relform->relname),
995-
"publish_via_partition_root")));
996-
997-
ReleaseSysCache(tuple);
998-
}
970+
ReleaseSysCache(rftuple);
971+
continue;
972+
}
999973

974+
relkind = get_rel_relkind(relid);
975+
if (relkind != RELKIND_PARTITIONED_TABLE)
976+
{
977+
ReleaseSysCache(rftuple);
978+
continue;
979+
}
980+
relname = get_rel_name(relid);
981+
if (relname == NULL) /* table concurrently dropped */
982+
{
1000983
ReleaseSysCache(rftuple);
984+
continue;
1001985
}
986+
987+
if (has_rowfilter)
988+
ereport(ERROR,
989+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
990+
errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
991+
"publish_via_partition_root",
992+
stmt->pubname),
993+
errdetail("The publication contains a WHERE clause for partitioned table \"%s\", which is not allowed when \"%s\" is false.",
994+
relname, "publish_via_partition_root")));
995+
Assert(has_collist);
996+
ereport(ERROR,
997+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
998+
errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
999+
"publish_via_partition_root",
1000+
stmt->pubname),
1001+
errdetail("The publication contains a column list for partitioned table \"%s\", which is not allowed when \"%s\" is false.",
1002+
relname, "publish_via_partition_root")));
10021003
}
10031004
}
10041005

‎src/test/regress/expected/publication.out

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,12 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
588588
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
589589
-- used for partitioned table
590590
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
591-
ERROR: cannot set publish_via_partition_root = false for publication "testpub6"
592-
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.
591+
ERROR: cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
592+
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.
593+
-- remove partitioned table's row filter
594+
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
595+
-- ok - we don't have row filter for partitioned table.
596+
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
593597
-- Now change the root filter to use a column "b"
594598
-- (which is not in the replica identity)
595599
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;
951955
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
952956
-- used for partitioned table
953957
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
954-
ERROR: cannot set publish_via_partition_root = false for publication "testpub6"
955-
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.
958+
ERROR: cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
959+
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.
960+
-- remove partitioned table's column list
961+
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
962+
-- ok - we don't have column list for partitioned table.
963+
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
956964
-- Now change the root column list to use a column "b"
957965
-- (which is not in the replica identity)
958966
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (b);

‎src/test/regress/sql/publication.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,10 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
352352
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
353353
-- used for partitioned table
354354
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
355+
-- remove partitioned table's row filter
356+
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
357+
-- ok - we don't have row filter for partitioned table.
358+
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
355359
-- Now change the root filter to use a column "b"
356360
-- (which is not in the replica identity)
357361
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;
635639
-- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
636640
-- used for partitioned table
637641
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
642+
-- remove partitioned table's column list
643+
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk;
644+
-- ok - we don't have column list for partitioned table.
645+
ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
638646
-- Now change the root column list to use a column "b"
639647
-- (which is not in the replica identity)
640648
ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (b);

0 commit comments

Comments
 (0)