Forbid SJE with result relation

The target relation for INSERT/UPDATE/DELETE/MERGE has a different behavior
than other relations in EvalPlanQual() and RETURNING clause.  This is why we
forbid target relation to be either source or target relation in SJE.
It's not clear if we could ever support this.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/b9e8f460-f9a6-0e9b-e8ba-60d59f0bc22c%40gmail.com

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 1fb17ee29f2619b2a0ff54d39c65cc330b2cb290..1f2167a11de9af42c7fb2b02c0adfef8a3c93ef5 100644 (file)
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -2086,6 +2086,14 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids)
    {
        RelOptInfo *inner = root->simple_rel_array[r];
 
+       /*
+        * We don't accept result relation as either source or target relation
+        * of SJE, because result relation has different behavior in
+        * EvalPlanQual() and RETURNING clause.
+        */
+       if (root->parse->resultRelation == r)
+           continue;
+
        k = r;
 
        while ((k = bms_next_member(relids, k)) > 0)
@@ -2101,6 +2109,9 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids)
            PlanRowMark *imark = NULL;
            List       *uclauses = NIL;
 
+           if (root->parse->resultRelation == k)
+               continue;
+
            /* A sanity check: the relations have the same Oid. */
            Assert(root->simple_rte_array[k]->relid ==
                   root->simple_rte_array[r]->relid);

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index fb4a7ea66efbba0dd7c1fcec8373caf31e7efdab..321b6a7147bd6c2b00bdb456312852c6fc18ed88 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6868,18 +6868,58 @@ select * from emp1 t1
          ->  Seq Scan on emp1 t3
 (6 rows)
 
--- Check that SJE replaces target relation correctly
+-- Check that SJE doesn't replace the target relation
 explain (costs off)
 WITH t1 AS (SELECT * FROM emp1)
 UPDATE emp1 SET code = t1.code + 1 FROM t1
-WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
-            QUERY PLAN            
-----------------------------------
+WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code, t1.code;
+                      QUERY PLAN                       
+-------------------------------------------------------
  Update on emp1
-   ->  Seq Scan on emp1
-         Filter: (id IS NOT NULL)
-(3 rows)
+   ->  Nested Loop
+         ->  Seq Scan on emp1
+         ->  Index Scan using emp1_pkey on emp1 emp1_1
+               Index Cond: (id = emp1.id)
+(5 rows)
+
+INSERT INTO emp1 VALUES (1, 1), (2, 1);
+WITH t1 AS (SELECT * FROM emp1)
+UPDATE emp1 SET code = t1.code + 1 FROM t1
+WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code, t1.code;
+ id | code | code 
+----+------+------
+  1 |    2 |    1
+  2 |    2 |    1
+(2 rows)
+
+TRUNCATE emp1;
+EXPLAIN (COSTS OFF)
+UPDATE sj sq SET b = 1 FROM sj as sz WHERE sq.a = sz.a;
+             QUERY PLAN              
+-------------------------------------
+ Update on sj sq
+   ->  Nested Loop
+         Join Filter: (sq.a = sz.a)
+         ->  Seq Scan on sj sq
+         ->  Materialize
+               ->  Seq Scan on sj sz
+(6 rows)
 
+CREATE RULE sj_del_rule AS ON DELETE TO sj
+  DO INSTEAD
+    UPDATE sj SET a = 1 WHERE a = old.a;
+EXPLAIN (COSTS OFF) DELETE FROM sj;
+              QUERY PLAN              
+--------------------------------------
+ Update on sj sj_1
+   ->  Nested Loop
+         Join Filter: (sj.a = sj_1.a)
+         ->  Seq Scan on sj sj_1
+         ->  Materialize
+               ->  Seq Scan on sj
+(6 rows)
+
+DROP RULE sj_del_rule ON sj CASCADE;
 -- Check that SJE does not mistakenly omit qual clauses (bug #18187)
 insert into emp1 values (1, 1);
 explain (costs off)
@@ -7076,7 +7116,6 @@ ON sj_t1.id = _t2t3t4.id;
 --
 -- Test RowMarks-related code
 --
--- TODO: Why this select returns two copies of ctid field? Should we fix it?
 EXPLAIN (COSTS OFF) -- Both sides have explicit LockRows marks
 SELECT a1.a FROM sj a1,sj a2 WHERE (a1.a=a2.a) FOR UPDATE;
            QUERY PLAN            
@@ -7086,27 +7125,6 @@ SELECT a1.a FROM sj a1,sj a2 WHERE (a1.a=a2.a) FOR UPDATE;
          Filter: (a IS NOT NULL)
 (3 rows)
 
-EXPLAIN (COSTS OFF) -- A RowMark exists for the table being kept
-UPDATE sj sq SET b = 1 FROM sj as sz WHERE sq.a = sz.a;
-           QUERY PLAN            
----------------------------------
- Update on sj sz
-   ->  Seq Scan on sj sz
-         Filter: (a IS NOT NULL)
-(3 rows)
-
-CREATE RULE sj_del_rule AS ON DELETE TO sj
-  DO INSTEAD
-    UPDATE sj SET a = 1 WHERE a = old.a;
-EXPLAIN (COSTS OFF) DELETE FROM sj; -- A RowMark exists for the table being dropped
-           QUERY PLAN            
----------------------------------
- Update on sj
-   ->  Seq Scan on sj
-         Filter: (a IS NOT NULL)
-(3 rows)
-
-DROP RULE sj_del_rule ON sj CASCADE;
 reset enable_hashjoin;
 reset enable_mergejoin;
 --

diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index a73c1f90c4af848847a3f7a204069fc66fa70792..1950e6f281f5f729eae12be95024f39490e53bef 100644 (file)
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2499,13 +2499,16 @@ SELECT * FROM rw_view1;
 (1 row)
 
 EXPLAIN (costs off) DELETE FROM rw_view1 WHERE id = 1 AND snoop(data);
-                    QUERY PLAN                    
---------------------------------------------------
- Update on base_tbl
-   ->  Index Scan using base_tbl_pkey on base_tbl
-         Index Cond: (id = 1)
-         Filter: ((NOT deleted) AND snoop(data))
-(4 rows)
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Update on base_tbl base_tbl_1
+   ->  Nested Loop
+         ->  Index Scan using base_tbl_pkey on base_tbl base_tbl_1
+               Index Cond: (id = 1)
+         ->  Index Scan using base_tbl_pkey on base_tbl
+               Index Cond: (id = 1)
+               Filter: ((NOT deleted) AND snoop(data))
+(7 rows)
 
 DELETE FROM rw_view1 WHERE id = 1 AND snoop(data);
 NOTICE:  snooped value: Row 1

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 2321dbf949fe8cce37c1ed6b670d68f46dce9521..6d066ef9528b4369c97cf3bc2eff6d34bd63a6b7 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2616,11 +2616,25 @@ select * from emp1 t1
    inner join emp1 t2 on t1.id = t2.id
     left join emp1 t3 on t1.id > 1 and t1.id < 2;
 
--- Check that SJE replaces target relation correctly
+-- Check that SJE doesn't replace the target relation
 explain (costs off)
 WITH t1 AS (SELECT * FROM emp1)
 UPDATE emp1 SET code = t1.code + 1 FROM t1
-WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code;
+WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code, t1.code;
+INSERT INTO emp1 VALUES (1, 1), (2, 1);
+WITH t1 AS (SELECT * FROM emp1)
+UPDATE emp1 SET code = t1.code + 1 FROM t1
+WHERE t1.id = emp1.id RETURNING emp1.id, emp1.code, t1.code;
+TRUNCATE emp1;
+
+EXPLAIN (COSTS OFF)
+UPDATE sj sq SET b = 1 FROM sj as sz WHERE sq.a = sz.a;
+
+CREATE RULE sj_del_rule AS ON DELETE TO sj
+  DO INSTEAD
+    UPDATE sj SET a = 1 WHERE a = old.a;
+EXPLAIN (COSTS OFF) DELETE FROM sj;
+DROP RULE sj_del_rule ON sj CASCADE;
 
 -- Check that SJE does not mistakenly omit qual clauses (bug #18187)
 insert into emp1 values (1, 1);
@@ -2729,19 +2743,9 @@ ON sj_t1.id = _t2t3t4.id;
 -- Test RowMarks-related code
 --
 
--- TODO: Why this select returns two copies of ctid field? Should we fix it?
 EXPLAIN (COSTS OFF) -- Both sides have explicit LockRows marks
 SELECT a1.a FROM sj a1,sj a2 WHERE (a1.a=a2.a) FOR UPDATE;
 
-EXPLAIN (COSTS OFF) -- A RowMark exists for the table being kept
-UPDATE sj sq SET b = 1 FROM sj as sz WHERE sq.a = sz.a;
-
-CREATE RULE sj_del_rule AS ON DELETE TO sj
-  DO INSTEAD
-    UPDATE sj SET a = 1 WHERE a = old.a;
-EXPLAIN (COSTS OFF) DELETE FROM sj; -- A RowMark exists for the table being dropped
-DROP RULE sj_del_rule ON sj CASCADE;
-
 reset enable_hashjoin;
 reset enable_mergejoin;