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

Commitca68053

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 parentc67a86f commitca68053

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
@@ -1283,19 +1283,13 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
12831283
outer_itlist=build_tlist_index(outer_plan->targetlist);
12841284
inner_itlist=build_tlist_index(inner_plan->targetlist);
12851285

1286-
/* All join plans have tlist, qual, and joinqual */
1287-
join->plan.targetlist=fix_join_expr(root,
1288-
join->plan.targetlist,
1289-
outer_itlist,
1290-
inner_itlist,
1291-
(Index)0,
1292-
rtoffset);
1293-
join->plan.qual=fix_join_expr(root,
1294-
join->plan.qual,
1295-
outer_itlist,
1296-
inner_itlist,
1297-
(Index)0,
1298-
rtoffset);
1286+
/*
1287+
* First process the joinquals (including merge or hash clauses). These
1288+
* are logically below the join so they can always use all values
1289+
* available from the input tlists. It's okay to also handle
1290+
* NestLoopParams now, because those couldn't refer to nullable
1291+
* subexpressions.
1292+
*/
12991293
join->joinqual=fix_join_expr(root,
13001294
join->joinqual,
13011295
outer_itlist,
@@ -1347,6 +1341,49 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
13471341
rtoffset);
13481342
}
13491343

1344+
/*
1345+
* Now we need to fix up the targetlist and qpqual, which are logically
1346+
* above the join. This means they should not re-use any input expression
1347+
* that was computed in the nullable side of an outer join. Vars and
1348+
* PlaceHolderVars are fine, so we can implement this restriction just by
1349+
* clearing has_non_vars in the indexed_tlist structs.
1350+
*
1351+
* XXX This is a grotty workaround for the fact that we don't clearly
1352+
* distinguish between a Var appearing below an outer join and the "same"
1353+
* Var appearing above it. If we did, we'd not need to hack the matching
1354+
* rules this way.
1355+
*/
1356+
switch (join->jointype)
1357+
{
1358+
caseJOIN_LEFT:
1359+
caseJOIN_SEMI:
1360+
caseJOIN_ANTI:
1361+
inner_itlist->has_non_vars= false;
1362+
break;
1363+
caseJOIN_RIGHT:
1364+
outer_itlist->has_non_vars= false;
1365+
break;
1366+
caseJOIN_FULL:
1367+
outer_itlist->has_non_vars= false;
1368+
inner_itlist->has_non_vars= false;
1369+
break;
1370+
default:
1371+
break;
1372+
}
1373+
1374+
join->plan.targetlist=fix_join_expr(root,
1375+
join->plan.targetlist,
1376+
outer_itlist,
1377+
inner_itlist,
1378+
(Index)0,
1379+
rtoffset);
1380+
join->plan.qual=fix_join_expr(root,
1381+
join->plan.qual,
1382+
outer_itlist,
1383+
inner_itlist,
1384+
(Index)0,
1385+
rtoffset);
1386+
13501387
pfree(outer_itlist);
13511388
pfree(inner_itlist);
13521389
}
@@ -1625,7 +1662,9 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
16251662
* If no match, return NULL.
16261663
*
16271664
* NOTE: it is a waste of time to call this unless itlist->has_ph_vars or
1628-
* itlist->has_non_vars
1665+
* itlist->has_non_vars. Furthermore, set_join_references() relies on being
1666+
* able to prevent matching of non-Vars by clearing itlist->has_non_vars,
1667+
* so there's a correctness reason not to call it unless that's set.
16291668
*/
16301669
staticVar*
16311670
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
@@ -3176,6 +3176,54 @@ explain (costs off)
31763176
Index Cond: (unique2 = 42)
31773177
(6 rows)
31783178

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

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

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

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

0 commit comments

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