Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitda88191

Browse files
committed
Fix incorrect matching of subexpressions in outer-join plan nodes.
Previously we would re-use input subexpressions in all expression treesattached to a Join plan node. However, if it's an outer join and thesubexpression appears in the nullable-side input, this is potentiallyincorrect for apparently-matching subexpressions that came from abovethe outer join (ie, targetlist and qpqual expressions), because theexecutor will treat the subexpression value as NULL when maybe it shouldnot be.The case is fairly hard to hit because (a) you need a non-strictsubexpression (else NULL is correct), and (b) we don't usually computeexpressions in the outputs of non-toplevel plan nodes. But we might doso if the expressions are sort keys for a mergejoin, for example.Probably in the long run we should make a more explicit distinction betweenVars appearing above and below an outer join, but that will be a majorplanner redesign and not at all back-patchable. For the moment, just hackset_join_references so that it will not match any non-Var expressionscoming from nullable inputs to expressions that came from above the join.(This is somewhat overkill, in that a strict expression could still bematched, but it doesn't seem worth the effort to check that.)Per report from Qingqing Zhou. The added regression test case is basedon his example.This has been broken for a very long time, so back-patch to all activebranches.
1 parent6450814 commitda88191

File tree

3 files changed

+121
-14
lines changed

3 files changed

+121
-14
lines changed

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

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -943,19 +943,13 @@ set_join_references(PlannerGlobal *glob, Join *join, int rtoffset)
943943
outer_itlist=build_tlist_index(outer_plan->targetlist);
944944
inner_itlist=build_tlist_index(inner_plan->targetlist);
945945

946-
/* All join plans have tlist, qual, and joinqual */
947-
join->plan.targetlist=fix_join_expr(glob,
948-
join->plan.targetlist,
949-
outer_itlist,
950-
inner_itlist,
951-
(Index)0,
952-
rtoffset);
953-
join->plan.qual=fix_join_expr(glob,
954-
join->plan.qual,
955-
outer_itlist,
956-
inner_itlist,
957-
(Index)0,
958-
rtoffset);
946+
/*
947+
* First process the joinquals (including merge or hash clauses). These
948+
* are logically below the join so they can always use all values
949+
* available from the input tlists. It's okay to also handle
950+
* NestLoopParams now, because those couldn't refer to nullable
951+
* subexpressions.
952+
*/
959953
join->joinqual=fix_join_expr(glob,
960954
join->joinqual,
961955
outer_itlist,
@@ -992,6 +986,49 @@ set_join_references(PlannerGlobal *glob, Join *join, int rtoffset)
992986
rtoffset);
993987
}
994988

989+
/*
990+
* Now we need to fix up the targetlist and qpqual, which are logically
991+
* above the join. This means they should not re-use any input expression
992+
* that was computed in the nullable side of an outer join. Vars and
993+
* PlaceHolderVars are fine, so we can implement this restriction just by
994+
* clearing has_non_vars in the indexed_tlist structs.
995+
*
996+
* XXX This is a grotty workaround for the fact that we don't clearly
997+
* distinguish between a Var appearing below an outer join and the "same"
998+
* Var appearing above it. If we did, we'd not need to hack the matching
999+
* rules this way.
1000+
*/
1001+
switch (join->jointype)
1002+
{
1003+
caseJOIN_LEFT:
1004+
caseJOIN_SEMI:
1005+
caseJOIN_ANTI:
1006+
inner_itlist->has_non_vars= false;
1007+
break;
1008+
caseJOIN_RIGHT:
1009+
outer_itlist->has_non_vars= false;
1010+
break;
1011+
caseJOIN_FULL:
1012+
outer_itlist->has_non_vars= false;
1013+
inner_itlist->has_non_vars= false;
1014+
break;
1015+
default:
1016+
break;
1017+
}
1018+
1019+
join->plan.targetlist=fix_join_expr(glob,
1020+
join->plan.targetlist,
1021+
outer_itlist,
1022+
inner_itlist,
1023+
(Index)0,
1024+
rtoffset);
1025+
join->plan.qual=fix_join_expr(glob,
1026+
join->plan.qual,
1027+
outer_itlist,
1028+
inner_itlist,
1029+
(Index)0,
1030+
rtoffset);
1031+
9951032
pfree(outer_itlist);
9961033
pfree(inner_itlist);
9971034
}
@@ -1453,7 +1490,9 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
14531490
* If no match, return NULL.
14541491
*
14551492
* NOTE: it is a waste of time to call this unless itlist->has_ph_vars or
1456-
* itlist->has_non_vars
1493+
* itlist->has_non_vars. Furthermore, set_join_references() relies on being
1494+
* able to prevent matching of non-Vars by clearing itlist->has_non_vars,
1495+
* so there's a correctness reason not to call it unless that's set.
14571496
*/
14581497
staticVar*
14591498
search_indexed_tlist_for_non_var(Node*node,

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2751,6 +2751,54 @@ explain (costs off)
27512751
Index Cond: (b.unique2 = 42)
27522752
(6 rows)
27532753

2754+
--
2755+
-- test that quals attached to an outer join have correct semantics,
2756+
-- specifically that they don't re-use expressions computed below the join;
2757+
-- we force a mergejoin so that coalesce(b.q1, 1) appears as a join input
2758+
--
2759+
set enable_hashjoin to off;
2760+
set enable_nestloop to off;
2761+
explain (verbose, costs off)
2762+
select a.q2, b.q1
2763+
from int8_tbl a left join int8_tbl b on a.q2 = coalesce(b.q1, 1)
2764+
where coalesce(b.q1, 1) > 0;
2765+
QUERY PLAN
2766+
-------------------------------------------------------
2767+
Merge Left Join
2768+
Output: a.q2, b.q1
2769+
Merge Cond: (a.q2 = (COALESCE(b.q1, 1::bigint)))
2770+
Filter: (COALESCE(b.q1, 1::bigint) > 0)
2771+
-> Sort
2772+
Output: a.q2
2773+
Sort Key: a.q2
2774+
-> Seq Scan on public.int8_tbl a
2775+
Output: a.q2
2776+
-> Sort
2777+
Output: b.q1, (COALESCE(b.q1, 1::bigint))
2778+
Sort Key: (COALESCE(b.q1, 1::bigint))
2779+
-> Seq Scan on public.int8_tbl b
2780+
Output: b.q1, COALESCE(b.q1, 1::bigint)
2781+
(14 rows)
2782+
2783+
select a.q2, b.q1
2784+
from int8_tbl a left join int8_tbl b on a.q2 = coalesce(b.q1, 1)
2785+
where coalesce(b.q1, 1) > 0;
2786+
q2 | q1
2787+
-------------------+------------------
2788+
-4567890123456789 |
2789+
123 | 123
2790+
123 | 123
2791+
456 |
2792+
4567890123456789 | 4567890123456789
2793+
4567890123456789 | 4567890123456789
2794+
4567890123456789 | 4567890123456789
2795+
4567890123456789 | 4567890123456789
2796+
4567890123456789 | 4567890123456789
2797+
4567890123456789 | 4567890123456789
2798+
(10 rows)
2799+
2800+
reset enable_hashjoin;
2801+
reset enable_nestloop;
27542802
--
27552803
-- test join removal
27562804
--

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,26 @@ explain (costs off)
729729
explain (costs off)
730730
select*from tenk1 a fulljoin tenk1 b using(unique2)where unique2=42;
731731

732+
--
733+
-- test that quals attached to an outer join have correct semantics,
734+
-- specifically that they don't re-use expressions computed below the join;
735+
-- we force a mergejoin so that coalesce(b.q1, 1) appears as a join input
736+
--
737+
738+
set enable_hashjoin to off;
739+
set enable_nestloop to off;
740+
741+
explain (verbose, costs off)
742+
selecta.q2,b.q1
743+
from int8_tbl aleft join int8_tbl bona.q2= coalesce(b.q1,1)
744+
where coalesce(b.q1,1)>0;
745+
selecta.q2,b.q1
746+
from int8_tbl aleft join int8_tbl bona.q2= coalesce(b.q1,1)
747+
where coalesce(b.q1,1)>0;
748+
749+
reset enable_hashjoin;
750+
reset enable_nestloop;
751+
732752
--
733753
-- test join removal
734754
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp