Skip to content

Commit

Permalink
opt: don't drop LeftJoin filter during join ordering
Browse files Browse the repository at this point in the history
This patch fixes a bug in the join reordering logic that can lead to
incorrect results due to a dropped filter and incorrect conversion of
a left join to an inner join. The bug can occur when the join tree
contains an inner join with a left join as an input, where the inner
join has two separate conjuncts in its ON condition that reference
both inputs of the left join. Additionally, the inner join filters
must not filter NULL values from the right side of the left join
(or alternatively null-rejection rules must be disabled).

The incorrect transformation looks something like this:
```
(INNER JOIN xy (LEFT JOIN ab (INNER JOIN uv wz ON v = w) ON b = v) ON a = x AND u = x)
```
=>
```
(INNER JOIN ab (INNER JOIN xy (INNER JOIN uv wz ON v = w) ON u = x) ON a = x)
```
Notice how `xy` has been "pushed" into the right side of the left
join and the left join's `b = v` filter (and the left join itself)
dropped in the process.

To understand what causes the bug, it is necessary to understand three
points about the join reordering algorithm:
1. Cross products are never introduced in the enumerated plans. So, for
   two sub-plans, a join is only considered between them if there is an
   applicable edge between those sub-plans.
2. The original paper associates each join with exactly one edge in the
   hypergraph that encodes "reorderability" properties.
3. The `JoinOrderBuilder` departs from the paper by associating each
   inner join *conjunct* with a hypergraph edge. This allows each
   conjunct to be independently reordered from the others. See the
   `Special handling of inner joins` section in the `JoinOrderBuilder`
   comment for more details.

(1) combined with (2) implies that a reordered join tree is only
considered if every edge in the hypergraph could be applied to form joins
in the join tree. This allows the original algorithm to prevent invalid
orderings by making just a single edge inapplicable. However, because
of (3) the same is no longer true for the `JoinOrderBuilder`. In the
example given above, the left join fails the applicability check,
indicating an invalid plan. However, the inner join's `a = x` filter
passes the check and ends up replacing the left join. This prevents
the the check in (1) from catching the invalid plan.

This patch fixes the bug by keeping track of the edges that *should*
be applied somewhere in each join tree based on the TES of each edge.
This is then compared against the actual edges that are applied in
the construction of the join tree. If the edge sets aren't equal,
the plan is invalid and cannot be added to the memo. This allows the
`JoinOrderBuilder` to recover the property that an inapplicable edge
invalidates an enumerated plan.

Fixes cockroachdb#90761

Release note (bug fix): Fixed a bug existing since 20.2 that could
cause incorrect results in rare cases for queries with inner joins
and left joins. For the bug to occur, the left join had to be in
the input of the inner join and the inner join filters had to
reference both inputs of the left join, and not filter NULL values
from the right input of the left join. Additionally, the right input
of the left join had to contain at least one join, with one input not
referenced by the left join's ON condition.
  • Loading branch information
DrewKimball committed Nov 24, 2022
1 parent fd8530c commit 4f4e4e3
Show file tree
Hide file tree
Showing 3 changed files with 370 additions and 135 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/opt/testutils/opttester/reorder_joins.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ import (
func (ot *OptTester) ReorderJoins() (string, error) {
ot.builder.Reset()
o := ot.makeOptimizer()
o.NotifyOnMatchedRule(func(ruleName opt.RuleName) bool {
return !ot.Flags.DisableRules.Contains(int(ruleName))
})
jof := newJoinOrderFormatter(o)

// joinsConsidered counts the number of joins which joinOrderBuilder attempts
Expand Down
Loading

0 comments on commit 4f4e4e3

Please sign in to comment.