Apply a simple solution to the problem of making INSERT/UPDATE/DELETE

RETURNING play nice with views/rules. To wit, have the rule rewriter rewrite any RETURNING clause found in a rule to produce what the rule's triggering query asked for in its RETURNING clause, in particular drop the RETURNING clause if no RETURNING in the triggering query. This leaves the responsibility for knowing how to produce the view's output columns on the rule author, without requiring any fundamental changes in rule semantics such as adding new rule event types would do. The initial implementation constrains things to ensure that there is exactly one, unconditionally invoked RETURNING clause among the rules for an event --- later we might be able to relax that, but for a post feature freeze fix it seems better to minimize how much invention we do. Per gripe from Jaime Casanova.

Apply a simple solution to the problem of making INSERT/UPDATE/DELETE
RETURNING play nice with views/rules. To wit, have the rule rewriter rewrite any RETURNING clause found in a rule to produce what the rule's triggering query asked for in its RETURNING clause, in particular drop the RETURNING clause if no RETURNING in the triggering query. This leaves the responsibility for knowing how to produce the view's output columns on the rule author, without requiring any fundamental changes in rule semantics such as adding new rule event types would do. The initial implementation constrains things to ensure that there is exactly one, unconditionally invoked RETURNING clause among the rules for an event --- later we might be able to relax that, but for a post feature freeze fix it seems better to minimize how much invention we do. Per gripe from Jaime Casanova.
917bbebf · Tom Lane · 74924d29 · 917bbebf · 917bbebf · 917bbebf
Commit 917bbebf authored Sep 02, 2006 by Tom Lane
6 changed files
--- a/doc/src/sgml/ref/create_rule.sgml
+++ b/doc/src/sgml/ref/create_rule.sgml
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.45 2005/01/04 00:39:53 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.46 2006/09/02 17:06:52 tgl Exp $
 PostgreSQL documentation
 -->
@@ -70,7 +70,9 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
   <literal>ON INSERT</literal>, <literal>ON UPDATE</literal>, and
   <literal>ON DELETE</literal> rules (or any subset of those that's
   sufficient for your purposes) to replace update actions on the view
-   with appropriate updates on other tables.
+   with appropriate updates on other tables.  If you want to support
+   <command>INSERT RETURNING</> and so on, then be sure to put a suitable
+   <literal>RETURNING</> clause into each of these rules.
  </para>
  <para>
@@ -87,7 +89,8 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
   understands it will never be called on to update the dummy table.
   Then make the conditional rules non-<literal>INSTEAD</literal>; in
   the cases where they are applied, they add to the default
-   <literal>INSTEAD NOTHING</literal> action.
+   <literal>INSTEAD NOTHING</literal> action.  (This method does not
+   currently work to support <literal>RETURNING</> queries, however.)
  </para>
 </refsect1>
@@ -201,13 +204,30 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
   be allowed to define a rule on it.
  </para>
+  <para>
+   In a rule for <literal>INSERT</literal>, <literal>UPDATE</literal>, or
+   <literal>DELETE</literal> on a view, you can add a <literal>RETURNING</>
+   clause that emits the view's columns.  This clause will be used to compute
+   the outputs if the rule is triggered by an <command>INSERT RETURNING</>,
+   <command>UPDATE RETURNING</>, or <command>DELETE RETURNING</> command
+   respectively.  When the rule is triggered by a command without
+   <literal>RETURNING</>, the rule's <literal>RETURNING</> clause will be
+   ignored.  The current implementation allows only unconditional
+   <literal>INSTEAD</> rules to contain <literal>RETURNING</>; furthermore
+   there can be at most one <literal>RETURNING</> clause among all the rules
+   for the same event.  (This ensures that there is only one candidate
+   <literal>RETURNING</> clause to be used to compute the results.)
+   <literal>RETURNING</> queries on the view will be rejected if
+   there is no <literal>RETURNING</> clause in any available rule.
+  </para>
  <para>
   It is very important to take care to avoid circular rules.  For
   example, though each of the following two rule definitions are
   accepted by <productname>PostgreSQL</productname>, the
   <command>SELECT</command> command would cause
   <productname>PostgreSQL</productname> to report an error because
-   the query cycled too many times:
+   of recursive expansion of a rule:
 <programlisting>
 CREATE RULE "_RETURN" AS

--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
-<!-- $PostgreSQL: pgsql/doc/src/sgml/rules.sgml,v 1.45 2006/04/23 03:39:52 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/rules.sgml,v 1.46 2006/09/02 17:06:52 tgl Exp $ -->
 <chapter id="rules">
 <title>The Rule System</title>
@@ -873,7 +873,7 @@ SELECT t1.a, t2.b, t1.ctid FROM t1, t2 WHERE t1.a = t2.a;
        <listitem>
        <para>
-            They can be <literal>INSTEAD</> or <literal>ALSO</> (default).
+            They can be <literal>INSTEAD</> or <literal>ALSO</> (the default).
        </para>
        </listitem>
@@ -920,7 +920,8 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
    Initially the query-tree list is empty.
    There can be zero (<literal>NOTHING</> key word), one, or multiple actions.
    To simplify, we will look at a rule with one action. This rule
-    can have a qualification or not and it can be <literal>INSTEAD</> or <literal>ALSO</> (default).
+    can have a qualification or not and it can be <literal>INSTEAD</> or
+    <literal>ALSO</> (the default).
 </para>
 <para>
@@ -932,22 +933,13 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
 </para>
   <para>
-    So we have four cases that produce the following query trees for
+    So we have three cases that produce the following query trees for
    a one-action rule.
    <variablelist>
     <varlistentry>
-      <term>No qualification and <literal>ALSO</></term>
+      <term>No qualification, with either <literal>ALSO</> or
-      <listitem>
+      <literal>INSTEAD</></term>
-       <para>
-        the query tree from the rule action with the original query
-        tree's qualification added
-       </para>
-      </listitem>
-     </varlistentry>
-     <varlistentry>
-      <term>No qualification but <literal>INSTEAD</></term>
      <listitem>
       <para>
        the query tree from the rule action with the original query
@@ -1283,7 +1275,7 @@ SELECT shoelace_data.sl_name, 0,
    A simple way to protect view relations from the mentioned
    possibility that someone can try to run <command>INSERT</command>,
    <command>UPDATE</command>, or <command>DELETE</command> on them is
-    to let those query trees get thrown away.  So we create the rules
+    to let those query trees get thrown away.  So we could create the rules
 <programlisting>
 CREATE RULE shoe_ins_protect AS ON INSERT TO shoe
@@ -1338,6 +1330,34 @@ CREATE RULE shoelace_del AS ON DELETE TO shoelace
 </programlisting>
   </para>
+   <para>
+    If you want to support <literal>RETURNING</> queries on the view,
+    you need to make the rules include <literal>RETURNING</> clauses that
+    compute the view rows.  This is usually pretty trivial for views on a
+    single table, but it's a bit tedious for join views such as
+    <literal>shoelace</literal>.  An example for the insert case is
+<programlisting>
+CREATE RULE shoelace_ins AS ON INSERT TO shoelace
+    DO INSTEAD
+    INSERT INTO shoelace_data VALUES (
+           NEW.sl_name,
+           NEW.sl_avail,
+           NEW.sl_color,
+           NEW.sl_len,
+           NEW.sl_unit
+    )
+    RETURNING
+           shoelace_data.*,
+           (SELECT shoelace_data.sl_len * u.un_fact
+            FROM unit u WHERE shoelace_data.sl_unit = u.un_name);
+</programlisting>
+    Note that this one rule supports both <command>INSERT</> and
+    <command>INSERT RETURNING</> queries on the view &mdash; the
+    <literal>RETURNING</> clause is simply ignored for <command>INSERT</>.
+   </para>
   <para>
    Now assume that once in a while, a pack of shoelaces arrives at
    the shop and a big parts list along with it.  But you don't want

--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -8,7 +8,7 @@
 *
 *
 * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.112 2006/08/12 20:05:55 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.113 2006/09/02 17:06:52 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -31,6 +31,8 @@
 #include "utils/syscache.h"
+static void checkRuleResultList(List *targetList, TupleDesc resultDesc,
+								bool isSelect);
 static void setRuleCheckAsUser_Query(Query *qry, Oid userid);
 static void setRuleCheckAsUser_Expr(Node *node, Oid userid);
 static bool setRuleCheckAsUser_walker(Node *node, Oid *context);
@@ -235,15 +237,11 @@ DefineQueryRewrite(RuleStmt *stmt)
 					 errhint("Use triggers instead.")));
 	}
-	/*
-	 * Rules ON SELECT are restricted to view definitions
-	 */
 	if (event_type == CMD_SELECT)
 	{
-		ListCell   *tllist;
-		int			i;
 		/*
+		 * Rules ON SELECT are restricted to view definitions
+		 *
 		 * So there cannot be INSTEAD NOTHING, ...
 		 */
 		if (list_length(action) == 0)
@@ -282,71 +280,17 @@ DefineQueryRewrite(RuleStmt *stmt)
 		 * ... the targetlist of the SELECT action must exactly match the
 		 * event relation, ...
 		 */
-		i = 0;
+		checkRuleResultList(query->targetList,
-		foreach(tllist, query->targetList)
+							RelationGetDescr(event_relation),
-		{
+							true);
-			TargetEntry *tle = (TargetEntry *) lfirst(tllist);
-			int32		tletypmod;
-			Form_pg_attribute attr;
-			char	   *attname;
-			if (tle->resjunk)
-				continue;
-			i++;
-			if (i > event_relation->rd_att->natts)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				  errmsg("SELECT rule's target list has too many entries")));
-			attr = event_relation->rd_att->attrs[i - 1];
-			attname = NameStr(attr->attname);
-			/*
-			 * Disallow dropped columns in the relation.  This won't happen in
-			 * the cases we actually care about (namely creating a view via
-			 * CREATE TABLE then CREATE RULE).	Trying to cope with it is much
-			 * more trouble than it's worth, because we'd have to modify the
-			 * rule to insert dummy NULLs at the right positions.
-			 */
-			if (attr->attisdropped)
-				ereport(ERROR,
-						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("cannot convert relation containing dropped columns to view")));
-			if (strcmp(tle->resname, attname) != 0)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-						 errmsg("SELECT rule's target entry %d has different column name from \"%s\"", i, attname)));
-			if (attr->atttypid != exprType((Node *) tle->expr))
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-						 errmsg("SELECT rule's target entry %d has different type from column \"%s\"", i, attname)));
-			/*
-			 * Allow typmods to be different only if one of them is -1, ie,
-			 * "unspecified".  This is necessary for cases like "numeric",
-			 * where the table will have a filled-in default length but the
-			 * select rule's expression will probably have typmod = -1.
-			 */
-			tletypmod = exprTypmod((Node *) tle->expr);
-			if (attr->atttypmod != tletypmod &&
-				attr->atttypmod != -1 && tletypmod != -1)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-						 errmsg("SELECT rule's target entry %d has different size from column \"%s\"", i, attname)));
-		}
-		if (i != event_relation->rd_att->natts)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				   errmsg("SELECT rule's target list has too few entries")));
 		/*
 		 * ... there must not be another ON SELECT rule already ...
 		 */
 		if (!replace && event_relation->rd_rules != NULL)
 		{
+			int		i;
 			for (i = 0; i < event_relation->rd_rules->numLocks; i++)
 			{
 				RewriteRule *rule;
@@ -425,6 +369,42 @@ DefineQueryRewrite(RuleStmt *stmt)
 			RelisBecomingView = true;
 		}
 	}
+	else
+	{
+		/*
+		 * For non-SELECT rules, a RETURNING list can appear in at most one
+		 * of the actions ... and there can't be any RETURNING list at all
+		 * in a conditional or non-INSTEAD rule.  (Actually, there can be
+		 * at most one RETURNING list across all rules on the same event,
+		 * but it seems best to enforce that at rule expansion time.)  If
+		 * there is a RETURNING list, it must match the event relation.
+		 */
+		bool	haveReturning = false;
+		foreach(l, action)
+		{
+			query = (Query *) lfirst(l);
+			if (!query->returningList)
+				continue;
+			if (haveReturning)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("cannot have multiple RETURNING lists in a rule")));
+			haveReturning = true;
+			if (event_qual != NULL)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("RETURNING lists are not supported in conditional rules")));
+			if (!is_instead)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("RETURNING lists are not supported in non-INSTEAD rules")));
+			checkRuleResultList(query->returningList,
+								RelationGetDescr(event_relation),
+								false);
+		}
+	}
 	/*
 	 * This rule is allowed - prepare to install it.
@@ -484,6 +464,95 @@ DefineQueryRewrite(RuleStmt *stmt)
 	heap_close(event_relation, NoLock);
 }
+/*
+ * checkRuleResultList
+ *		Verify that targetList produces output compatible with a tupledesc
+ *
+ * The targetList might be either a SELECT targetlist, or a RETURNING list;
+ * isSelect tells which.  (This is mostly used for choosing error messages,
+ * but also we don't enforce column name matching for RETURNING.)
+ */
+static void
+checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect)
+{
+	ListCell   *tllist;
+	int			i;
+	i = 0;
+	foreach(tllist, targetList)
+	{
+		TargetEntry *tle = (TargetEntry *) lfirst(tllist);
+		int32		tletypmod;
+		Form_pg_attribute attr;
+		char	   *attname;
+		/* resjunk entries may be ignored */
+		if (tle->resjunk)
+			continue;
+		i++;
+		if (i > resultDesc->natts)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+					 isSelect ?
+					 errmsg("SELECT rule's target list has too many entries") :
+					 errmsg("RETURNING list has too many entries")));
+		attr = resultDesc->attrs[i - 1];
+		attname = NameStr(attr->attname);
+		/*
+		 * Disallow dropped columns in the relation.  This won't happen in the
+		 * cases we actually care about (namely creating a view via CREATE
+		 * TABLE then CREATE RULE, or adding a RETURNING rule to a view).
+		 * Trying to cope with it is much more trouble than it's worth,
+		 * because we'd have to modify the rule to insert dummy NULLs at the
+		 * right positions.
+		 */
+		if (attr->attisdropped)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot convert relation containing dropped columns to view")));
+		if (isSelect && strcmp(tle->resname, attname) != 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+					 errmsg("SELECT rule's target entry %d has different column name from \"%s\"", i, attname)));
+		if (attr->atttypid != exprType((Node *) tle->expr))
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+					 isSelect ?
+					 errmsg("SELECT rule's target entry %d has different type from column \"%s\"",
+							i, attname) :
+					 errmsg("RETURNING list's entry %d has different type from column \"%s\"",
+							i, attname)));
+		/*
+		 * Allow typmods to be different only if one of them is -1, ie,
+		 * "unspecified".  This is necessary for cases like "numeric",
+		 * where the table will have a filled-in default length but the
+		 * select rule's expression will probably have typmod = -1.
+		 */
+		tletypmod = exprTypmod((Node *) tle->expr);
+		if (attr->atttypmod != tletypmod &&
+			attr->atttypmod != -1 && tletypmod != -1)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+					 isSelect ?
+					 errmsg("SELECT rule's target entry %d has different size from column \"%s\"",
+							i, attname) :
+					 errmsg("RETURNING list's entry %d has different size from column \"%s\"",
+							i, attname)));
+	}
+	if (i != resultDesc->natts)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 isSelect ?
+				 errmsg("SELECT rule's target list has too few entries") :
+				 errmsg("RETURNING list has too few entries")));
+}
 /*
 * setRuleCheckAsUser_Query
 *		Recursively scan a query and set the checkAsUser field to the

--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -7,7 +7,7 @@
 * Portions Copyright (c) 1994, Regents of the University of California
 *
 * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.165 2006/08/02 01:59:47 joe Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.166 2006/09/02 17:06:52 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -39,7 +39,8 @@ static Query *rewriteRuleAction(Query *parsetree,
 				  Query *rule_action,
 				  Node *rule_qual,
 				  int rt_index,
-				  CmdType event);
+				  CmdType event,
+				  bool *returning_flag);
 static List *adjustJoinTreeList(Query *parsetree, bool removert, int rt_index);
 static void rewriteTargetList(Query *parsetree, Relation target_relation,
 							  List **attrno_list);
@@ -251,13 +252,26 @@ acquireLocksOnSubLinks(Node *node, void *context)
 * rewriteRuleAction -
 *	  Rewrite the rule action with appropriate qualifiers (taken from
 *	  the triggering query).
+ *
+ * Input arguments:
+ *	parsetree - original query
+ *	rule_action - one action (query) of a rule
+ *	rule_qual - WHERE condition of rule, or NULL if unconditional
+ *	rt_index - RT index of result relation in original query
+ *	event - type of rule event
+ * Output arguments:
+ *	*returning_flag - set TRUE if we rewrite RETURNING clause in rule_action
+ *					(must be initialized to FALSE)
+ * Return value:
+ *	rewritten form of rule_action
 */
 static Query *
 rewriteRuleAction(Query *parsetree,
 				  Query *rule_action,
 				  Node *rule_qual,
 				  int rt_index,
-				  CmdType event)
+				  CmdType event,
+				  bool *returning_flag)
 {
 	int			current_varno,
 				new_varno;
@@ -416,6 +430,32 @@ rewriteRuleAction(Query *parsetree,
 			rule_action = sub_action;
 	}
+	/*
+	 * If rule_action has a RETURNING clause, then either throw it away
+	 * if the triggering query has no RETURNING clause, or rewrite it to
+	 * emit what the triggering query's RETURNING clause asks for.  Throw
+	 * an error if more than one rule has a RETURNING clause.
+	 */
+	if (!parsetree->returningList)
+		rule_action->returningList = NIL;
+	else if (rule_action->returningList)
+	{
+		if (*returning_flag)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot have RETURNING lists in multiple rules")));
+		*returning_flag = true;
+		rule_action->returningList = (List *)
+			ResolveNew((Node *) parsetree->returningList,
+					   parsetree->resultRelation,
+					   0,
+					   rt_fetch(parsetree->resultRelation,
+								parsetree->rtable),
+					   rule_action->returningList,
+					   CMD_SELECT,
+					   0);
+	}
 	return rule_action;
 }
@@ -1357,6 +1397,8 @@ CopyAndAddInvertedQual(Query *parsetree,
 * Output arguments:
 *	*instead_flag - set TRUE if any unqualified INSTEAD rule is found
 *					(must be initialized to FALSE)
+ *	*returning_flag - set TRUE if we rewrite RETURNING clause in any rule
+ *					(must be initialized to FALSE)
 *	*qual_product - filled with modified original query if any qualified
 *					INSTEAD rule is found (must be initialized to NULL)
 * Return value:
@@ -1377,6 +1419,7 @@ fireRules(Query *parsetree,
 		  CmdType event,
 		  List *locks,
 		  bool *instead_flag,
+		  bool *returning_flag,
 		  Query **qual_product)
 {
 	List	   *results = NIL;
@@ -1438,7 +1481,8 @@ fireRules(Query *parsetree,
 				continue;
 			rule_action = rewriteRuleAction(parsetree, rule_action,
-											event_qual, rt_index, event);
+											event_qual, rt_index, event,
+											returning_flag);
 			rule_action->querySource = qsrc;
 			rule_action->canSetTag = false;		/* might change later */
@@ -1463,6 +1507,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 {
 	CmdType		event = parsetree->commandType;
 	bool		instead = false;
+	bool		returning = false;
 	Query	   *qual_product = NULL;
 	List	   *rewritten = NIL;
@@ -1551,6 +1596,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 										event,
 										locks,
 										&instead,
+										&returning,
 										&qual_product);
 			/*
@@ -1591,6 +1637,47 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 			}
 		}
+		/*
+		 * If there is an INSTEAD, and the original query has a RETURNING,
+		 * we have to have found a RETURNING in the rule(s), else fail.
+		 * (Because DefineQueryRewrite only allows RETURNING in unconditional
+		 * INSTEAD rules, there's no need to worry whether the substituted
+		 * RETURNING will actually be executed --- it must be.)
+		 */
+		if ((instead || qual_product != NULL) &&
+			parsetree->returningList &&
+			!returning)
+		{
+			switch (event)
+			{
+				case CMD_INSERT:
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("cannot INSERT RETURNING on relation \"%s\"",
+									RelationGetRelationName(rt_entry_relation)),
+							 errhint("You need an unconditional ON INSERT DO INSTEAD rule with a RETURNING clause.")));
+					break;
+				case CMD_UPDATE:
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("cannot UPDATE RETURNING on relation \"%s\"",
+									RelationGetRelationName(rt_entry_relation)),
+							 errhint("You need an unconditional ON UPDATE DO INSTEAD rule with a RETURNING clause.")));
+					break;
+				case CMD_DELETE:
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("cannot DELETE RETURNING on relation \"%s\"",
+									RelationGetRelationName(rt_entry_relation)),
+							 errhint("You need an unconditional ON DELETE DO INSTEAD rule with a RETURNING clause.")));
+					break;
+				default:
+					elog(ERROR, "unrecognized commandType: %d",
+						 (int) event);
+					break;
+			}
+		}
 		heap_close(rt_entry_relation, NoLock);
 	}

--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -192,4 +192,143 @@ SELECT * FROM foochild;
 ----+----+----+----+----
 (0 rows)
-DROP TABLE foochild, foo;
+DROP TABLE foochild;
+-- Rules and views
+CREATE TEMP VIEW voo AS SELECT f1, f2 FROM foo;
+CREATE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57);
+INSERT INTO voo VALUES(11,'zit');
+-- fails:
+INSERT INTO voo VALUES(12,'zoo') RETURNING *, f1*2;
+ERROR:  cannot INSERT RETURNING on relation "voo"
+HINT:  You need an unconditional ON INSERT DO INSTEAD rule with a RETURNING clause.
+-- fails, incompatible list:
+CREATE OR REPLACE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57) RETURNING *;
+ERROR:  RETURNING list has too many entries
+CREATE OR REPLACE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57) RETURNING f1, f2;
+-- should still work
+INSERT INTO voo VALUES(13,'zit2');
+-- works now
+INSERT INTO voo VALUES(14,'zoo2') RETURNING *;
+ f1 |  f2  
+----+------
+ 14 | zoo2
+(1 row)
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4  
+----+------+----+-----
+  2 | more | 42 | 141
+ 11 | zit  | 57 |  99
+ 13 | zit2 | 57 |  99
+ 14 | zoo2 | 57 |  99
+(4 rows)
+SELECT * FROM voo;
+ f1 |  f2  
+----+------
+  2 | more
+ 11 | zit
+ 13 | zit2
+ 14 | zoo2
+(4 rows)
+CREATE OR REPLACE RULE voo_u AS ON UPDATE TO voo DO INSTEAD
+  UPDATE foo SET f1 = new.f1, f2 = new.f2 WHERE f1 = old.f1
+  RETURNING f1, f2;
+update voo set f1 = f1 + 1 where f2 = 'zoo2';
+update voo set f1 = f1 + 1 where f2 = 'zoo2' RETURNING *, f1*2;
+ f1 |  f2  | ?column? 
+----+------+----------
+ 16 | zoo2 |       32
+(1 row)
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4  
+----+------+----+-----
+  2 | more | 42 | 141
+ 11 | zit  | 57 |  99
+ 13 | zit2 | 57 |  99
+ 16 | zoo2 | 57 |  99
+(4 rows)
+SELECT * FROM voo;
+ f1 |  f2  
+----+------
+  2 | more
+ 11 | zit
+ 13 | zit2
+ 16 | zoo2
+(4 rows)
+CREATE OR REPLACE RULE voo_d AS ON DELETE TO voo DO INSTEAD
+  DELETE FROM foo WHERE f1 = old.f1
+  RETURNING f1, f2;
+DELETE FROM foo WHERE f1 = 13;
+DELETE FROM foo WHERE f2 = 'zit' RETURNING *;
+ f1 | f2  | f3 | f4 
+----+-----+----+----
+ 11 | zit | 57 | 99
+(1 row)
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4  
+----+------+----+-----
+  2 | more | 42 | 141
+ 16 | zoo2 | 57 |  99
+(2 rows)
+SELECT * FROM voo;
+ f1 |  f2  
+----+------
+  2 | more
+ 16 | zoo2
+(2 rows)
+-- Try a join case
+CREATE TEMP TABLE joinme (f2j text, other int);
+INSERT INTO joinme VALUES('more', 12345);
+INSERT INTO joinme VALUES('zoo2', 54321);
+INSERT INTO joinme VALUES('other', 0);
+CREATE TEMP VIEW joinview AS
+  SELECT foo.*, other FROM foo JOIN joinme ON (f2 = f2j);
+SELECT * FROM joinview;
+ f1 |  f2  | f3 | f4  | other 
+----+------+----+-----+-------
+  2 | more | 42 | 141 | 12345
+ 16 | zoo2 | 57 |  99 | 54321
+(2 rows)
+CREATE RULE joinview_u AS ON UPDATE TO joinview DO INSTEAD
+  UPDATE foo SET f1 = new.f1, f3 = new.f3
+    FROM joinme WHERE f2 = f2j AND f2 = old.f2
+    RETURNING foo.*, other;
+UPDATE joinview SET f1 = f1 + 1 WHERE f3 = 57 RETURNING *, other + 1;
+ f1 |  f2  | f3 | f4 | other | ?column? 
+----+------+----+----+-------+----------
+ 17 | zoo2 | 57 | 99 | 54321 |    54322
+(1 row)
+SELECT * FROM joinview;
+ f1 |  f2  | f3 | f4  | other 
+----+------+----+-----+-------
+  2 | more | 42 | 141 | 12345
+ 17 | zoo2 | 57 |  99 | 54321
+(2 rows)
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4  
+----+------+----+-----
+  2 | more | 42 | 141
+ 17 | zoo2 | 57 |  99
+(2 rows)
+SELECT * FROM voo;
+ f1 |  f2  
+----+------
+  2 | more
+ 17 | zoo2
+(2 rows)
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -84,4 +84,73 @@ DELETE FROM foo
 SELECT * FROM foo;
 SELECT * FROM foochild;
-DROP TABLE foochild, foo;
+DROP TABLE foochild;
+-- Rules and views
+CREATE TEMP VIEW voo AS SELECT f1, f2 FROM foo;
+CREATE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57);
+INSERT INTO voo VALUES(11,'zit');
+-- fails:
+INSERT INTO voo VALUES(12,'zoo') RETURNING *, f1*2;
+-- fails, incompatible list:
+CREATE OR REPLACE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57) RETURNING *;
+CREATE OR REPLACE RULE voo_i AS ON INSERT TO voo DO INSTEAD
+  INSERT INTO foo VALUES(new.*, 57) RETURNING f1, f2;
+-- should still work
+INSERT INTO voo VALUES(13,'zit2');
+-- works now
+INSERT INTO voo VALUES(14,'zoo2') RETURNING *;
+SELECT * FROM foo;
+SELECT * FROM voo;
+CREATE OR REPLACE RULE voo_u AS ON UPDATE TO voo DO INSTEAD
+  UPDATE foo SET f1 = new.f1, f2 = new.f2 WHERE f1 = old.f1
+  RETURNING f1, f2;
+update voo set f1 = f1 + 1 where f2 = 'zoo2';
+update voo set f1 = f1 + 1 where f2 = 'zoo2' RETURNING *, f1*2;
+SELECT * FROM foo;
+SELECT * FROM voo;
+CREATE OR REPLACE RULE voo_d AS ON DELETE TO voo DO INSTEAD
+  DELETE FROM foo WHERE f1 = old.f1
+  RETURNING f1, f2;
+DELETE FROM foo WHERE f1 = 13;
+DELETE FROM foo WHERE f2 = 'zit' RETURNING *;
+SELECT * FROM foo;
+SELECT * FROM voo;
+-- Try a join case
+CREATE TEMP TABLE joinme (f2j text, other int);
+INSERT INTO joinme VALUES('more', 12345);
+INSERT INTO joinme VALUES('zoo2', 54321);
+INSERT INTO joinme VALUES('other', 0);
+CREATE TEMP VIEW joinview AS
+  SELECT foo.*, other FROM foo JOIN joinme ON (f2 = f2j);
+SELECT * FROM joinview;
+CREATE RULE joinview_u AS ON UPDATE TO joinview DO INSTEAD
+  UPDATE foo SET f1 = new.f1, f3 = new.f3
+    FROM joinme WHERE f2 = f2j AND f2 = old.f2
+    RETURNING foo.*, other;
+UPDATE joinview SET f1 = f1 + 1 WHERE f3 = 57 RETURNING *, other + 1;
+SELECT * FROM joinview;
+SELECT * FROM foo;
+SELECT * FROM voo;