Skip to content

Commit

Permalink
Merge pull request #8987 from planetscale/rewrite-nots
Browse files Browse the repository at this point in the history 
rewrite NOT expressions for cleaner plans
  • Loading branch information
@systay
systay authored Oct 13, 2021
2 parents 90c2edb + beb7e66 commit b33fa76
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 55 deletions.
54 changes: 11 additions & 43 deletions go/vt/sqlparser/analyzer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,67 +338,35 @@ func SplitAndExpression(filters []Expr, node Expr) []Expr {
return append(filters, node)
}

// SplitOrExpression breaks up the Expr into OR-separated conditions
// and appends them to filters. Outer parenthesis are removed. Precedence
// should be taken into account if expressions are recombined.
func SplitOrExpression(filters []Expr, node Expr) []Expr {
if node == nil {
return filters
}
switch node := node.(type) {
case *OrExpr:
filters = SplitOrExpression(filters, node.Left)
return SplitOrExpression(filters, node.Right)
}
return append(filters, node)
}

// joinExpressions join together a list of Expr using the baseType as operator (either AndExpr or OrExpr).
func joinExpressions(baseType Expr, exprs ...Expr) Expr {
// AndExpressions ands together two or more expressions, minimising the expr when possible
func AndExpressions(exprs ...Expr) Expr {
switch len(exprs) {
case 0:
return nil
case 1:
return exprs[0]
default:
result := (Expr)(nil)
outer:
// we'll loop and remove any duplicates
for i, expr := range exprs {
if expr == nil {
continue
}
if result == nil {
result = expr
} else {
found := false
for j := 0; j < i; j++ {
if EqualsExpr(expr, exprs[j]) {
found = true
break
}
}
if !found {
switch baseType.(type) {
case *AndExpr:
result = &AndExpr{Left: result, Right: expr}
case *OrExpr:
result = &OrExpr{Left: result, Right: expr}
}
continue outer
}

for j := 0; j < i; j++ {
if EqualsExpr(expr, exprs[j]) {
continue outer
}
}
result = &AndExpr{Left: result, Right: expr}
}
return result
}

}

// AndExpressions ands together two or more expressions, minimising the expr when possible
func AndExpressions(exprs ...Expr) Expr {
return joinExpressions(&AndExpr{}, exprs...)
}

// OrExpressions ors together two or more expressions, minimising the expr when possible
func OrExpressions(exprs ...Expr) Expr {
return joinExpressions(&OrExpr{}, exprs...)
}

// TableFromStatement returns the qualified table name for the query.
Expand Down
6 changes: 5 additions & 1 deletion go/vt/sqlparser/ast_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1721,6 +1721,10 @@ func (node *RenameTable) Format(buf *TrackedBuffer) {
}
}

// Format formats the node.
// If an extracted subquery is still in the AST when we print it,
// it will be formatted as if the subquery has been extracted, and instead
// show up like argument comparisons
func (node *ExtractedSubquery) Format(buf *TrackedBuffer) {
switch original := node.Original.(type) {
case *ExistsExpr:
Expand All @@ -1743,7 +1747,7 @@ func (node *ExtractedSubquery) Format(buf *TrackedBuffer) {
// :__sq_has_values = 0 or other_side not in ::__sq
cmp.Right = NewListArg(node.ArgName)
hasValue := &ComparisonExpr{Left: NewArgument(node.HasValuesArg), Right: NewIntLiteral("0"), Operator: EqualOp}
expr = OrExpressions(hasValue, cmp)
expr = &OrExpr{hasValue, cmp}
}
buf.astPrintf(node, "%v", expr)
case *Subquery:
Expand Down
6 changes: 5 additions & 1 deletion go/vt/sqlparser/ast_format_fast.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions go/vt/sqlparser/ast_rewriting.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,25 @@ func (er *expressionRewriter) rewrite(cursor *Cursor) bool {
er.unnestSubQueries(cursor, node)
case *JoinCondition:
er.rewriteJoinCondition(cursor, node)
case *NotExpr:
switch inner := node.Expr.(type) {
case *ComparisonExpr:
// not col = 42 => col != 42
// not col > 42 => col <= 42
// etc
canChange, inverse := inverseOp(inner.Operator)
if canChange {
inner.Operator = inverse
cursor.Replace(inner)
}
case *NotExpr:
// not not true => true
cursor.Replace(inner.Expr)
case BoolVal:
// not true => false
inner = !inner
cursor.Replace(inner)
}
case *AliasedTableExpr:
if !SystemSchema(er.keyspace) {
break
Expand Down Expand Up @@ -367,6 +386,37 @@ func (er *expressionRewriter) rewrite(cursor *Cursor) bool {
return true
}

func inverseOp(i ComparisonExprOperator) (bool, ComparisonExprOperator) {
switch i {
case EqualOp:
return true, NotEqualOp
case LessThanOp:
return true, GreaterEqualOp
case GreaterThanOp:
return true, LessEqualOp
case LessEqualOp:
return true, GreaterThanOp
case GreaterEqualOp:
return true, LessThanOp
case NotEqualOp:
return true, EqualOp
case InOp:
return true, NotInOp
case NotInOp:
return true, InOp
case LikeOp:
return true, NotLikeOp
case NotLikeOp:
return true, LikeOp
case RegexpOp:
return true, NotRegexpOp
case NotRegexpOp:
return true, RegexpOp
}

return false, i
}

func (er *expressionRewriter) rewriteJoinCondition(cursor *Cursor, node *JoinCondition) {
if node.Using != nil && !er.hasStarInSelect {
joinTableExpr, ok := cursor.Parent().(*JoinTableExpr)
Expand Down
39 changes: 39 additions & 0 deletions go/vt/sqlparser/ast_rewriting_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,45 @@ func TestRewrites(in *testing.T) {
in: "CALL proc(@foo)",
expected: "CALL proc(:__vtudvfoo)",
udv: 1,
}, {
in: "SELECT * FROM tbl WHERE NOT id = 42",
expected: "SELECT * FROM tbl WHERE id != 42",
}, {
in: "SELECT * FROM tbl WHERE not id < 12",
expected: "SELECT * FROM tbl WHERE id >= 12",
}, {
in: "SELECT * FROM tbl WHERE not id > 12",
expected: "SELECT * FROM tbl WHERE id <= 12",
}, {
in: "SELECT * FROM tbl WHERE not id <= 33",
expected: "SELECT * FROM tbl WHERE id > 33",
}, {
in: "SELECT * FROM tbl WHERE not id >= 33",
expected: "SELECT * FROM tbl WHERE id < 33",
}, {
in: "SELECT * FROM tbl WHERE not id != 33",
expected: "SELECT * FROM tbl WHERE id = 33",
}, {
in: "SELECT * FROM tbl WHERE not id in (1,2,3)",
expected: "SELECT * FROM tbl WHERE id not in (1,2,3)",
}, {
in: "SELECT * FROM tbl WHERE not id not in (1,2,3)",
expected: "SELECT * FROM tbl WHERE id in (1,2,3)",
}, {
in: "SELECT * FROM tbl WHERE not id not in (1,2,3)",
expected: "SELECT * FROM tbl WHERE id in (1,2,3)",
}, {
in: "SELECT * FROM tbl WHERE not id like '%foobar'",
expected: "SELECT * FROM tbl WHERE id not like '%foobar'",
}, {
in: "SELECT * FROM tbl WHERE not id not like '%foobar'",
expected: "SELECT * FROM tbl WHERE id like '%foobar'",
}, {
in: "SELECT * FROM tbl WHERE not id regexp '%foobar'",
expected: "SELECT * FROM tbl WHERE id not regexp '%foobar'",
}, {
in: "SELECT * FROM tbl WHERE not id not regexp '%foobar'",
expected: "SELECT * FROM tbl WHERE id regexp '%foobar'",
}, {
in: "SHOW VARIABLES",
expected: "SHOW VARIABLES",
Expand Down
20 changes: 10 additions & 10 deletions go/vt/vtgate/planbuilder/testdata/filter_cases.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2352,7 +2352,7 @@ Gen4 plan same as above
"Original": "select id from user where not id in (select user_extra.col from user_extra where user_extra.user_id = 42) and id in (select user_extra.col from user_extra where user_extra.user_id = 411)",
"Instructions": {
"OperatorType": "Subquery",
"Variant": "PulloutIn",
"Variant": "PulloutNotIn",
"PulloutVars": [
"__sq_has_values2",
"__sq2"
Expand Down Expand Up @@ -2404,7 +2404,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals and not (:__sq_has_values2 = 1 and id in ::__sq2)",
"Query": "select id from `user` where :__sq_has_values1 = 1 and id in ::__vals and (:__sq_has_values2 = 0 or id not in ::__sq2)",
"Table": "`user`",
"Values": [
"::__sq1"
Expand Down Expand Up @@ -2444,7 +2444,7 @@ Gen4 plan same as above
},
{
"OperatorType": "Subquery",
"Variant": "PulloutIn",
"Variant": "PulloutNotIn",
"PulloutVars": [
"__sq_has_values1",
"__sq1"
Expand Down Expand Up @@ -2473,7 +2473,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where not (:__sq_has_values1 = 1 and id in ::__sq1) and (:__sq_has_values2 = 1 and id in ::__vals)",
"Query": "select id from `user` where :__sq_has_values1 = 0 or id not in ::__sq1 and (:__sq_has_values2 = 1 and id in ::__vals)",
"Table": "`user`",
"Values": [
"::__sq2"
Expand Down Expand Up @@ -2901,7 +2901,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where id = 5 and :__sq_has_values1 = 1 and id in ::__sq1 and not id in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Query": "select id from `user` where id = 5 and :__sq_has_values1 = 1 and id in ::__sq1 and id not in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Table": "`user`",
"Values": [
5
Expand Down Expand Up @@ -2945,7 +2945,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 5) and (:__sq_has_values2 = 1 and id in ::__sq2)",
"Query": "select id from `user` where id = 5 and id not in (select user_extra.col from user_extra where user_extra.user_id = 5) and (:__sq_has_values2 = 1 and id in ::__sq2)",
"Table": "`user`",
"Values": [
5
Expand All @@ -2963,7 +2963,7 @@ Gen4 plan same as above
"Original": "select id from user where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 4) and id in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Instructions": {
"OperatorType": "Subquery",
"Variant": "PulloutIn",
"Variant": "PulloutNotIn",
"PulloutVars": [
"__sq_has_values1",
"__sq1"
Expand Down Expand Up @@ -2992,7 +2992,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where id = 5 and id in (select user_extra.col from user_extra where user_extra.user_id = 5) and not (:__sq_has_values1 = 1 and id in ::__sq1)",
"Query": "select id from `user` where id = 5 and id in (select user_extra.col from user_extra where user_extra.user_id = 5) and (:__sq_has_values1 = 0 or id not in ::__sq1)",
"Table": "`user`",
"Values": [
5
Expand All @@ -3007,7 +3007,7 @@ Gen4 plan same as above
"Original": "select id from user where id = 5 and not id in (select user_extra.col from user_extra where user_extra.user_id = 4) and id in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Instructions": {
"OperatorType": "Subquery",
"Variant": "PulloutIn",
"Variant": "PulloutNotIn",
"PulloutVars": [
"__sq_has_values1",
"__sq1"
Expand Down Expand Up @@ -3036,7 +3036,7 @@ Gen4 plan same as above
"Sharded": true
},
"FieldQuery": "select id from `user` where 1 != 1",
"Query": "select id from `user` where id = 5 and not (:__sq_has_values1 = 1 and id in ::__sq1) and id in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Query": "select id from `user` where id = 5 and :__sq_has_values1 = 0 or id not in ::__sq1 and id in (select user_extra.col from user_extra where user_extra.user_id = 5)",
"Table": "`user`",
"Values": [
5
Expand Down

0 comments on commit b33fa76

Please sign in to comment.