Skip to content

Commit 1aa7cf9

Browse files
akorotkovakorotkov
committed
Disallow removing placeholders during Self-Join Elimination.
fc069a3 implements Self-Join Elimination (SJE), which can remove base relations when appropriate. However, regressions tests for SJE only cover the case when placeholder variables (PHVs) are evaluated and needed only in a single base rel. If this baserel is removed due to SJE, its clauses, including PHVs, will be transferred to the keeping relation. Removing these PHVs may trigger an error on plan creation -- thanks to the b3ff6c7 for detecting that. This commit skips removal of PHVs during SJE. This might also happen that we skip the removal of some PHVs that could be removed. However, the overhead of extra PHVs is small compared to the complexity of analysis needed to remove them. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Alena Rybakina <a.rybakina@postgrespro.ru> Author: Andrei Lepikhov <lepihov@gmail.com> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> Reviewed-by: Richard Guo <guofenglinux@gmail.com>
1 parent 2f5b056 commit 1aa7cf9

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

‎src/backend/optimizer/plan/analyzejoins.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,12 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
403403

404404
/*
405405
* Likewise remove references from PlaceHolderVar data structures,
406-
* removing any no-longer-needed placeholders entirely.
406+
* removing any no-longer-needed placeholders entirely. We remove PHV
407+
* only for left-join removal. With self-join elimination, PHVs already
408+
* get moved to the remaining relation, where they might still be needed.
409+
* It might also happen that we skip the removal of some PHVs that could
410+
* be removed. However, the overhead of extra PHVs is small compared to
411+
* the complexity of analysis needed to remove them.
407412
*
408413
* Removal is a bit trickier than it might seem: we can remove PHVs that
409414
* are used at the target rel and/or in the join qual, but not those that
@@ -420,10 +425,16 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
420425
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
421426

422427
Assert(sjinfo == NULL || !bms_is_member(relid, phinfo->ph_lateral));
423-
if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
428+
if (sjinfo != NULL &&
429+
bms_is_subset(phinfo->ph_needed, joinrelids) &&
424430
bms_is_member(relid, phinfo->ph_eval_at) &&
425-
(sjinfo == NULL || !bms_is_member(sjinfo->ojrelid, phinfo->ph_eval_at)))
431+
!bms_is_member(sjinfo->ojrelid, phinfo->ph_eval_at))
426432
{
433+
/*
434+
* This code shouldn't be executed if one relation is substituted
435+
* with another: in this case, the placeholder may be employed in
436+
* a filter inside the scan node the SJE removes.
437+
*/
427438
root->placeholder_list = foreach_delete_current(root->placeholder_list,
428439
l);
429440
root->placeholder_array[phinfo->phid] = NULL;

‎src/test/regress/expected/join.out

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7150,7 +7150,8 @@ on true;
71507150
-> Seq Scan on emp1 t4
71517151
(7 rows)
71527152

7153-
-- Check that SJE removes the whole PHVs correctly
7153+
-- Try PHV, which could potentially be removed completely by SJE, but that's
7154+
-- not implemented yet.
71547155
explain (verbose, costs off)
71557156
select 1 from emp1 t1 left join
71567157
((select 1 as x, * from emp1 t2) s1 inner join
@@ -7200,6 +7201,37 @@ on true;
72007201
Output: t3.id, t1.id
72017202
(7 rows)
72027203

7204+
-- This is a degenerate case of PHV usage: it is evaluated and needed inside
7205+
-- a baserel scan operation that the SJE removes. The PHV in this test should
7206+
-- be in the filter of parameterized Index Scan: the replace_nestloop_params()
7207+
-- code will detect if the placeholder list doesn't have a reference to this
7208+
-- parameter.
7209+
--
7210+
-- NOTE: enable_hashjoin and enable_mergejoin must be disabled.
7211+
CREATE TABLE tbl_phv(x int, y int PRIMARY KEY);
7212+
CREATE INDEX tbl_phv_idx ON tbl_phv(x);
7213+
INSERT INTO tbl_phv (x, y)
7214+
SELECT gs, gs FROM generate_series(1,100) AS gs;
7215+
VACUUM ANALYZE tbl_phv;
7216+
EXPLAIN (COSTS OFF, VERBOSE)
7217+
SELECT 1 FROM tbl_phv t1 LEFT JOIN
7218+
(SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
7219+
(SELECT y FROM tbl_phv tr) t4
7220+
ON t4.y = t3.y
7221+
ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
7222+
QUERY PLAN
7223+
---------------------------------------------------------
7224+
Nested Loop
7225+
Output: 1
7226+
-> Seq Scan on public.tbl_phv t1
7227+
Output: t1.x, t1.y
7228+
-> Index Scan using tbl_phv_idx on public.tbl_phv tr
7229+
Output: tr.x, tr.y
7230+
Index Cond: (tr.x = (t1.x % 2))
7231+
Filter: (1 IS NOT NULL)
7232+
(8 rows)
7233+
7234+
DROP TABLE IF EXISTS tbl_phv;
72037235
-- Check that SJE replaces join clauses involving the removed rel correctly
72047236
explain (costs off)
72057237
select * from emp1 t1

‎src/test/regress/sql/join.sql

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2756,7 +2756,8 @@ select * from emp1 t1 left join
27562756
on true)
27572757
on true;
27582758

2759-
-- Check that SJE removes the whole PHVs correctly
2759+
-- Try PHV, which could potentially be removed completely by SJE, but that's
2760+
-- not implemented yet.
27602761
explain (verbose, costs off)
27612762
select 1 from emp1 t1 left join
27622763
((select 1 as x, * from emp1 t2) s1 inner join
@@ -2774,6 +2775,26 @@ select * from generate_series(1,10) t1(id) left join
27742775
lateral (select t1.id as t1id, t2.id from emp1 t2 join emp1 t3 on t2.id = t3.id)
27752776
on true;
27762777

2778+
-- This is a degenerate case of PHV usage: it is evaluated and needed inside
2779+
-- a baserel scan operation that the SJE removes. The PHV in this test should
2780+
-- be in the filter of parameterized Index Scan: the replace_nestloop_params()
2781+
-- code will detect if the placeholder list doesn't have a reference to this
2782+
-- parameter.
2783+
--
2784+
-- NOTE: enable_hashjoin and enable_mergejoin must be disabled.
2785+
CREATE TABLE tbl_phv(x int, y int PRIMARY KEY);
2786+
CREATE INDEX tbl_phv_idx ON tbl_phv(x);
2787+
INSERT INTO tbl_phv (x, y)
2788+
SELECT gs, gs FROM generate_series(1,100) AS gs;
2789+
VACUUM ANALYZE tbl_phv;
2790+
EXPLAIN (COSTS OFF, VERBOSE)
2791+
SELECT 1 FROM tbl_phv t1 LEFT JOIN
2792+
(SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
2793+
(SELECT y FROM tbl_phv tr) t4
2794+
ON t4.y = t3.y
2795+
ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
2796+
DROP TABLE IF EXISTS tbl_phv;
2797+
27772798
-- Check that SJE replaces join clauses involving the removed rel correctly
27782799
explain (costs off)
27792800
select * from emp1 t1

0 commit comments

Comments
 (0)