From 8c441c082797f22ae96f50b641a8ef3f65c92b8d Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Tue, 9 Jan 2024 10:01:22 +0200 Subject: [PATCH] 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 --- src/backend/optimizer/plan/analyzejoins.c | 11 +++ src/test/regress/expected/join.out | 76 ++++++++++++------- src/test/regress/expected/updatable_views.out | 17 +++-- src/test/regress/sql/join.sql | 28 ++++--- 4 files changed, 84 insertions(+), 48 deletions(-) diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 1fb17ee29f..1f2167a11d 100644 --- 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 fb4a7ea66e..321b6a7147 100644 --- 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 a73c1f90c4..1950e6f281 100644 --- 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 2321dbf949..6d066ef952 100644 --- 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;