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

Commit 244f649

Browse files
committed
Fix an old corner-case error in match_unsorted_outer(): don't consider
the cheapest-total inner path as a new candidate while truncating the sort key list, if it already matched the full sort key list. This is too much of a corner case to be worth back-patching, since it's unusual for the cheapest total path to be sorted, and anyway no real harm is done (except in JOIN_SEMI/ANTI cases where cost_mergejoin is a bit broken at the moment). But it wasn't behaving as intended, so fix it. Noted while examining a test case from Kevin Grittner. This error doesn't explain his issue, but it does explain why "set enable_seqscan = off" seemed to reproduce it for me.
1 parent d1cf27a commit 244f649

File tree

1 file changed

+33
-12
lines changed

1 file changed

+33
-12
lines changed

src/backend/optimizer/path/joinpath.c

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/joinpath.c,v 1.120 2009/01/01 17:23:43 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/joinpath.c,v 1.121 2009/02/05 01:24:55 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -593,19 +593,44 @@ match_unsorted_outer(PlannerInfo *root,
593593
* Look for presorted inner paths that satisfy the innersortkey list
594594
* --- or any truncation thereof, if we are allowed to build a
595595
* mergejoin using a subset of the merge clauses. Here, we consider
596-
* both cheap startup cost and cheap total cost. We can ignore
597-
* inner_cheapest_total on the first iteration, since we already made
598-
* a path with it --- but not on later iterations with shorter sort
599-
* keys, because then we are considering a different situation, viz
600-
* using a simpler mergejoin to avoid a sort of the inner rel.
596+
* both cheap startup cost and cheap total cost.
597+
*
598+
* As we shorten the sortkey list, we should consider only paths that
599+
* are strictly cheaper than (in particular, not the same as) any path
600+
* found in an earlier iteration. Otherwise we'd be intentionally
601+
* using fewer merge keys than a given path allows (treating the rest
602+
* as plain joinquals), which is unlikely to be a good idea. Also,
603+
* eliminating paths here on the basis of compare_path_costs is a lot
604+
* cheaper than building the mergejoin path only to throw it away.
605+
*
606+
* If inner_cheapest_total is well enough sorted to have not required
607+
* a sort in the path made above, we shouldn't make a duplicate path
608+
* with it, either. We handle that case with the same logic that
609+
* handles the previous consideration, by initializing the variables
610+
* that track cheapest-so-far properly. Note that we do NOT reject
611+
* inner_cheapest_total if we find it matches some shorter set of
612+
* pathkeys. That case corresponds to using fewer mergekeys to avoid
613+
* sorting inner_cheapest_total, whereas we did sort it above, so the
614+
* plans being considered are different.
601615
*/
616+
if (pathkeys_contained_in(innersortkeys,
617+
inner_cheapest_total->pathkeys))
618+
{
619+
/* inner_cheapest_total didn't require a sort */
620+
cheapest_startup_inner = inner_cheapest_total;
621+
cheapest_total_inner = inner_cheapest_total;
622+
}
623+
else
624+
{
625+
/* it did require a sort, at least for the full set of keys */
626+
cheapest_startup_inner = NULL;
627+
cheapest_total_inner = NULL;
628+
}
602629
num_sortkeys = list_length(innersortkeys);
603630
if (num_sortkeys > 1 && !useallclauses)
604631
trialsortkeys = list_copy(innersortkeys); /* need modifiable copy */
605632
else
606633
trialsortkeys = innersortkeys; /* won't really truncate */
607-
cheapest_startup_inner = NULL;
608-
cheapest_total_inner = NULL;
609634

610635
for (sortkeycnt = num_sortkeys; sortkeycnt > 0; sortkeycnt--)
611636
{
@@ -622,8 +647,6 @@ match_unsorted_outer(PlannerInfo *root,
622647
trialsortkeys,
623648
TOTAL_COST);
624649
if (innerpath != NULL &&
625-
(innerpath != inner_cheapest_total ||
626-
sortkeycnt < num_sortkeys) &&
627650
(cheapest_total_inner == NULL ||
628651
compare_path_costs(innerpath, cheapest_total_inner,
629652
TOTAL_COST) < 0))
@@ -660,8 +683,6 @@ match_unsorted_outer(PlannerInfo *root,
660683
trialsortkeys,
661684
STARTUP_COST);
662685
if (innerpath != NULL &&
663-
(innerpath != inner_cheapest_total ||
664-
sortkeycnt < num_sortkeys) &&
665686
(cheapest_startup_inner == NULL ||
666687
compare_path_costs(innerpath, cheapest_startup_inner,
667688
STARTUP_COST) < 0))

0 commit comments

Comments
 (0)