Skip to content

Commit

Permalink
sql/analyzer/optimization_rules.go: moveJoinConditionsToFilter: Fix s…
Browse files Browse the repository at this point in the history
…mall inaccuracy where computed topJoin could be wrong.

This could result in the optimization pass adding the same Filter node to
multiple places in the join tree.
  • Loading branch information
reltuk committed Sep 17, 2021
1 parent f178c6b commit 52973dc
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
3 changes: 2 additions & 1 deletion sql/analyzer/optimization_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ func moveJoinConditionsToFilter(ctx *sql.Context, a *Analyzer, n sql.Node, scope
}

if filtersMoved == 0 {
return n, nil
topJoin = n
return topJoin, nil
}

if len(condFilters) > 0 {
Expand Down
39 changes: 39 additions & 0 deletions sql/analyzer/optimization_rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,45 @@ func TestMoveJoinConditionsToFilter(t *testing.T) {
)

assertNodesEqualWithDiff(t, expected, result)

node = plan.NewInnerJoin(
plan.NewResolvedTable(t1, nil, nil),
plan.NewInnerJoin(
plan.NewResolvedTable(t2, nil, nil),
plan.NewResolvedTable(t3, nil, nil),
expression.JoinAnd(
eq(col(0, "t2", "c"), col(0, "t3", "e")),
eq(col(0, "t3", "a"), lit(5)),
),
),
expression.JoinAnd(
eq(col(0, "t1", "c"), col(0, "t2", "e")),
),
)

result, err = rule.Apply(sql.NewEmptyContext(), NewDefault(nil), node, nil)
require.NoError(err)

expected = plan.NewFilter(
expression.JoinAnd(
eq(col(0, "t3", "a"), lit(5)),
),
plan.NewInnerJoin(
plan.NewResolvedTable(t1, nil, nil),
plan.NewInnerJoin(
plan.NewResolvedTable(t2, nil, nil),
plan.NewResolvedTable(t3, nil, nil),
expression.JoinAnd(
eq(col(0, "t2", "c"), col(0, "t3", "e")),
),
),
expression.JoinAnd(
eq(col(0, "t1", "c"), col(0, "t2", "e")),
),
),
)

assertNodesEqualWithDiff(t, expected, result)
}

func TestEvalFilter(t *testing.T) {
Expand Down

0 comments on commit 52973dc

Please sign in to comment.