Fix problems with rewriter failing to set Query.hasSubLinks when inserting

a SubLink expression into a rule query.  Pre-8.1 we essentially did this
unconditionally; 8.1 tries to do it only when needed, but was missing a
couple of cases.  Per report from Kyle Bateman.  Add some regression test
cases covering this area.

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index afde40dc54eb9318cce7f73f38c23569899b86a5..45b5b9207faddb3da4eee54ddca1151305471cb5 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
- *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.159 2005/11/22 18:17:19 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.160 2005/11/23 17:21:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -374,6 +374,14 @@ rewriteRuleAction(Query *parsetree,
 
                        sub_action->jointree->fromlist =
                                list_concat(newjointree, sub_action->jointree->fromlist);
+
+                       /*
+                        * There could have been some SubLinks in newjointree, in which
+                        * case we'd better mark the sub_action correctly.
+                        */
+                       if (parsetree->hasSubLinks && !sub_action->hasSubLinks)
+                               sub_action->hasSubLinks =
+                                       checkExprHasSubLink((Node *) newjointree);
                }
        }
 

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index dbdd62c3e15c96b82c86add41d25793521e59139..3f7af865775be71504f2d2c247ab4880b72f2988 100644 (file)
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.93 2005/11/22 18:17:19 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.94 2005/11/23 17:21:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -930,6 +930,7 @@ ResolveNew(Node *node, int target_varno, int sublevels_up,
                   RangeTblEntry *target_rte,
                   List *targetlist, int event, int update_varno)
 {
+       Node       *result;
        ResolveNew_context context;
 
        context.target_varno = target_varno;
@@ -944,8 +945,21 @@ ResolveNew(Node *node, int target_varno, int sublevels_up,
         * Must be prepared to start with a Query or a bare expression tree; if
         * it's a Query, we don't want to increment sublevels_up.
         */
-       return query_or_expression_tree_mutator(node,
-                                                                                       ResolveNew_mutator,
-                                                                                       (void *) &context,
-                                                                                       0);
+       result = query_or_expression_tree_mutator(node,
+                                                                                         ResolveNew_mutator,
+                                                                                         (void *) &context,
+                                                                                         0);
+
+       if (context.inserted_sublink)
+       {
+               if (IsA(result, Query))
+                       ((Query *) result)->hasSubLinks = true;
+               /*
+                * Note: if we're called on a non-Query node then it's the caller's
+                * responsibility to update hasSubLinks in the ancestor Query.
+                * This is pretty fragile and perhaps should be rethought ...
+                */
+       }
+
+       return result;
 }

diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index d99656eac774a94beea91a16cb97f1cf362feb6b..f36b5acfe8653f51d82d14c0e1ae2a28b8fa16de 100644 (file)
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -334,3 +334,55 @@ SELECT * FROM orders_view;
 DROP TABLE orderstest cascade;
 NOTICE:  drop cascades to rule _RETURN on view orders_view
 NOTICE:  drop cascades to view orders_view
+--
+-- Test cases to catch situations where rule rewriter fails to propagate
+-- hasSubLinks flag correctly.  Per example from Kyle Bateman.
+--
+create temp table parts (
+    partnum     text,
+    cost        float8
+);
+create temp table shipped (
+    ttype       char(2),
+    ordnum      int4,
+    partnum     text,
+    value       float8
+);
+create temp view shipped_view as
+    select * from shipped where ttype = 'wt';
+create rule shipped_view_insert as on insert to shipped_view do instead
+    insert into shipped values('wt', new.ordnum, new.partnum, new.value);
+insert into parts (partnum, cost) values (1, 1234.56);
+insert into shipped_view (ordnum, partnum, value)
+    values (0, 1, (select cost from parts where partnum = 1));
+select * from shipped_view;
+ ttype | ordnum | partnum |  value  
+-------+--------+---------+---------
+ wt    |      0 | 1       | 1234.56
+(1 row)
+
+create rule shipped_view_update as on update to shipped_view do instead
+    update shipped set partnum = new.partnum, value = new.value
+        where ttype = new.ttype and ordnum = new.ordnum;
+update shipped_view set value = 11
+    from int4_tbl a join int4_tbl b
+      on (a.f1 = (select f1 from int4_tbl c where c.f1=b.f1))
+    where ordnum = a.f1;
+select * from shipped_view;
+ ttype | ordnum | partnum | value 
+-------+--------+---------+-------
+ wt    |      0 | 1       |    11
+(1 row)
+
+select f1, ss1 as relabel from
+    (select *, (select sum(f1) from int4_tbl b where f1 >= a.f1) as ss1
+     from int4_tbl a) ss;
+     f1      |  relabel   
+-------------+------------
+           0 | 2147607103
+      123456 | 2147607103
+     -123456 | 2147483647
+  2147483647 | 2147483647
+ -2147483647 |          0
+(5 rows)
+

diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index a07cc337596c97c01605d0ec5396ba9b208b4eb5..b8cb45c6fc75d0204f6cecb68f978e0ba13edbd1 100644 (file)
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -191,3 +191,48 @@ FROM orderstest ord;
 SELECT * FROM orders_view;
 
 DROP TABLE orderstest cascade;
+
+--
+-- Test cases to catch situations where rule rewriter fails to propagate
+-- hasSubLinks flag correctly.  Per example from Kyle Bateman.
+--
+
+create temp table parts (
+    partnum     text,
+    cost        float8
+);
+
+create temp table shipped (
+    ttype       char(2),
+    ordnum      int4,
+    partnum     text,
+    value       float8
+);
+
+create temp view shipped_view as
+    select * from shipped where ttype = 'wt';
+
+create rule shipped_view_insert as on insert to shipped_view do instead
+    insert into shipped values('wt', new.ordnum, new.partnum, new.value);
+
+insert into parts (partnum, cost) values (1, 1234.56);
+
+insert into shipped_view (ordnum, partnum, value)
+    values (0, 1, (select cost from parts where partnum = 1));
+
+select * from shipped_view;
+
+create rule shipped_view_update as on update to shipped_view do instead
+    update shipped set partnum = new.partnum, value = new.value
+        where ttype = new.ttype and ordnum = new.ordnum;
+
+update shipped_view set value = 11
+    from int4_tbl a join int4_tbl b
+      on (a.f1 = (select f1 from int4_tbl c where c.f1=b.f1))
+    where ordnum = a.f1;
+
+select * from shipped_view;
+
+select f1, ss1 as relabel from
+    (select *, (select sum(f1) from int4_tbl b where f1 >= a.f1) as ss1
+     from int4_tbl a) ss;