Allow OLD and NEW in multi-row VALUES within rules.

tglsfdc · tglsfdc · commit 092d7ded29f3 · 2012-08-19T14:12:16.000-04:00
Now that we have LATERAL, it's fairly painless to allow this case, which
was left as a TODO in the original multi-row VALUES implementation.
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
@@ -1221,9 +1221,7 @@ set_values_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
 
 	/*
 	 * If it's a LATERAL RTE, it might contain some Vars of the current query
-	 * level, requiring it to be treated as parameterized.  (NB: even though
-	 * the parser never marks VALUES RTEs as LATERAL, they could be so marked
-	 * by now, as a result of subquery pullup.)
+	 * level, requiring it to be treated as parameterized.
 	 */
 	if (rte->lateral)
 	{
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
@@ -587,6 +587,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
 		List	   *exprsLists = NIL;
 		List	   *collations = NIL;
 		int			sublist_length = -1;
+		bool		lateral = false;
 		int			i;
 
 		Assert(selectStmt->intoClause == NULL);
@@ -647,25 +648,20 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
 			collations = lappend_oid(collations, InvalidOid);
 
 		/*
-		 * Another thing we can't currently support is NEW/OLD references in
-		 * rules --- seems we'd need something like SQL99's LATERAL construct
-		 * to ensure that the values would be available while evaluating the
-		 * VALUES RTE.	This is a shame.  FIXME
+		 * Ordinarily there can't be any current-level Vars in the expression
+		 * lists, because the namespace was empty ... but if we're inside
+		 * CREATE RULE, then NEW/OLD references might appear.  In that case we
+		 * have to mark the VALUES RTE as LATERAL.
 		 */
 		if (list_length(pstate->p_rtable) != 1 &&
 			contain_vars_of_level((Node *) exprsLists, 0))
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("VALUES must not contain OLD or NEW references"),
-					 errhint("Use SELECT ... UNION ALL ... instead."),
-					 parser_errposition(pstate,
-							  locate_var_of_level((Node *) exprsLists, 0))));
+			lateral = true;
 
 		/*
 		 * Generate the VALUES RTE
 		 */
 		rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
-										  NULL, true);
+										  NULL, lateral, true);
 		rtr = makeNode(RangeTblRef);
 		/* assume new rte is at end */
 		rtr->rtindex = list_length(pstate->p_rtable);
@@ -1032,6 +1028,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 	List	   *collations;
 	List	  **colexprs = NULL;
 	int			sublist_length = -1;
+	bool		lateral = false;
 	RangeTblEntry *rte;
 	int			rtindex;
 	ListCell   *lc;
@@ -1176,11 +1173,21 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 		list_free(colexprs[i]);
 	}
 
+	/*
+	 * Ordinarily there can't be any current-level Vars in the expression
+	 * lists, because the namespace was empty ... but if we're inside CREATE
+	 * RULE, then NEW/OLD references might appear.  In that case we have to
+	 * mark the VALUES RTE as LATERAL.
+	 */
+	if (pstate->p_rtable != NIL &&
+		contain_vars_of_level((Node *) exprsLists, 0))
+		lateral = true;
+
 	/*
 	 * Generate the VALUES RTE
 	 */
 	rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
-									  NULL, true);
+									  NULL, lateral, true);
 	addRTEtoQuery(pstate, rte, true, true, true);
 
 	/* assume new rte is at end */
@@ -1214,21 +1221,6 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 			 errmsg("SELECT FOR UPDATE/SHARE cannot be applied to VALUES")));
 
-	/*
-	 * Another thing we can't currently support is NEW/OLD references in rules
-	 * --- seems we'd need something like SQL99's LATERAL construct to ensure
-	 * that the values would be available while evaluating the VALUES RTE.
-	 * This is a shame.  FIXME
-	 */
-	if (list_length(pstate->p_rtable) != 1 &&
-		contain_vars_of_level((Node *) exprsLists, 0))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("VALUES must not contain OLD or NEW references"),
-				 errhint("Use SELECT ... UNION ALL ... instead."),
-				 parser_errposition(pstate,
-							  locate_var_of_level((Node *) exprsLists, 0))));
-
 	qry->rtable = pstate->p_rtable;
 	qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
 
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
@@ -1313,6 +1313,7 @@ addRangeTableEntryForValues(ParseState *pstate,
 							List *exprs,
 							List *collations,
 							Alias *alias,
+							bool lateral,
 							bool inFromCl)
 {
 	RangeTblEntry *rte = makeNode(RangeTblEntry);
@@ -1355,7 +1356,7 @@ addRangeTableEntryForValues(ParseState *pstate,
 	 *
 	 * Subqueries are never checked for access rights.
 	 */
-	rte->lateral = false;
+	rte->lateral = lateral;
 	rte->inh = false;			/* never true for values RTEs */
 	rte->inFromCl = inFromCl;
 
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -2919,11 +2919,48 @@ get_select_query_def(Query *query, deparse_context *context,
 	context->windowTList = save_windowtlist;
 }
 
+/*
+ * Detect whether query looks like SELECT ... FROM VALUES();
+ * if so, return the VALUES RTE.  Otherwise return NULL.
+ */
+static RangeTblEntry *
+get_simple_values_rte(Query *query)
+{
+	RangeTblEntry *result = NULL;
+	ListCell   *lc;
+
+	/*
+	 * We want to return TRUE even if the Query also contains OLD or NEW rule
+	 * RTEs.  So the idea is to scan the rtable and see if there is only one
+	 * inFromCl RTE that is a VALUES RTE.  We don't look at the targetlist at
+	 * all.  This is okay because parser/analyze.c will never generate a
+	 * "bare" VALUES RTE --- they only appear inside auto-generated
+	 * sub-queries with very restricted structure.
+	 */
+	foreach(lc, query->rtable)
+	{
+		RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+
+		if (rte->rtekind == RTE_VALUES && rte->inFromCl)
+		{
+			if (result)
+				return NULL;	/* multiple VALUES (probably not possible) */
+			result = rte;
+		}
+		else if (rte->rtekind == RTE_RELATION && !rte->inFromCl)
+			continue;			/* ignore rule entries */
+		else
+			return NULL;		/* something else -> not simple VALUES */
+	}
+	return result;
+}
+
 static void
 get_basic_select_query(Query *query, deparse_context *context,
 					   TupleDesc resultDesc)
 {
 	StringInfo	buf = context->buf;
+	RangeTblEntry *values_rte;
 	char	   *sep;
 	ListCell   *l;
 
@@ -2936,23 +2973,13 @@ get_basic_select_query(Query *query, deparse_context *context,
 	/*
 	 * If the query looks like SELECT * FROM (VALUES ...), then print just the
 	 * VALUES part.  This reverses what transformValuesClause() did at parse
-	 * time.  If the jointree contains just a single VALUES RTE, we assume
-	 * this case applies (without looking at the targetlist...)
+	 * time.
 	 */
-	if (list_length(query->jointree->fromlist) == 1)
+	values_rte = get_simple_values_rte(query);
+	if (values_rte)
 	{
-		RangeTblRef *rtr = (RangeTblRef *) linitial(query->jointree->fromlist);
-
-		if (IsA(rtr, RangeTblRef))
-		{
-			RangeTblEntry *rte = rt_fetch(rtr->rtindex, query->rtable);
-
-			if (rte->rtekind == RTE_VALUES)
-			{
-				get_values_def(rte->values_lists, context);
-				return;
-			}
-		}
+		get_values_def(values_rte->values_lists, context);
+		return;
 	}
 
 	/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
@@ -758,7 +758,7 @@ typedef struct RangeTblEntry
 	 */
 	Alias	   *alias;			/* user-written alias clause, if any */
 	Alias	   *eref;			/* expanded reference names */
-	bool		lateral;		/* subquery or function is marked LATERAL? */
+	bool		lateral;		/* subquery, function, or values is LATERAL? */
 	bool		inh;			/* inheritance requested? */
 	bool		inFromCl;		/* present in FROM clause? */
 	AclMode		requiredPerms;	/* bitmask of required access permissions */
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
@@ -67,6 +67,7 @@ extern RangeTblEntry *addRangeTableEntryForValues(ParseState *pstate,
 							List *exprs,
 							List *collations,
 							Alias *alias,
+							bool lateral,
 							bool inFromCl);
 extern RangeTblEntry *addRangeTableEntryForJoin(ParseState *pstate,
 						  List *colnames,
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
@@ -1593,3 +1593,81 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
    WHERE sh.slunit = un.un_name;
 (1 row)
 
+--
+-- check multi-rule VALUES in rules
+--
+create table rules_src(f1 int, f2 int);
+create table rules_log(f1 int, f2 int, tag text);
+insert into rules_src values(1,2), (11,12);
+create rule r1 as on update to rules_src do also
+  insert into rules_log values(old.*, 'old'), (new.*, 'new');
+update rules_src set f2 = f2 + 1;
+update rules_src set f2 = f2 * 10;
+select * from rules_src;
+ f1 | f2  
+----+-----
+  1 |  30
+ 11 | 130
+(2 rows)
+
+select * from rules_log;
+ f1 | f2  | tag 
+----+-----+-----
+  1 |   2 | old
+  1 |   3 | new
+ 11 |  12 | old
+ 11 |  13 | new
+  1 |   3 | old
+  1 |  30 | new
+ 11 |  13 | old
+ 11 | 130 | new
+(8 rows)
+
+create rule r2 as on update to rules_src do also
+  values(old.*, 'old'), (new.*, 'new');
+update rules_src set f2 = f2 / 10;
+ column1 | column2 | column3 
+---------+---------+---------
+       1 |      30 | old
+       1 |       3 | new
+      11 |     130 | old
+      11 |      13 | new
+(4 rows)
+
+select * from rules_src;
+ f1 | f2 
+----+----
+  1 |  3
+ 11 | 13
+(2 rows)
+
+select * from rules_log;
+ f1 | f2  | tag 
+----+-----+-----
+  1 |   2 | old
+  1 |   3 | new
+ 11 |  12 | old
+ 11 |  13 | new
+  1 |   3 | old
+  1 |  30 | new
+ 11 |  13 | old
+ 11 | 130 | new
+  1 |  30 | old
+  1 |   3 | new
+ 11 | 130 | old
+ 11 |  13 | new
+(12 rows)
+
+\d+ rules_src
+                      Table "public.rules_src"
+ Column |  Type   | Modifiers | Storage | Stats target | Description 
+--------+---------+-----------+---------+--------------+-------------
+ f1     | integer |           | plain   |              | 
+ f2     | integer |           | plain   |              | 
+Rules:
+    r1 AS
+    ON UPDATE TO rules_src DO  INSERT INTO rules_log (f1, f2, tag) VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
+    r2 AS
+    ON UPDATE TO rules_src DO  VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
+Has OIDs: no
+
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
@@ -933,3 +933,23 @@ select * from only t1_2;
 select pg_get_viewdef('shoe'::regclass) as unpretty;
 select pg_get_viewdef('shoe'::regclass,true) as pretty;
 select pg_get_viewdef('shoe'::regclass,0) as prettier;
+
+--
+-- check multi-rule VALUES in rules
+--
+
+create table rules_src(f1 int, f2 int);
+create table rules_log(f1 int, f2 int, tag text);
+insert into rules_src values(1,2), (11,12);
+create rule r1 as on update to rules_src do also
+  insert into rules_log values(old.*, 'old'), (new.*, 'new');
+update rules_src set f2 = f2 + 1;
+update rules_src set f2 = f2 * 10;
+select * from rules_src;
+select * from rules_log;
+create rule r2 as on update to rules_src do also
+  values(old.*, 'old'), (new.*, 'new');
+update rules_src set f2 = f2 / 10;
+select * from rules_src;
+select * from rules_log;
+\d+ rules_src
