rewrite NOT expressions for cleaner plans

go/vt/sqlparser/analyzer.go

@@ -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.

go/vt/sqlparser/ast_format.go

@@ -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:
@@ -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:

go/vt/sqlparser/ast_rewriting.go

@@ -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
@@ -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)

go/vt/sqlparser/ast_rewriting_test.go

@@ -214,6 +214,18 @@ func TestRewrites(in *testing.T) {
 		in:       "CALL proc(@foo)",
 		expected: "CALL proc(:__vtudvfoo)",
 		udv:      1,
+	}, {
+		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 = 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 true",
+		expected: "SELECT * FROM tbl WHERE false",
 	}, {
 		in:                          "SHOW VARIABLES",
 		expected:                    "SHOW VARIABLES",

go/vt/vtgate/planbuilder/testdata/filter_cases.txt

@@ -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"
@@ -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"
@@ -2444,7 +2444,7 @@ Gen4 plan same as above
       },
       {
         "OperatorType": "Subquery",
-        "Variant": "PulloutIn",
+        "Variant": "PulloutNotIn",
         "PulloutVars": [
           "__sq_has_values1",
           "__sq1"
@@ -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"
@@ -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
@@ -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
@@ -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"
@@ -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
@@ -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"
@@ -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