Rule rewriter was doing the wrong thing with conditional INSTEAD rules

whose conditions might yield NULL.  The negated qual to attach to the
original query is properly 'x IS NOT TRUE', not 'NOT x'.  This fix
produces correct behavior, but we may be taking a performance hit because
the planner is much stupider about IS NOT TRUE than it is about NOT
clauses.  Future TODO: teach prepqual, other parts of planner how to
cope with BooleanTest clauses more effectively.

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 8bdc772318447897cee88ba929efebaee812d8f5..bb58db8e8a5584deaf07dc47a51105e8ad0cbb63 100644 (file)
--- 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.112 2002/10/19 19:00:47 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.113 2002/10/20 00:58:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -842,9 +842,11 @@ fireRIRrules(Query *parsetree)
 
 
 /*
- * Modify the given query by adding 'AND NOT rule_qual' to its qualification.
- * This is used to generate suitable "else clauses" for conditional INSTEAD
- * rules.
+ * Modify the given query by adding 'AND rule_qual IS NOT TRUE' to its
+ * qualification.  This is used to generate suitable "else clauses" for
+ * conditional INSTEAD rules.  (Unfortunately we must use "x IS NOT TRUE",
+ * not just "NOT x" which the planner is much smarter about, else we will
+ * do the wrong thing when the qual evaluates to NULL.)
  *
  * The rule_qual may contain references to OLD or NEW. OLD references are
  * replaced by references to the specified rt_index (the relation that the
@@ -853,10 +855,10 @@ fireRIRrules(Query *parsetree)
  * of the related entries in the query's own targetlist.
  */
 static Query *
-CopyAndAddQual(Query *parsetree,
-                          Node *rule_qual,
-                          int rt_index,
-                          CmdType event)
+CopyAndAddInvertedQual(Query *parsetree,
+                                          Node *rule_qual,
+                                          int rt_index,
+                                          CmdType event)
 {
        Query      *new_tree = (Query *) copyObject(parsetree);
        Node       *new_qual = (Node *) copyObject(rule_qual);
@@ -872,7 +874,7 @@ CopyAndAddQual(Query *parsetree,
                                                          event,
                                                          rt_index);
        /* And attach the fixed qual */
-       AddNotQual(new_tree, new_qual);
+       AddInvertedQual(new_tree, new_qual);
 
        return new_tree;
 }
@@ -956,10 +958,10 @@ fireRules(Query *parsetree,
                        {
                                if (*qual_product == NULL)
                                        *qual_product = parsetree;
-                               *qual_product = CopyAndAddQual(*qual_product,
-                                                                                          event_qual,
-                                                                                          rt_index,
-                                                                                          event);
+                               *qual_product = CopyAndAddInvertedQual(*qual_product,
+                                                                                                          event_qual,
+                                                                                                          rt_index,
+                                                                                                          event);
                        }
                }
 

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 12154b8da6e8c9c395252da8ea6c9bd8bad41517..94a940648c002bae6b628f03664b767652f506b0 100644 (file)
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.66 2002/09/11 14:48:54 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteManip.c,v 1.67 2002/10/20 00:58:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -690,34 +690,26 @@ AddHavingQual(Query *parsetree, Node *havingQual)
        parsetree->hasSubLinks |= checkExprHasSubLink(copy);
 }
 
-#ifdef NOT_USED
-void
-AddNotHavingQual(Query *parsetree, Node *havingQual)
-{
-       Node       *notqual;
-
-       if (havingQual == NULL)
-               return;
-
-       /* Need not copy input qual, because AddHavingQual will... */
-       notqual = (Node *) make_notclause((Expr *) havingQual);
-
-       AddHavingQual(parsetree, notqual);
-}
-#endif
 
+/*
+ * Invert the given clause and add it to the WHERE qualifications of the
+ * given querytree.  Inversion means "x IS NOT TRUE", not just "NOT x",
+ * else we will do the wrong thing when x evaluates to NULL.
+ */
 void
-AddNotQual(Query *parsetree, Node *qual)
+AddInvertedQual(Query *parsetree, Node *qual)
 {
-       Node       *notqual;
+       BooleanTest *invqual;
 
        if (qual == NULL)
                return;
 
        /* Need not copy input qual, because AddQual will... */
-       notqual = (Node *) make_notclause((Expr *) qual);
+       invqual = makeNode(BooleanTest);
+       invqual->arg = qual;
+       invqual->booltesttype = IS_NOT_TRUE;
 
-       AddQual(parsetree, notqual);
+       AddQual(parsetree, (Node *) invqual);
 }
 
 

diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index df54169fd28060813ec61385cbc0eef6cd4d2f0f..9a9b0ae7adf4df1776a20164e942269f4f2e3a4e 100644 (file)
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: rewriteManip.h,v 1.31 2002/06/20 20:29:52 momjian Exp $
+ * $Id: rewriteManip.h,v 1.32 2002/10/20 00:58:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -32,7 +32,7 @@ extern Query *getInsertSelectQuery(Query *parsetree, Query ***subquery_ptr);
 
 extern void AddQual(Query *parsetree, Node *qual);
 extern void AddHavingQual(Query *parsetree, Node *havingQual);
-extern void AddNotQual(Query *parsetree, Node *qual);
+extern void AddInvertedQual(Query *parsetree, Node *qual);
 
 extern bool checkExprHasAggs(Node *node);
 extern bool checkExprHasSubLink(Node *node);