0001-Fix-map_partition_varattnos-to-not-error-on-found_wh-v3.patch

text/plain

Filename: 0001-Fix-map_partition_varattnos-to-not-error-on-found_wh-v3.patch
Type: text/plain
Part: 0
Message: Re: map_partition_varattnos() and whole-row vars

Patch

Format: format-patch
Series: patch v3-0001
Subject: Fix map_partition_varattnos to not error on found_whole_row
File+
src/backend/catalog/partition.c 14 6
src/backend/commands/tablecmds.c 7 1
src/backend/executor/nodeModifyTable.c 14 4
src/include/catalog/partition.h 2 1
src/test/regress/expected/insert.out 10 0
src/test/regress/expected/updatable_views.out 10 0
src/test/regress/sql/insert.sql 6 0
src/test/regress/sql/updatable_views.sql 9 0
From 9b2d16ec4c8eadd7261849d5aa0f34ee2577b405 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 26 Jul 2017 16:45:46 +0900
Subject: [PATCH 1/2] Fix map_partition_varattnos to not error on
 found_whole_row

It was designed assuming that the expressions passed to it can never
contain whole-row vars, but it's wrong since it's called from places
that pass it WCO constraint expressions and RETURNING target list
expressions, which can very well contain whole-row vars.

Move the responsibility of error'ing out to the callers, because they
are in position to know that finding whole-row vars is an error
condition.

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               | 20 ++++++++++++++------
 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, 72 insertions(+), 12 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 9d50efb6a0..80ca928a9c 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -898,16 +898,20 @@ get_qual_from_partbound(Relation rel, Relation parent,
  * We must allow for cases where physical attnos of a partition can be
  * different from the parent's.
  *
+ * If found_whole_row is not NULL, *found_whole_row returns whether a
+ * whole-row variable was found in the input expression.
+ *
  * Note: this will work on any node tree, so really the argument and result
  * should be declared "Node *".  But a substantial majority of the callers
  * are working on Lists, so it's less messy to do the casts internally.
  */
 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;
+	bool		my_found_whole_row;
 
 	if (expr == NIL)
 		return NIL;
@@ -919,10 +923,9 @@ map_partition_varattnos(List *expr, int target_varno,
 										target_varno, 0,
 										part_attnos,
 										RelationGetDescr(parent)->natts,
-										&found_whole_row);
-	/* There can never be a whole-row reference here */
+										&my_found_whole_row);
 	if (found_whole_row)
-		elog(ERROR, "unexpected whole-row reference found in partition key");
+		*found_whole_row = my_found_whole_row;
 
 	return expr;
 }
@@ -1783,6 +1786,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 +1829,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..c60a9f7952 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1993,10 +1993,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			List	   *wcoExprs = NIL;
 			ListCell   *ll;
 
-			/* varno = node->nominalRelation */
+			/*
+			 * We are passing node->nominalRelation as the varno to match
+			 * Vars to be changed.  We don't care to know whether wcoList
+			 * contains a whole-row Var, so passing NULL for the last
+			 * argument.
+			 */
 			mapped_wcoList = map_partition_varattnos(wcoList,
 													 node->nominalRelation,
-													 partrel, rel);
+													 partrel, rel, NULL);
 			foreach(ll, mapped_wcoList)
 			{
 				WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
@@ -2066,10 +2071,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			Relation	partrel = resultRelInfo->ri_RelationDesc;
 			List	   *rlist;
 
-			/* varno = node->nominalRelation */
+			/*
+			 * We are passing node->nominalRelation as the varno to match
+			 * Vars to be changed.  We don't care to know whether
+			 * returningList contains a whole-row Var, so passing NULL for
+			 * the last argument.
+			 */
 			rlist = map_partition_varattnos(returningList,
 											node->nominalRelation,
-											partrel, rel);
+											partrel, rel, NULL);
 			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..da08062b4a 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 RETURNING 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..de7d54ee5e 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 RETURNING 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