0002-Teach-map_variable_attnos_mutator-to-convert-whole-r-v2.patch

text/plain

Filename: 0002-Teach-map_variable_attnos_mutator-to-convert-whole-r-v2.patch
Type: text/plain
Part: 1
Message: Re: map_partition_varattnos() and whole-row vars

Patch

Format: format-patch
Series: patch v2-0002
Subject: Teach map_variable_attnos_mutator to convert whole-row Vars
File+
src/backend/catalog/partition.c 4 1
src/backend/commands/tablecmds.c 2 2
src/backend/executor/nodeModifyTable.c 2 8
src/backend/parser/parse_utilcmd.c 3 3
src/backend/rewrite/rewriteManip.c 38 12
src/include/rewrite/rewriteManip.h 1 1
src/include/utils/rel.h 7 0
src/test/regress/expected/insert.out 10 0
src/test/regress/expected/updatable_views.out 18 2
src/test/regress/sql/insert.sql 6 0
src/test/regress/sql/updatable_views.sql 20 2
From f2703383d3ddff50b4325f5686011b20a486aa02 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Mon, 31 Jul 2017 17:48:07 +0900
Subject: [PATCH 2/2] Teach map_variable_attnos_mutator to convert whole-row
 Vars

Partitioning code that uses it requires it.
---
 src/backend/catalog/partition.c               |  5 ++-
 src/backend/commands/tablecmds.c              |  4 +--
 src/backend/executor/nodeModifyTable.c        | 10 ++----
 src/backend/parser/parse_utilcmd.c            |  6 ++--
 src/backend/rewrite/rewriteManip.c            | 50 ++++++++++++++++++++-------
 src/include/rewrite/rewriteManip.h            |  2 +-
 src/include/utils/rel.h                       |  7 ++++
 src/test/regress/expected/insert.out          | 10 ++++++
 src/test/regress/expected/updatable_views.out | 20 +++++++++--
 src/test/regress/sql/insert.sql               |  6 ++++
 src/test/regress/sql/updatable_views.sql      | 22 ++++++++++--
 11 files changed, 111 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 5891af9cf2..889d9635bc 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -909,6 +909,7 @@ map_partition_varattnos(List *expr, int target_varno,
 {
 	AttrNumber *part_attnos;
 	bool		my_found_whole_row;
+	Oid		to_rowtype;
 
 	if (expr == NIL)
 		return NIL;
@@ -916,11 +917,13 @@ 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"));
+
+	to_rowtype = RelationGetRelType(partrel);
 	expr = (List *) map_variable_attnos((Node *) expr,
 										target_varno, 0,
 										part_attnos,
 										RelationGetDescr(parent)->natts,
-										&my_found_whole_row);
+										&my_found_whole_row, to_rowtype);
 	if (found_whole_row)
 		*found_whole_row = my_found_whole_row;
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cc5d3d6faf..86cefe4696 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1989,7 +1989,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 				expr = map_variable_attnos(stringToNode(check[i].ccbin),
 										   1, 0,
 										   newattno, tupleDesc->natts,
-										   &found_whole_row);
+										   &found_whole_row, InvalidOid);
 
 				/*
 				 * For the moment we have to reject whole-row variables. We
@@ -8874,7 +8874,7 @@ ATPrepAlterColumnType(List **wqueue,
 					map_variable_attnos(def->cooked_default,
 										1, 0,
 										attmap, RelationGetDescr(rel)->natts,
-										&found_whole_row);
+										&found_whole_row, InvalidOid);
 				if (found_whole_row)
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 361241a6ce..435aed3b8b 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1993,10 +1993,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			List	   *wcoExprs = NIL;
 			ListCell   *ll;
 
-			/*
-			 * We are passing node->nominalRelation as the varno.  wcoList
-			 * might contain whole-row vars which is legal.
-			 */
+			/* varno = node->nominalRelation */
 			mapped_wcoList = map_partition_varattnos(wcoList,
 													 node->nominalRelation,
 													 partrel, rel, NULL);
@@ -2069,10 +2066,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			Relation	partrel = resultRelInfo->ri_RelationDesc;
 			List	   *rlist;
 
-			/*
-			 * We are passing node->nominalRelation as the varno.
-			 * returningList might contain wholerow vars which is legal.
-			 */
+			/* varno = node->nominalRelation */
 			rlist = map_partition_varattnos(returningList,
 											node->nominalRelation,
 											partrel, rel, NULL);
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 9f37f1b920..f2f0ea540b 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1107,7 +1107,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 			ccbin_node = map_variable_attnos(stringToNode(ccbin),
 											 1, 0,
 											 attmap, tupleDesc->natts,
-											 &found_whole_row);
+											 &found_whole_row, InvalidOid);
 
 			/*
 			 * We reject whole-row variables because the whole point of LIKE
@@ -1463,7 +1463,7 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
 			indexkey = map_variable_attnos(indexkey,
 										   1, 0,
 										   attmap, attmap_length,
-										   &found_whole_row);
+										   &found_whole_row, InvalidOid);
 
 			/* As in transformTableLikeClause, reject whole-row variables */
 			if (found_whole_row)
@@ -1539,7 +1539,7 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
 		pred_tree = map_variable_attnos(pred_tree,
 										1, 0,
 										attmap, attmap_length,
-										&found_whole_row);
+										&found_whole_row, InvalidOid);
 
 		/* As in transformTableLikeClause, reject whole-row variables */
 		if (found_whole_row)
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index b89b435da0..bd3129b7cd 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1202,15 +1202,18 @@ replace_rte_variables_mutator(Node *node,
  * A zero in the mapping array represents a dropped column, which should not
  * appear in the expression.
  *
- * If the expression tree contains a whole-row Var for the target RTE,
- * the Var is not changed but *found_whole_row is returned as TRUE.
- * For most callers this is an error condition, but we leave it to the caller
- * to report the error so that useful context can be provided.  (In some
- * usages it would be appropriate to modify the Var's vartype and insert a
- * ConvertRowtypeExpr node to map back to the original vartype.  We might
- * someday extend this function's API to support that.  For now, the only
- * concession to that future need is that this function is a tree mutator
- * not just a walker.)
+ * Depending on the caller, whole-row Vars in the expression tree are
+ * converted if the necessary information is provided (see below.)  If the
+ * caller cannot provide the information, it might mean that finding a
+ * whole-row Var in the first place is an error.  In that case, we let the
+ * caller know that a whole-row Var was found by returning *found_whole_row
+ * as TRUE, which the caller then can report as an error by providing useful
+ * context information.
+ *
+ * When the information necessary to convert whole-row Vars is present, we
+ * substitute the Var's vartype (replace by to_rowtype) and add a
+ * ConvertRowtypeExpr node to map back to the original vartype.  Currently,
+ * only map_partition_varattnos() requests conversion of whole-row Vars.
  *
  * This could be built using replace_rte_variables and a callback function,
  * but since we don't ever need to insert sublinks, replace_rte_variables is
@@ -1224,6 +1227,8 @@ typedef struct
 	const AttrNumber *attno_map;	/* map array for user attnos */
 	int			map_length;		/* number of entries in attno_map[] */
 	bool	   *found_whole_row;	/* output flag */
+	/* Target type when converting whole-row vars */
+	Oid			to_rowtype;
 } map_variable_attnos_context;
 
 static Node *
@@ -1240,10 +1245,9 @@ map_variable_attnos_mutator(Node *node,
 			var->varlevelsup == context->sublevels_up)
 		{
 			/* Found a matching variable, make the substitution */
-			Var		   *newvar = (Var *) palloc(sizeof(Var));
+			Var		   *newvar = copyObject(var);
 			int			attno = var->varattno;
 
-			*newvar = *var;
 			if (attno > 0)
 			{
 				/* user-defined column, replace attno */
@@ -1257,6 +1261,27 @@ map_variable_attnos_mutator(Node *node,
 			{
 				/* whole-row variable, warn caller */
 				*(context->found_whole_row) = true;
+
+				/* If the callers expects us to convert the same, do so. */
+				if (OidIsValid(context->to_rowtype))
+				{
+					ConvertRowtypeExpr *r;
+
+					/* Var itself is converted to the requested rowtype. */
+					newvar->vartype = context->to_rowtype;
+
+					/*
+					 * And a conversion step on top to convert back to the
+					 * original type.
+					 */
+					r = makeNode(ConvertRowtypeExpr);
+					r->arg = (Expr *) newvar;
+					r->resulttype = var->vartype;
+					r->convertformat = COERCE_IMPLICIT_CAST;
+					r->location = -1;
+
+					return (Node *) r;
+				}
 			}
 			return (Node *) newvar;
 		}
@@ -1283,7 +1308,7 @@ Node *
 map_variable_attnos(Node *node,
 					int target_varno, int sublevels_up,
 					const AttrNumber *attno_map, int map_length,
-					bool *found_whole_row)
+					bool *found_whole_row, Oid to_rowtype)
 {
 	map_variable_attnos_context context;
 
@@ -1291,6 +1316,7 @@ map_variable_attnos(Node *node,
 	context.sublevels_up = sublevels_up;
 	context.attno_map = attno_map;
 	context.map_length = map_length;
+	context.to_rowtype = to_rowtype;
 	context.found_whole_row = found_whole_row;
 
 	*found_whole_row = false;
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 282ff9967f..56f9bb8013 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -72,7 +72,7 @@ extern Node *replace_rte_variables_mutator(Node *node,
 extern Node *map_variable_attnos(Node *node,
 					int target_varno, int sublevels_up,
 					const AttrNumber *attno_map, int map_length,
-					bool *found_whole_row);
+					bool *found_whole_row, Oid to_rowtype);
 
 extern Node *ReplaceVarsFromTargetList(Node *node,
 						  int target_varno, int sublevels_up,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4bc61e5380..bd5428f72f 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -444,6 +444,13 @@ typedef struct ViewOptions
 	((relation)->rd_rel->relnamespace)
 
 /*
+ * RelationGetRelType
+ *		Returns the rel's pg_type OID.
+ */
+#define RelationGetRelType(relation) \
+	((relation)->rd_rel->reltype)
+
+/*
  * RelationIsMapped
  *		True if the relation uses the relfilenode map.
  *
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b642de39d5..38b364d10f 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -668,4 +668,14 @@ insert into returningwrtest values (1) returning returningwrtest;
  (1)
 (1 row)
 
+alter table returningwrtest add b text;
+create table returningwrtest2 (b text, c int, a int);
+alter table returningwrtest2 drop c;
+alter table returningwrtest attach partition returningwrtest2 for values in (2);
+insert into returningwrtest values (2, 'foo') returning returningwrtest;
+ returningwrtest 
+-----------------
+ (2,foo)
+(1 row)
+
 drop table returningwrtest;
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 51a21f10c2..2090a411fe 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2436,5 +2436,21 @@ create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcow
 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;
+alter table wcowrtest add b text;
+create table wcowrtest2 (b text, c int, a int);
+alter table wcowrtest2 drop c;
+alter table wcowrtest attach partition wcowrtest2 for values in (2);
+create table sometable (a int, b text);
+insert into sometable values (1, 'a'), (2, 'b');
+create view wcowrtest_v2 as
+    select *
+      from wcowrtest r
+      where r in (select s from sometable s where r.a = s.a)
+with check option;
+-- WITH CHECK qual will be processed with wcowrtest2's
+-- rowtype after tuple-routing
+insert into wcowrtest_v2 values (2, 'no such row in sometable');
+ERROR:  new row violates check option for view "wcowrtest_v2"
+DETAIL:  Failing row contains (2, no such row in sometable).
+drop view wcowrtest_v, wcowrtest_v2;
+drop table wcowrtest, sometable;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 2eff832e59..f51d72484d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -404,4 +404,10 @@ drop table mcrparted;
 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;
+
+alter table returningwrtest add b text;
+create table returningwrtest2 (b text, c int, a int);
+alter table returningwrtest2 drop c;
+alter table returningwrtest attach partition returningwrtest2 for values in (2);
+insert into returningwrtest values (2, 'foo') returning returningwrtest;
 drop table returningwrtest;
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index af8499a019..a6ba5aad9e 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1148,5 +1148,23 @@ 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;
+
+alter table wcowrtest add b text;
+create table wcowrtest2 (b text, c int, a int);
+alter table wcowrtest2 drop c;
+alter table wcowrtest attach partition wcowrtest2 for values in (2);
+
+create table sometable (a int, b text);
+insert into sometable values (1, 'a'), (2, 'b');
+create view wcowrtest_v2 as
+    select *
+      from wcowrtest r
+      where r in (select s from sometable s where r.a = s.a)
+with check option;
+
+-- WITH CHECK qual will be processed with wcowrtest2's
+-- rowtype after tuple-routing
+insert into wcowrtest_v2 values (2, 'no such row in sometable');
+
+drop view wcowrtest_v, wcowrtest_v2;
+drop table wcowrtest, sometable;
-- 
2.11.0