From d6b2169360d7951e229bb39ef53992edde70627b Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 26 Jul 2017 16:45:46 +0900 Subject: [PATCH 1/2] Fix map_partition_varattnos to sometimes accept wholerow vars It was thought that it would never encount wholerow vars in its input expressions (partition constraint expressions for example). But then it was used to convert expressions where wholerow vars are legal, such as, WCO constraint expressions and RETURNING target list members. So, add an argument to tell it whether or not to error on finding wholerows. Adds test in insert.sql and updatable_views.sql. Reported by: Rajkumar Raghuwanshi Report: https://postgr.es/m/CAKcux6%3Dz38gH4K6YAFi%2BYvo5tHTwBL4tam4VM33CAPZ5dDMk1Q%40mail.gmail.com --- src/backend/catalog/partition.c | 17 ++++++++++------- src/backend/commands/tablecmds.c | 8 +++++++- src/backend/executor/nodeModifyTable.c | 18 ++++++++++++++---- src/include/catalog/partition.h | 3 ++- src/test/regress/expected/insert.out | 10 ++++++++++ src/test/regress/expected/updatable_views.out | 10 ++++++++++ src/test/regress/sql/insert.sql | 6 ++++++ src/test/regress/sql/updatable_views.sql | 9 +++++++++ 8 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index e20ddce2db..824898939e 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -904,10 +904,10 @@ get_qual_from_partbound(Relation rel, Relation parent, */ List * map_partition_varattnos(List *expr, int target_varno, - Relation partrel, Relation parent) + Relation partrel, Relation parent, + bool *found_whole_row) { AttrNumber *part_attnos; - bool found_whole_row; if (expr == NIL) return NIL; @@ -915,14 +915,12 @@ map_partition_varattnos(List *expr, int target_varno, part_attnos = convert_tuples_by_name_map(RelationGetDescr(partrel), RelationGetDescr(parent), gettext_noop("could not convert row type")); + *found_whole_row = false; expr = (List *) map_variable_attnos((Node *) expr, target_varno, 0, part_attnos, RelationGetDescr(parent)->natts, - &found_whole_row); - /* There can never be a whole-row reference here */ - if (found_whole_row) - elog(ERROR, "unexpected whole-row reference found in partition key"); + found_whole_row); return expr; } @@ -1783,6 +1781,7 @@ generate_partition_qual(Relation rel) List *my_qual = NIL, *result = NIL; Relation parent; + bool found_whole_row; /* Guard against stack overflow due to overly deep partition tree */ check_stack_depth(); @@ -1825,7 +1824,11 @@ generate_partition_qual(Relation rel) * in it to bear this relation's attnos. It's safe to assume varno = 1 * here. */ - result = map_partition_varattnos(result, 1, rel, parent); + result = map_partition_varattnos(result, 1, rel, parent, + &found_whole_row); + /* There can never be a whole-row reference here */ + if (found_whole_row) + elog(ERROR, "unexpected whole-row reference found in partition key"); /* Save a copy in the relcache */ oldcxt = MemoryContextSwitchTo(CacheMemoryContext); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index bb00858ad1..cc5d3d6faf 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13713,6 +13713,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) Oid part_relid = lfirst_oid(lc); Relation part_rel; Expr *constr; + bool found_whole_row; /* Lock already taken */ if (part_relid != RelationGetRelid(attachRel)) @@ -13738,7 +13739,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) constr = linitial(partConstraint); tab->partition_constraint = (Expr *) map_partition_varattnos((List *) constr, 1, - part_rel, rel); + part_rel, rel, + &found_whole_row); + /* There can never be a whole-row reference here */ + if (found_whole_row) + elog(ERROR, "unexpected whole-row reference found in partition key"); + /* keep our lock until commit */ if (part_rel != attachRel) heap_close(part_rel, NoLock); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 0dde0ed6eb..21205244da 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1992,11 +1992,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) List *mapped_wcoList; List *wcoExprs = NIL; ListCell *ll; + bool found_whole_row; - /* varno = node->nominalRelation */ + /* + * We are passing node->nominalRelation as the varno. wcoList + * might contain whole-row vars which is legal. + */ mapped_wcoList = map_partition_varattnos(wcoList, node->nominalRelation, - partrel, rel); + partrel, rel, + &found_whole_row); foreach(ll, mapped_wcoList) { WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll)); @@ -2065,11 +2070,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) { Relation partrel = resultRelInfo->ri_RelationDesc; List *rlist; + bool found_whole_row; - /* varno = node->nominalRelation */ + /* + * We are passing node->nominalRelation as the varno. + * returningList might contain wholerow vars which is legal. + */ rlist = map_partition_varattnos(returningList, node->nominalRelation, - partrel, rel); + partrel, rel, + &found_whole_row); resultRelInfo->ri_projectReturning = ExecBuildProjectionInfo(rlist, econtext, slot, &mtstate->ps, resultRelInfo->ri_RelationDesc->rd_att); diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index f10879a162..434ded37d7 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -80,7 +80,8 @@ extern Oid get_partition_parent(Oid relid); extern List *get_qual_from_partbound(Relation rel, Relation parent, PartitionBoundSpec *spec); extern List *map_partition_varattnos(List *expr, int target_varno, - Relation partrel, Relation parent); + Relation partrel, Relation parent, + bool *found_whole_row); extern List *RelationGetPartitionQual(Relation rel); extern Expr *get_partition_qual_relid(Oid relid); diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 0dcc86fef4..b642de39d5 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -659,3 +659,13 @@ select tableoid::regclass, * from mcrparted order by a, b; (11 rows) drop table mcrparted; +-- check that wholerow vars in the RETUNING list work with partitioned tables +create table returningwrtest (a int) partition by list (a); +create table returningwrtest1 partition of returningwrtest for values in (1); +insert into returningwrtest values (1) returning returningwrtest; + returningwrtest +----------------- + (1) +(1 row) + +drop table returningwrtest; diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index eab5c0334c..51a21f10c2 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -2428,3 +2428,13 @@ ERROR: new row violates check option for view "ptv_wco" DETAIL: Failing row contains (1, 2, null). drop view ptv, ptv_wco; drop table pt, pt1, pt11; +-- check that wholerow vars appearing in WITH CHECK OPTION constraint expressions +-- work fine with partitioned tables +create table wcowrtest (a int) partition by list (a); +create table wcowrtest1 partition of wcowrtest for values in (1); +create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcowrtest with check option; +insert into wcowrtest_v values (1); +ERROR: new row violates check option for view "wcowrtest_v" +DETAIL: Failing row contains (1). +drop view wcowrtest_v; +drop table wcowrtest; diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 6adf25da40..2eff832e59 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -399,3 +399,9 @@ insert into mcrparted values ('aaa', 0), ('b', 0), ('bz', 10), ('c', -10), ('commons', 0), ('d', -10), ('e', 0); select tableoid::regclass, * from mcrparted order by a, b; drop table mcrparted; + +-- check that wholerow vars in the RETUNING list work with partitioned tables +create table returningwrtest (a int) partition by list (a); +create table returningwrtest1 partition of returningwrtest for values in (1); +insert into returningwrtest values (1) returning returningwrtest; +drop table returningwrtest; diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql index 2ede44c02b..af8499a019 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -1141,3 +1141,12 @@ create view ptv_wco as select * from pt where a = 0 with check option; insert into ptv_wco values (1, 2); drop view ptv, ptv_wco; drop table pt, pt1, pt11; + +-- check that wholerow vars appearing in WITH CHECK OPTION constraint expressions +-- work fine with partitioned tables +create table wcowrtest (a int) partition by list (a); +create table wcowrtest1 partition of wcowrtest for values in (1); +create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcowrtest with check option; +insert into wcowrtest_v values (1); +drop view wcowrtest_v; +drop table wcowrtest; -- 2.11.0