Fix rule rewriter so that new ordering of ON INSERT actions applies

tglsfdc · tglsfdc · commit cb052e0bf139 · 2001-07-09T23:50:32.000Z
in cases of qualified rules as well as unqualified ones.  Tweak rules
test to avoid cluttering output with dummy SELECT results.  Update
documentation to match code.
diff --git a/doc/src/sgml/ref/create_rule.sgml b/doc/src/sgml/ref/create_rule.sgml
@@ -1,5 +1,5 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.23 2001/05/27 09:59:27 petere Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/ref/create_rule.sgml,v 1.24 2001/07/09 23:50:31 tgl Exp $
 Postgres documentation
 -->
 
@@ -167,7 +167,8 @@ CREATE
    <replaceable class="parameter">action</replaceable> part of the rule is
    executed.  The <replaceable class="parameter">action</replaceable> is
    done instead of the original query if INSTEAD is specified; otherwise
-   it is done before the original query is performed.
+   it is done after the original query in the case of ON INSERT, or before
+   the original query in the case of ON UPDATE or ON DELETE.
    Within both the <replaceable class="parameter">condition</replaceable>
    and <replaceable class="parameter">action</replaceable>, values from
    fields in the old instance and/or the new instance are substituted for
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
@@ -1,3 +1,5 @@
+<!-- $Header: /cvsroot/pgsql/doc/src/sgml/rules.sgml,v 1.14 2001/07/09 23:50:32 tgl Exp $ -->
+
 <Chapter Id="rules">
 <Title>The <ProductName>Postgres</ProductName> Rule System</Title>
 
@@ -1014,6 +1016,16 @@
     for a rule with one action.
 </Para>
 
+<Para>
+    For ON INSERT rules, the original query (if not suppressed by INSTEAD)
+    is done before any actions added by rules.  This allows the actions to
+    see the inserted row(s).  But for ON UPDATE and ON
+    DELETE rules, the original query is done after the actions added by rules.
+    This ensures that the actions can see the to-be-updated or to-be-deleted
+    rows; otherwise, the actions might do nothing because they find no rows
+    matching their qualifications.
+</Para>
+
 <Para>
     The parsetrees generated from rule actions are thrown into the
     rewrite system again and maybe more rules get applied resulting
@@ -1022,8 +1034,8 @@
     or another resultrelation. Otherwise this recursive process will end up in a loop.
     There is a compiled in recursion limit of currently 10 iterations.
     If after 10 iterations there are still update rules to apply the
-    rule system assumes a loop over multiple rule definitions and aborts the
-    transaction.
+    rule system assumes a loop over multiple rule definitions and reports
+    an error.
 </Para>
 
 <Para>
@@ -1262,10 +1274,8 @@
 </Para>
 
 <Para>
-    It is important, that the original parsetree is executed last.
-    The <ProductName>Postgres</ProductName> "traffic cop" does
-    a command counter increment between the execution of the two
-    parsetrees so the second one can see changes made by the first.
+    Here we can see why it is important that the original parsetree is
+    executed last.
     If the UPDATE would have been executed first, all the rows
     are already set to zero, so the logging INSERT
     would not find any row where 0 != shoelace_data.sl_avail.
diff --git 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
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.96 2001/07/06 13:40:47 wieck Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.97 2001/07/09 23:50:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -831,7 +831,7 @@ deepRewriteQuery(Query *parsetree)
 			 numQueryRewriteInvoked - 1);
 	}
 
-	instead = FALSE;
+	instead = false;
 	result = RewriteQuery(parsetree, &instead, &qual_products);
 
 	foreach(n, result)
@@ -845,25 +845,41 @@ deepRewriteQuery(Query *parsetree)
 	}
 
 	/*
-	 * qual_products are the original query with the negated rule
-	 * qualification of an instead rule
+	 * For INSERTs, the original query is done first; for UPDATE/DELETE, it is
+	 * done last.  This is needed because update and delete rule actions might
+	 * not do anything if they are invoked after the update or delete is
+	 * performed. The command counter increment between the query execution
+	 * makes the deleted (and maybe the updated) tuples disappear so the scans
+	 * for them in the rule actions cannot find them.
 	 */
-	if (qual_products != NIL)
-		rewritten = nconc(rewritten, qual_products);
-
-	/*
-	 * The original query is appended last (if no "instead" rule) because
-	 * update and delete rule actions might not do anything if they are
-	 * invoked after the update or delete is performed. The command
-	 * counter increment between the query execution makes the deleted
-	 * (and maybe the updated) tuples disappear so the scans for them in
-	 * the rule actions cannot find them.
-	 */
-	if (!instead)
-		if (parsetree->commandType == CMD_INSERT)
+	if (parsetree->commandType == CMD_INSERT)
+	{
+		/*
+		 * qual_products are the original query with the negated rule
+		 * qualification of an INSTEAD rule
+		 */
+		if (qual_products != NIL)
+			rewritten = nconc(qual_products, rewritten);
+		/*
+		 * Add the unmodified original query, if no INSTEAD rule was seen.
+		 */
+		if (!instead)
 			rewritten = lcons(parsetree, rewritten);
-		else
+	}
+	else
+	{
+		/*
+		 * qual_products are the original query with the negated rule
+		 * qualification of an INSTEAD rule
+		 */
+		if (qual_products != NIL)
+			rewritten = nconc(rewritten, qual_products);
+		/*
+		 * Add the unmodified original query, if no INSTEAD rule was seen.
+		 */
+		if (!instead)
 			rewritten = lappend(rewritten, parsetree);
+	}
 
 	return rewritten;
 }
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
@@ -107,7 +107,7 @@ create table rtest_nothn2 (a int4, b text);
 create table rtest_nothn3 (a int4, b text);
 create table rtest_nothn4 (a int4, b text);
 create rule rtest_nothn_r1 as on insert to rtest_nothn1
-	where new.a >= 10 and new.a < 20 do instead (select 1);
+	where new.a >= 10 and new.a < 20 do instead nothing;
 create rule rtest_nothn_r2 as on insert to rtest_nothn1
 	where new.a >= 30 and new.a < 40 do instead nothing;
 create rule rtest_nothn_r3 as on insert to rtest_nothn2
@@ -1313,7 +1313,7 @@ SELECT tablename, rulename, definition FROM pg_rules
  rtest_emp     | rtest_emp_del   | CREATE RULE rtest_emp_del AS ON DELETE TO rtest_emp DO INSERT INTO rtest_emplog (ename, who, "action", newsal, oldsal) VALUES (old.ename, "current_user"(), 'fired     '::bpchar, '$0.00'::money, old.salary);
  rtest_emp     | rtest_emp_ins   | CREATE RULE rtest_emp_ins AS ON INSERT TO rtest_emp DO INSERT INTO rtest_emplog (ename, who, "action", newsal, oldsal) VALUES (new.ename, "current_user"(), 'hired     '::bpchar, new.salary, '$0.00'::money);
  rtest_emp     | rtest_emp_upd   | CREATE RULE rtest_emp_upd AS ON UPDATE TO rtest_emp WHERE (new.salary <> old.salary) DO INSERT INTO rtest_emplog (ename, who, "action", newsal, oldsal) VALUES (new.ename, "current_user"(), 'honored   '::bpchar, new.salary, old.salary);
- rtest_nothn1  | rtest_nothn_r1  | CREATE RULE rtest_nothn_r1 AS ON INSERT TO rtest_nothn1 WHERE ((new.a >= 10) AND (new.a < 20)) DO INSTEAD SELECT 1;
+ rtest_nothn1  | rtest_nothn_r1  | CREATE RULE rtest_nothn_r1 AS ON INSERT TO rtest_nothn1 WHERE ((new.a >= 10) AND (new.a < 20)) DO INSTEAD NOTHING;
  rtest_nothn1  | rtest_nothn_r2  | CREATE RULE rtest_nothn_r2 AS ON INSERT TO rtest_nothn1 WHERE ((new.a >= 30) AND (new.a < 40)) DO INSTEAD NOTHING;
  rtest_nothn2  | rtest_nothn_r3  | CREATE RULE rtest_nothn_r3 AS ON INSERT TO rtest_nothn2 WHERE (new.a >= 100) DO INSTEAD INSERT INTO rtest_nothn3 (a, b) VALUES (new.a, new.b);
  rtest_nothn2  | rtest_nothn_r4  | CREATE RULE rtest_nothn_r4 AS ON INSERT TO rtest_nothn2 DO INSTEAD NOTHING;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
@@ -131,7 +131,7 @@ create table rtest_nothn3 (a int4, b text);
 create table rtest_nothn4 (a int4, b text);
 
 create rule rtest_nothn_r1 as on insert to rtest_nothn1
-	where new.a >= 10 and new.a < 20 do instead (select 1);
+	where new.a >= 10 and new.a < 20 do instead nothing;
 
 create rule rtest_nothn_r2 as on insert to rtest_nothn1
 	where new.a >= 30 and new.a < 40 do instead nothing;
