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

Commit1d71d36

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 parent406b2f0 commit1d71d36

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
@@ -1231,19 +1231,13 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
12311231
outer_itlist=build_tlist_index(outer_plan->targetlist);
12321232
inner_itlist=build_tlist_index(inner_plan->targetlist);
12331233

1234-
/* All join plans have tlist, qual, and joinqual */
1235-
join->plan.targetlist=fix_join_expr(root,
1236-
join->plan.targetlist,
1237-
outer_itlist,
1238-
inner_itlist,
1239-
(Index)0,
1240-
rtoffset);
1241-
join->plan.qual=fix_join_expr(root,
1242-
join->plan.qual,
1243-
outer_itlist,
1244-
inner_itlist,
1245-
(Index)0,
1246-
rtoffset);
1234+
/*
1235+
* First process the joinquals (including merge or hash clauses). These
1236+
* are logically below the join so they can always use all values
1237+
* available from the input tlists. It's okay to also handle
1238+
* NestLoopParams now, because those couldn't refer to nullable
1239+
* subexpressions.
1240+
*/
12471241
join->joinqual=fix_join_expr(root,
12481242
join->joinqual,
12491243
outer_itlist,
@@ -1295,6 +1289,49 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
12951289
rtoffset);
12961290
}
12971291

1292+
/*
1293+
* Now we need to fix up the targetlist and qpqual, which are logically
1294+
* above the join. This means they should not re-use any input expression
1295+
* that was computed in the nullable side of an outer join. Vars and
1296+
* PlaceHolderVars are fine, so we can implement this restriction just by
1297+
* clearing has_non_vars in the indexed_tlist structs.
1298+
*
1299+
* XXX This is a grotty workaround for the fact that we don't clearly
1300+
* distinguish between a Var appearing below an outer join and the "same"
1301+
* Var appearing above it. If we did, we'd not need to hack the matching
1302+
* rules this way.
1303+
*/
1304+
switch (join->jointype)
1305+
{
1306+
caseJOIN_LEFT:
1307+
caseJOIN_SEMI:
1308+
caseJOIN_ANTI:
1309+
inner_itlist->has_non_vars= false;
1310+
break;
1311+
caseJOIN_RIGHT:
1312+
outer_itlist->has_non_vars= false;
1313+
break;
1314+
caseJOIN_FULL:
1315+
outer_itlist->has_non_vars= false;
1316+
inner_itlist->has_non_vars= false;
1317+
break;
1318+
default:
1319+
break;
1320+
}
1321+
1322+
join->plan.targetlist=fix_join_expr(root,
1323+
join->plan.targetlist,
1324+
outer_itlist,
1325+
inner_itlist,
1326+
(Index)0,
1327+
rtoffset);
1328+
join->plan.qual=fix_join_expr(root,
1329+
join->plan.qual,
1330+
outer_itlist,
1331+
inner_itlist,
1332+
(Index)0,
1333+
rtoffset);
1334+
12981335
pfree(outer_itlist);
12991336
pfree(inner_itlist);
13001337
}
@@ -1573,7 +1610,9 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
15731610
* If no match, return NULL.
15741611
*
15751612
* NOTE: it is a waste of time to call this unless itlist->has_ph_vars or
1576-
* itlist->has_non_vars
1613+
* itlist->has_non_vars. Furthermore, set_join_references() relies on being
1614+
* able to prevent matching of non-Vars by clearing itlist->has_non_vars,
1615+
* so there's a correctness reason not to call it unless that's set.
15771616
*/
15781617
staticVar*
15791618
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
@@ -3170,6 +3170,54 @@ explain (costs off)
31703170
Index Cond: (unique2 = 42)
31713171
(6 rows)
31723172

3173+
--
3174+
-- test that quals attached to an outer join have correct semantics,
3175+
-- specifically that they don't re-use expressions computed below the join;
3176+
-- we force a mergejoin so that coalesce(b.q1, 1) appears as a join input
3177+
--
3178+
set enable_hashjoin to off;
3179+
set enable_nestloop to off;
3180+
explain (verbose, costs off)
3181+
select a.q2, b.q1
3182+
from int8_tbl a left join int8_tbl b on a.q2 = coalesce(b.q1, 1)
3183+
where coalesce(b.q1, 1) > 0;
3184+
QUERY PLAN
3185+
-------------------------------------------------------
3186+
Merge Left Join
3187+
Output: a.q2, b.q1
3188+
Merge Cond: (a.q2 = (COALESCE(b.q1, 1::bigint)))
3189+
Filter: (COALESCE(b.q1, 1::bigint) > 0)
3190+
-> Sort
3191+
Output: a.q2
3192+
Sort Key: a.q2
3193+
-> Seq Scan on public.int8_tbl a
3194+
Output: a.q2
3195+
-> Sort
3196+
Output: b.q1, (COALESCE(b.q1, 1::bigint))
3197+
Sort Key: (COALESCE(b.q1, 1::bigint))
3198+
-> Seq Scan on public.int8_tbl b
3199+
Output: b.q1, COALESCE(b.q1, 1::bigint)
3200+
(14 rows)
3201+
3202+
select a.q2, b.q1
3203+
from int8_tbl a left join int8_tbl b on a.q2 = coalesce(b.q1, 1)
3204+
where coalesce(b.q1, 1) > 0;
3205+
q2 | q1
3206+
-------------------+------------------
3207+
-4567890123456789 |
3208+
123 | 123
3209+
123 | 123
3210+
456 |
3211+
4567890123456789 | 4567890123456789
3212+
4567890123456789 | 4567890123456789
3213+
4567890123456789 | 4567890123456789
3214+
4567890123456789 | 4567890123456789
3215+
4567890123456789 | 4567890123456789
3216+
4567890123456789 | 4567890123456789
3217+
(10 rows)
3218+
3219+
reset enable_hashjoin;
3220+
reset enable_nestloop;
31733221
--
31743222
-- test join removal
31753223
--

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

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

926+
--
927+
-- test that quals attached to an outer join have correct semantics,
928+
-- specifically that they don't re-use expressions computed below the join;
929+
-- we force a mergejoin so that coalesce(b.q1, 1) appears as a join input
930+
--
931+
932+
set enable_hashjoin to off;
933+
set enable_nestloop to off;
934+
935+
explain (verbose, costs off)
936+
selecta.q2,b.q1
937+
from int8_tbl aleft join int8_tbl bona.q2= coalesce(b.q1,1)
938+
where coalesce(b.q1,1)>0;
939+
selecta.q2,b.q1
940+
from int8_tbl aleft join int8_tbl bona.q2= coalesce(b.q1,1)
941+
where coalesce(b.q1,1)>0;
942+
943+
reset enable_hashjoin;
944+
reset enable_nestloop;
945+
926946
--
927947
-- test join removal
928948
--

0 commit comments

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