Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8ad77f0

Browse files
tglsfdcdmpgpro
authored andcommitted
Fix old corner-case logic error in final_cost_nestloop().
When costing a nestloop with stop-at-first-inner-match semantics, and a non-indexscan inner path, final_cost_nestloop() wants to charge the full scan cost of the inner rel at least once, with additional scans charged at inner_rescan_run_cost which might be less. However the logic for doing this effectively assumed that outer_matched_rows is at least 1. If it's zero, which is not unlikely for a small outer rel, we ended up charging inner_run_cost plus N times inner_rescan_run_cost, as much as double the correct charge for an outer rel with only one row that we're betting won't be matched. (Unless the inner rel is materialized, in which case it has very small inner_rescan_run_cost and the cost is not so far off what it should have been.) The upshot of this was that the planner had a tendency to select plans that failed to make effective use of the stop-at-first-inner-match semantics, and that might have Materialize nodes in them even when the predicted number of executions of the Materialize subplan was only 1. This was not so obvious before commit 9c7f522, because the case only arose in connection with semi/anti joins where there's not freedom to reverse the join order. But with the addition of unique-inner joins, it could result in some fairly bad planning choices, as reported by Teodor Sigaev. Indeed, some of the test cases added by that commit have plans that look dubious on closer inspection, and are changed by this patch. Fix the logic to ensure that we don't charge for too many inner scans. I chose to adjust it so that the full-freight scan cost is associated with an unmatched outer row if possible, not a matched one, since that seems like a better model of what would happen at runtime. This is a longstanding bug, but given the lesser impact in back branches, and the lack of field complaints, I won't risk a back-patch. Discussion: https://postgr.es/m/CAKJS1f-LzkUsFxdJ_-Luy38orQ+AdEXM5o+vANR+-pHAWPSecg@mail.gmail.com
1 parent 86242c1 commit 8ad77f0

File tree

2 files changed

+42
-38
lines changed

2 files changed

+42
-38
lines changed

src/backend/optimizer/path/costsize.c

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,6 +2027,7 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
20272027
Cost inner_run_cost = workspace->inner_run_cost;
20282028
Cost inner_rescan_run_cost = workspace->inner_rescan_run_cost;
20292029
double outer_matched_rows;
2030+
double outer_unmatched_rows;
20302031
Selectivity inner_scan_frac;
20312032

20322033
/*
@@ -2039,6 +2040,7 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
20392040
* least 1, no such clamp is needed now.)
20402041
*/
20412042
outer_matched_rows = rint(outer_path_rows * extra->semifactors.outer_match_frac);
2043+
outer_unmatched_rows = outer_path_rows - outer_matched_rows;
20422044
inner_scan_frac = 2.0 / (extra->semifactors.match_count + 1.0);
20432045

20442046
/*
@@ -2082,7 +2084,7 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
20822084
* of a nonempty scan. We consider that these are all rescans,
20832085
* since we used inner_run_cost once already.
20842086
*/
2085-
run_cost += (outer_path_rows - outer_matched_rows) *
2087+
run_cost += outer_unmatched_rows *
20862088
inner_rescan_run_cost / inner_path_rows;
20872089

20882090
/*
@@ -2100,20 +2102,28 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
21002102
* difficult to estimate whether that will happen (and it could
21012103
* not happen if there are any unmatched outer rows!), so be
21022104
* conservative and always charge the whole first-scan cost once.
2105+
* We consider this charge to correspond to the first unmatched
2106+
* outer row, unless there isn't one in our estimate, in which
2107+
* case blame it on the first matched row.
21032108
*/
2109+
2110+
/* First, count all unmatched join tuples as being processed */
2111+
ntuples += outer_unmatched_rows * inner_path_rows;
2112+
2113+
/* Now add the forced full scan, and decrement appropriate count */
21042114
run_cost += inner_run_cost;
2115+
if (outer_unmatched_rows >= 1)
2116+
outer_unmatched_rows -= 1;
2117+
else
2118+
outer_matched_rows -= 1;
21052119

21062120
/* Add inner run cost for additional outer tuples having matches */
2107-
if (outer_matched_rows > 1)
2108-
run_cost += (outer_matched_rows - 1) * inner_rescan_run_cost * inner_scan_frac;
2109-
2110-
/* Add inner run cost for unmatched outer tuples */
2111-
run_cost += (outer_path_rows - outer_matched_rows) *
2112-
inner_rescan_run_cost;
2121+
if (outer_matched_rows > 0)
2122+
run_cost += outer_matched_rows * inner_rescan_run_cost * inner_scan_frac;
21132123

2114-
/* And count the unmatched join tuples as being processed */
2115-
ntuples += (outer_path_rows - outer_matched_rows) *
2116-
inner_path_rows;
2124+
/* Add inner run cost for additional unmatched outer tuples */
2125+
if (outer_unmatched_rows > 0)
2126+
run_cost += outer_unmatched_rows * inner_rescan_run_cost;
21172127
}
21182128
}
21192129
else

src/test/regress/expected/join.out

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5472,48 +5472,44 @@ select * from j1 natural join j2;
54725472
explain (verbose, costs off)
54735473
select * from j1
54745474
inner join (select distinct id from j3) j3 on j1.id = j3.id;
5475-
QUERY PLAN
5476-
-----------------------------------------------
5475+
QUERY PLAN
5476+
-----------------------------------------
54775477
Nested Loop
54785478
Output: j1.id, j3.id
54795479
Inner Unique: true
54805480
Join Filter: (j1.id = j3.id)
5481-
-> Seq Scan on public.j1
5482-
Output: j1.id
5483-
-> Materialize
5481+
-> Unique
54845482
Output: j3.id
5485-
-> Unique
5483+
-> Sort
54865484
Output: j3.id
5487-
-> Sort
5485+
Sort Key: j3.id
5486+
-> Seq Scan on public.j3
54885487
Output: j3.id
5489-
Sort Key: j3.id
5490-
-> Seq Scan on public.j3
5491-
Output: j3.id
5492-
(15 rows)
5488+
-> Seq Scan on public.j1
5489+
Output: j1.id
5490+
(13 rows)
54935491

54945492
-- ensure group by clause allows the inner to become unique
54955493
explain (verbose, costs off)
54965494
select * from j1
54975495
inner join (select id from j3 group by id) j3 on j1.id = j3.id;
5498-
QUERY PLAN
5499-
-----------------------------------------------
5496+
QUERY PLAN
5497+
-----------------------------------------
55005498
Nested Loop
55015499
Output: j1.id, j3.id
55025500
Inner Unique: true
55035501
Join Filter: (j1.id = j3.id)
5504-
-> Seq Scan on public.j1
5505-
Output: j1.id
5506-
-> Materialize
5502+
-> Group
55075503
Output: j3.id
5508-
-> Group
5504+
Group Key: j3.id
5505+
-> Sort
55095506
Output: j3.id
5510-
Group Key: j3.id
5511-
-> Sort
5507+
Sort Key: j3.id
5508+
-> Seq Scan on public.j3
55125509
Output: j3.id
5513-
Sort Key: j3.id
5514-
-> Seq Scan on public.j3
5515-
Output: j3.id
5516-
(16 rows)
5510+
-> Seq Scan on public.j1
5511+
Output: j1.id
5512+
(14 rows)
55175513

55185514
drop table j1;
55195515
drop table j2;
@@ -5554,13 +5550,11 @@ inner join j2 on j1.id1 = j2.id1 and j1.id2 = j2.id2;
55545550
Output: j1.id1, j1.id2, j2.id1, j2.id2
55555551
Inner Unique: true
55565552
Join Filter: ((j1.id1 = j2.id1) AND (j1.id2 = j2.id2))
5553+
-> Seq Scan on public.j2
5554+
Output: j2.id1, j2.id2
55575555
-> Seq Scan on public.j1
55585556
Output: j1.id1, j1.id2
5559-
-> Materialize
5560-
Output: j2.id1, j2.id2
5561-
-> Seq Scan on public.j2
5562-
Output: j2.id1, j2.id2
5563-
(10 rows)
5557+
(8 rows)
55645558

55655559
-- ensure we don't detect the join to be unique when quals are not part of the
55665560
-- join condition

0 commit comments

Comments
 (0)