From add5652d6ee414ca796e4882fd44897fa6b58ecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Taylor?= Date: Mon, 14 Oct 2024 11:01:19 +0200 Subject: [PATCH 1/2] fixes bugs around expression precedence and LIKE (#16934) Signed-off-by: Andres Taylor Signed-off-by: Manan Gupta Co-authored-by: Manan Gupta --- .../expressions/expressions.test | 30 +++++ go/vt/sqlparser/analyzer.go | 2 +- go/vt/sqlparser/ast_funcs.go | 4 - go/vt/sqlparser/constants.go | 26 ++--- go/vt/sqlparser/parse_test.go | 8 +- go/vt/sqlparser/precedence.go | 5 +- go/vt/sqlparser/precedence_test.go | 2 + go/vt/sqlparser/sql.go | 4 +- go/vt/sqlparser/sql.y | 4 +- go/vt/sqlparser/tracked_buffer_test.go | 2 +- go/vt/vtgate/evalengine/compiler_test.go | 105 ++++++++++-------- go/vt/vtgate/evalengine/expr.go | 2 +- go/vt/vtgate/evalengine/expr_compare.go | 34 +++--- go/vt/vtgate/evalengine/testcases/cases.go | 16 +-- go/vt/vtgate/evalengine/translate.go | 6 +- go/vt/vtgate/executor_test.go | 2 +- .../planbuilder/testdata/select_cases.json | 4 +- go/vt/vtgate/semantics/early_rewriter_test.go | 3 + 18 files changed, 149 insertions(+), 110 deletions(-) create mode 100644 go/test/endtoend/vtgate/vitess_tester/expressions/expressions.test diff --git a/go/test/endtoend/vtgate/vitess_tester/expressions/expressions.test b/go/test/endtoend/vtgate/vitess_tester/expressions/expressions.test new file mode 100644 index 00000000000..60c1e641463 --- /dev/null +++ b/go/test/endtoend/vtgate/vitess_tester/expressions/expressions.test @@ -0,0 +1,30 @@ +# This file contains queries that test expressions in Vitess. +# We've found a number of bugs around precedences that we want to test. +CREATE TABLE t0 +( + c1 BIT, + INDEX idx_c1 (c1) +); + +INSERT INTO t0(c1) +VALUES (''); + + +SELECT * +FROM t0; + +SELECT ((t0.c1 = 'a')) +FROM t0; + +SELECT * +FROM t0 +WHERE ((t0.c1 = 'a')); + + +SELECT (1 LIKE ('a' IS NULL)); +SELECT (NOT (1 LIKE ('a' IS NULL))); + +SELECT (~ (1 || 0)) IS NULL; + +SELECT 1 +WHERE (~ (1 || 0)) IS NULL; diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index ea0773d99cc..98b7677a1f3 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -137,7 +137,7 @@ func ASTToStatementType(stmt Statement) StatementType { // CanNormalize takes Statement and returns if the statement can be normalized. func CanNormalize(stmt Statement) bool { switch stmt.(type) { - case *Select, *Union, *Insert, *Update, *Delete, *Set, *CallProc, *Stream: // TODO: we could merge this logic into ASTrewriter + case *Select, *Union, *Insert, *Update, *Delete, *Set, *CallProc, *Stream, *VExplainStmt: // TODO: we could merge this logic into ASTrewriter return true } return false diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index b3798bbd28f..0579552751d 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1468,10 +1468,6 @@ func (op BinaryExprOperator) ToString() string { return ShiftLeftStr case ShiftRightOp: return ShiftRightStr - case JSONExtractOp: - return JSONExtractOpStr - case JSONUnquoteExtractOp: - return JSONUnquoteExtractOpStr default: return "Unknown BinaryExprOperator" } diff --git a/go/vt/sqlparser/constants.go b/go/vt/sqlparser/constants.go index 44b88384551..806d14f8588 100644 --- a/go/vt/sqlparser/constants.go +++ b/go/vt/sqlparser/constants.go @@ -160,19 +160,17 @@ const ( IsNotFalseStr = "is not false" // BinaryExpr.Operator - BitAndStr = "&" - BitOrStr = "|" - BitXorStr = "^" - PlusStr = "+" - MinusStr = "-" - MultStr = "*" - DivStr = "/" - IntDivStr = "div" - ModStr = "%" - ShiftLeftStr = "<<" - ShiftRightStr = ">>" - JSONExtractOpStr = "->" - JSONUnquoteExtractOpStr = "->>" + BitAndStr = "&" + BitOrStr = "|" + BitXorStr = "^" + PlusStr = "+" + MinusStr = "-" + MultStr = "*" + DivStr = "/" + IntDivStr = "div" + ModStr = "%" + ShiftLeftStr = "<<" + ShiftRightStr = ">>" // UnaryExpr.Operator UPlusStr = "+" @@ -727,8 +725,6 @@ const ( ModOp ShiftLeftOp ShiftRightOp - JSONExtractOp - JSONUnquoteExtractOp ) // Constant for Enum Type - UnaryExprOperator diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go index c6e2e37a1ef..1d5a00aa892 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/go/vt/sqlparser/parse_test.go @@ -1005,9 +1005,11 @@ var ( }, { input: "select /* u~ */ 1 from t where a = ~b", }, { - input: "select /* -> */ a.b -> 'ab' from t", + input: "select /* -> */ a.b -> 'ab' from t", + output: "select /* -> */ json_extract(a.b, 'ab') from t", }, { - input: "select /* -> */ a.b ->> 'ab' from t", + input: "select /* -> */ a.b ->> 'ab' from t", + output: "select /* -> */ json_unquote(json_extract(a.b, 'ab')) from t", }, { input: "select /* empty function */ 1 from t where a = b()", }, { @@ -5772,7 +5774,7 @@ partition by range (YEAR(purchased)) subpartition by hash (TO_DAYS(purchased)) }, { input: "create table t (id int, info JSON, INDEX zips((CAST(info->'$.field' AS unsigned ARRAY))))", - output: "create table t (\n\tid int,\n\tinfo JSON,\n\tkey zips ((cast(info -> '$.field' as unsigned array)))\n)", + output: "create table t (\n\tid int,\n\tinfo JSON,\n\tkey zips ((cast(json_extract(info, '$.field') as unsigned array)))\n)", }, } parser := NewTestParser() diff --git a/go/vt/sqlparser/precedence.go b/go/vt/sqlparser/precedence.go index ec590b23f95..58d5fa078ea 100644 --- a/go/vt/sqlparser/precedence.go +++ b/go/vt/sqlparser/precedence.go @@ -58,10 +58,7 @@ func precedenceFor(in Expr) Precendence { case *BetweenExpr: return P12 case *ComparisonExpr: - switch node.Operator { - case EqualOp, NotEqualOp, GreaterThanOp, GreaterEqualOp, LessThanOp, LessEqualOp, LikeOp, InOp, RegexpOp, NullSafeEqualOp: - return P11 - } + return P11 case *IsExpr: return P11 case *BinaryExpr: diff --git a/go/vt/sqlparser/precedence_test.go b/go/vt/sqlparser/precedence_test.go index 774ada31dbd..6090c436440 100644 --- a/go/vt/sqlparser/precedence_test.go +++ b/go/vt/sqlparser/precedence_test.go @@ -159,6 +159,8 @@ func TestParens(t *testing.T) { {in: "(10 - 2) - 1", expected: "10 - 2 - 1"}, {in: "10 - (2 - 1)", expected: "10 - (2 - 1)"}, {in: "0 <=> (1 and 0)", expected: "0 <=> (1 and 0)"}, + {in: "1 not like ('a' is null)", expected: "1 not like ('a' is null)"}, + {in: ":vtg1 not like (:vtg2 is null)", expected: ":vtg1 not like (:vtg2 is null)"}, } parser := NewTestParser() diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index 2e4c30eb682..41e942d9406 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -17949,7 +17949,7 @@ yydefault: var yyLOCAL Expr //line sql.y:5515 { - yyLOCAL = &BinaryExpr{Left: yyDollar[1].exprUnion(), Operator: JSONExtractOp, Right: yyDollar[3].exprUnion()} + yyLOCAL = &JSONExtractExpr{JSONDoc: yyDollar[1].exprUnion(), PathList: []Expr{yyDollar[3].exprUnion()}} } yyVAL.union = yyLOCAL case 1066: @@ -17957,7 +17957,7 @@ yydefault: var yyLOCAL Expr //line sql.y:5519 { - yyLOCAL = &BinaryExpr{Left: yyDollar[1].exprUnion(), Operator: JSONUnquoteExtractOp, Right: yyDollar[3].exprUnion()} + yyLOCAL = &JSONUnquoteExpr{JSONValue: &JSONExtractExpr{JSONDoc: yyDollar[1].exprUnion(), PathList: []Expr{yyDollar[3].exprUnion()}}} } yyVAL.union = yyLOCAL case 1067: diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index 420155020f8..4bf0e4834f6 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -5513,11 +5513,11 @@ function_call_keyword } | column_name_or_offset JSON_EXTRACT_OP text_literal_or_arg { - $$ = &BinaryExpr{Left: $1, Operator: JSONExtractOp, Right: $3} + $$ = &JSONExtractExpr{JSONDoc: $1, PathList: []Expr{$3}} } | column_name_or_offset JSON_UNQUOTE_EXTRACT_OP text_literal_or_arg { - $$ = &BinaryExpr{Left: $1, Operator: JSONUnquoteExtractOp, Right: $3} + $$ = &JSONUnquoteExpr{JSONValue: &JSONExtractExpr{JSONDoc: $1, PathList: []Expr{$3}}} } column_names_opt_paren: diff --git a/go/vt/sqlparser/tracked_buffer_test.go b/go/vt/sqlparser/tracked_buffer_test.go index 4dff65634e8..1ff5ab3bcb9 100644 --- a/go/vt/sqlparser/tracked_buffer_test.go +++ b/go/vt/sqlparser/tracked_buffer_test.go @@ -270,7 +270,7 @@ func TestCanonicalOutput(t *testing.T) { }, { "create table t (id int, info JSON, INDEX zips((CAST(info->'$.field' AS unsigned array))))", - "CREATE TABLE `t` (\n\t`id` int,\n\t`info` JSON,\n\tKEY `zips` ((CAST(`info` -> '$.field' AS unsigned array)))\n)", + "CREATE TABLE `t` (\n\t`id` int,\n\t`info` JSON,\n\tKEY `zips` ((CAST(JSON_EXTRACT(`info`, '$.field') AS unsigned array)))\n)", }, { "select 1 from t1 into outfile 'test/t1.txt'", diff --git a/go/vt/vtgate/evalengine/compiler_test.go b/go/vt/vtgate/evalengine/compiler_test.go index a9d5cb561f6..797daa4d1b1 100644 --- a/go/vt/vtgate/evalengine/compiler_test.go +++ b/go/vt/vtgate/evalengine/compiler_test.go @@ -24,6 +24,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/olekukonko/tablewriter" "vitess.io/vitess/go/mysql/collations" @@ -93,7 +95,18 @@ func (s *Tracker) String() string { return s.buf.String() } +func TestOneCase(t *testing.T) { + query := `` + if query == "" { + t.Skip("no query to test") + } + venv := vtenv.NewTestEnv() + env := evalengine.EmptyExpressionEnv(venv) + testCompilerCase(t, query, venv, nil, env) +} + func TestCompilerReference(t *testing.T) { + // This test runs a lot of queries and compares the results of the evalengine in eval mode to the results of the compiler. now := time.Now() evalengine.SystemTime = func() time.Time { return now } defer func() { evalengine.SystemTime = time.Now }() @@ -107,52 +120,11 @@ func TestCompilerReference(t *testing.T) { tc.Run(func(query string, row []sqltypes.Value) { env.Row = row - - stmt, err := venv.Parser().ParseExpr(query) - if err != nil { - // no need to test un-parseable queries - return - } - - fields := evalengine.FieldResolver(tc.Schema) - cfg := &evalengine.Config{ - ResolveColumn: fields.Column, - ResolveType: fields.Type, - Collation: collations.CollationUtf8mb4ID, - Environment: venv, - NoConstantFolding: true, - } - - converted, err := evalengine.Translate(stmt, cfg) - if err != nil { - return - } - - expected, evalErr := env.EvaluateAST(converted) total++ - - res, vmErr := env.Evaluate(converted) - if vmErr != nil { - switch { - case evalErr == nil: - t.Errorf("failed evaluation from compiler:\nSQL: %s\nError: %s", query, vmErr) - case evalErr.Error() != vmErr.Error(): - t.Errorf("error mismatch:\nSQL: %s\nError eval: %s\nError comp: %s", query, evalErr, vmErr) - default: - supported++ - } - return + testCompilerCase(t, query, venv, tc.Schema, env) + if !t.Failed() { + supported++ } - - eval := expected.String() - comp := res.String() - - if eval != comp { - t.Errorf("bad evaluation from compiler:\nSQL: %s\nEval: %s\nComp: %s", query, eval, comp) - return - } - - supported++ }) track.Add(tc.Name(), supported, total) @@ -162,6 +134,51 @@ func TestCompilerReference(t *testing.T) { t.Logf("\n%s", track.String()) } +func testCompilerCase(t *testing.T, query string, venv *vtenv.Environment, schema []*querypb.Field, env *evalengine.ExpressionEnv) { + stmt, err := venv.Parser().ParseExpr(query) + if err != nil { + // no need to test un-parseable queries + return + } + + fields := evalengine.FieldResolver(schema) + cfg := &evalengine.Config{ + ResolveColumn: fields.Column, + ResolveType: fields.Type, + Collation: collations.CollationUtf8mb4ID, + Environment: venv, + NoConstantFolding: true, + } + + converted, err := evalengine.Translate(stmt, cfg) + if err != nil { + return + } + + var expected evalengine.EvalResult + var evalErr error + assert.NotPanics(t, func() { + expected, evalErr = env.EvaluateAST(converted) + }) + var res evalengine.EvalResult + var vmErr error + assert.NotPanics(t, func() { + res, vmErr = env.Evaluate(converted) + }) + switch { + case vmErr == nil && evalErr == nil: + eval := expected.String() + comp := res.String() + assert.Equalf(t, eval, comp, "bad evaluation from compiler:\nSQL: %s\nEval: %s\nComp: %s", query, eval, comp) + case vmErr == nil: + t.Errorf("failed evaluation from evalengine:\nSQL: %s\nError: %s", query, evalErr) + case evalErr == nil: + t.Errorf("failed evaluation from compiler:\nSQL: %s\nError: %s", query, vmErr) + case evalErr.Error() != vmErr.Error(): + t.Errorf("error mismatch:\nSQL: %s\nError eval: %s\nError comp: %s", query, evalErr, vmErr) + } +} + func TestCompilerSingle(t *testing.T) { var testCases = []struct { expression string diff --git a/go/vt/vtgate/evalengine/expr.go b/go/vt/vtgate/evalengine/expr.go index 44026f97e69..b90390e1ba8 100644 --- a/go/vt/vtgate/evalengine/expr.go +++ b/go/vt/vtgate/evalengine/expr.go @@ -56,7 +56,7 @@ func (expr *BinaryExpr) arguments(env *ExpressionEnv) (eval, eval, error) { } right, err := expr.Right.eval(env) if err != nil { - return nil, nil, err + return left, nil, err } return left, right, nil } diff --git a/go/vt/vtgate/evalengine/expr_compare.go b/go/vt/vtgate/evalengine/expr_compare.go index ca4cdd75f74..8cff9cf25a9 100644 --- a/go/vt/vtgate/evalengine/expr_compare.go +++ b/go/vt/vtgate/evalengine/expr_compare.go @@ -580,13 +580,18 @@ func (l *LikeExpr) matchWildcard(left, right []byte, coll collations.ID) bool { } fullColl := colldata.Lookup(coll) wc := fullColl.Wildcard(right, 0, 0, 0) - return wc.Match(left) + return wc.Match(left) == !l.Negate } func (l *LikeExpr) eval(env *ExpressionEnv) (eval, error) { - left, right, err := l.arguments(env) - if left == nil || right == nil || err != nil { - return nil, err + left, err := l.Left.eval(env) + if err != nil || left == nil { + return left, err + } + + right, err := l.Right.eval(env) + if err != nil || right == nil { + return right, err } var col collations.TypedCollation @@ -595,18 +600,9 @@ func (l *LikeExpr) eval(env *ExpressionEnv) (eval, error) { return nil, err } - var matched bool - switch { - case typeIsTextual(left.SQLType()) && typeIsTextual(right.SQLType()): - matched = l.matchWildcard(left.(*evalBytes).bytes, right.(*evalBytes).bytes, col.Collation) - case typeIsTextual(right.SQLType()): - matched = l.matchWildcard(left.ToRawBytes(), right.(*evalBytes).bytes, col.Collation) - case typeIsTextual(left.SQLType()): - matched = l.matchWildcard(left.(*evalBytes).bytes, right.ToRawBytes(), col.Collation) - default: - matched = l.matchWildcard(left.ToRawBytes(), right.ToRawBytes(), collations.CollationBinaryID) - } - return newEvalBool(matched == !l.Negate), nil + matched := l.matchWildcard(left.ToRawBytes(), right.ToRawBytes(), col.Collation) + + return newEvalBool(matched), nil } func (expr *LikeExpr) compile(c *compiler) (ctype, error) { @@ -615,12 +611,14 @@ func (expr *LikeExpr) compile(c *compiler) (ctype, error) { return ctype{}, err } + skip1 := c.compileNullCheck1(lt) + rt, err := expr.Right.compile(c) if err != nil { return ctype{}, err } - skip := c.compileNullCheck2(lt, rt) + skip2 := c.compileNullCheck1(rt) if !lt.isTextual() { c.asm.Convert_xc(2, sqltypes.VarChar, c.collation, nil) @@ -672,6 +670,6 @@ func (expr *LikeExpr) compile(c *compiler) (ctype, error) { }) } - c.asm.jumpDestination(skip) + c.asm.jumpDestination(skip1, skip2) return ctype{Type: sqltypes.Int64, Col: collationNumeric, Flag: flagIsBoolean | flagNullable}, nil } diff --git a/go/vt/vtgate/evalengine/testcases/cases.go b/go/vt/vtgate/evalengine/testcases/cases.go index b9ae41722eb..5ef8aab78d7 100644 --- a/go/vt/vtgate/evalengine/testcases/cases.go +++ b/go/vt/vtgate/evalengine/testcases/cases.go @@ -1065,24 +1065,26 @@ func CollationOperations(yield Query) { } func LikeComparison(yield Query) { - var left = []string{ + var left = append(inputConversions, `'foobar'`, `'FOOBAR'`, `'1234'`, `1234`, `_utf8mb4 'foobar' COLLATE utf8mb4_0900_as_cs`, - `_utf8mb4 'FOOBAR' COLLATE utf8mb4_0900_as_cs`, - } - var right = append([]string{ + `_utf8mb4 'FOOBAR' COLLATE utf8mb4_0900_as_cs`) + + var right = append(left, + `NULL`, `1`, `0`, `'foo%'`, `'FOO%'`, `'foo_ar'`, `'FOO_AR'`, `'12%'`, `'12_4'`, `_utf8mb4 'foo%' COLLATE utf8mb4_0900_as_cs`, `_utf8mb4 'FOO%' COLLATE utf8mb4_0900_as_cs`, `_utf8mb4 'foo_ar' COLLATE utf8mb4_0900_as_cs`, - `_utf8mb4 'FOO_AR' COLLATE utf8mb4_0900_as_cs`, - }, left...) + `_utf8mb4 'FOO_AR' COLLATE utf8mb4_0900_as_cs`) for _, lhs := range left { for _, rhs := range right { - yield(fmt.Sprintf("%s LIKE %s", lhs, rhs), nil) + for _, op := range []string{"LIKE", "NOT LIKE"} { + yield(fmt.Sprintf("%s %s %s", lhs, op, rhs), nil) + } } } } diff --git a/go/vt/vtgate/evalengine/translate.go b/go/vt/vtgate/evalengine/translate.go index b4ced2cdd61..4952e4328fe 100644 --- a/go/vt/vtgate/evalengine/translate.go +++ b/go/vt/vtgate/evalengine/translate.go @@ -87,7 +87,7 @@ func (ast *astCompiler) translateComparisonExpr2(op sqlparser.ComparisonExprOper Negate: op == sqlparser.NotRegexpOp, }, nil default: - return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, op.ToString()) + return nil, vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, op.ToString()) } } @@ -309,10 +309,6 @@ func (ast *astCompiler) translateBinaryExpr(binary *sqlparser.BinaryExpr) (IR, e return &BitwiseExpr{BinaryExpr: binaryExpr, Op: &opBitShl{}}, nil case sqlparser.ShiftRightOp: return &BitwiseExpr{BinaryExpr: binaryExpr, Op: &opBitShr{}}, nil - case sqlparser.JSONExtractOp: - return builtinJSONExtractRewrite(left, right) - case sqlparser.JSONUnquoteExtractOp: - return builtinJSONExtractUnquoteRewrite(left, right) default: return nil, translateExprNotSupported(binary) } diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index c4d231cf651..498d87eca8c 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -2280,7 +2280,7 @@ func TestExecutorVExplain(t *testing.T) { result, err = executorExec(ctx, executor, session, "vexplain plan select 42", bindVars) require.NoError(t, err) - expected := `[[VARCHAR("{\n\t\"OperatorType\": \"Projection\",\n\t\"Expressions\": [\n\t\t\"42 as 42\"\n\t],\n\t\"Inputs\": [\n\t\t{\n\t\t\t\"OperatorType\": \"SingleRow\"\n\t\t}\n\t]\n}")]]` + expected := `[[VARCHAR("{\n\t\"OperatorType\": \"Projection\",\n\t\"Expressions\": [\n\t\t\":vtg1 as :vtg1 /* INT64 */\"\n\t],\n\t\"Inputs\": [\n\t\t{\n\t\t\t\"OperatorType\": \"SingleRow\"\n\t\t}\n\t]\n}")]]` require.Equal(t, expected, fmt.Sprintf("%v", result.Rows)) } diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index f502e162705..bf1e1950760 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2933,8 +2933,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select a -> '$[4]', a ->> '$[3]' from `user` where 1 != 1", - "Query": "select a -> '$[4]', a ->> '$[3]' from `user`", + "FieldQuery": "select json_extract(a, '$[4]'), json_unquote(json_extract(a, '$[3]')) from `user` where 1 != 1", + "Query": "select json_extract(a, '$[4]'), json_unquote(json_extract(a, '$[3]')) from `user`", "Table": "`user`" }, "TablesUsed": [ diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go index a5c16ba6b78..e9576b5a3b7 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -827,6 +827,9 @@ func TestRewriteNot(t *testing.T) { }, { sql: "select a from t1 where not a > 12", expected: "select a from t1 where a <= 12", + }, { + sql: "select (not (1 like ('a' is null)))", + expected: "select 1 not like ('a' is null) from dual", }} for _, tcase := range tcases { t.Run(tcase.sql, func(t *testing.T) { From c81dba3e2682abd2bcc611a94811a51f34b6bc42 Mon Sep 17 00:00:00 2001 From: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com> Date: Wed, 28 Aug 2024 08:40:52 +0530 Subject: [PATCH 2/2] Fix Precedence rule for Tilda op (#16649) Signed-off-by: Manan Gupta Signed-off-by: Andres Taylor Co-authored-by: Andres Taylor --- go/test/endtoend/vtgate/vitess_tester/join/join.test | 1 - go/vt/sqlparser/normalizer_test.go | 8 ++++++++ go/vt/sqlparser/precedence.go | 3 +-- go/vt/sqlparser/precedence_test.go | 1 + 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/go/test/endtoend/vtgate/vitess_tester/join/join.test b/go/test/endtoend/vtgate/vitess_tester/join/join.test index 72d79a1206e..e550145f8d5 100644 --- a/go/test/endtoend/vtgate/vitess_tester/join/join.test +++ b/go/test/endtoend/vtgate/vitess_tester/join/join.test @@ -76,4 +76,3 @@ from t1 left join (select t4.col, count(*) as count from t4 group by t4.col) t3 on t3.col = t2.id where t1.id IN (1, 2) group by t2.id, t4.col; - diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go index 15d24d9d3be..394968f2893 100644 --- a/go/vt/sqlparser/normalizer_test.go +++ b/go/vt/sqlparser/normalizer_test.go @@ -414,6 +414,13 @@ func TestNormalize(t *testing.T) { "bv2": sqltypes.Int64BindVariable(2), "bv3": sqltypes.TestBindVariable([]any{1, 2}), }, + }, { + in: "SELECT 1 WHERE (~ (1||0)) IS NULL", + outstmt: "select :bv1 /* INT64 */ from dual where ~(:bv1 /* INT64 */ or :bv2 /* INT64 */) is null", + outbv: map[string]*querypb.BindVariable{ + "bv1": sqltypes.Int64BindVariable(1), + "bv2": sqltypes.Int64BindVariable(0), + }, }} parser := NewTestParser() for _, tc := range testcases { @@ -504,6 +511,7 @@ func TestNormalizeOneCasae(t *testing.T) { err = Normalize(tree, NewReservedVars("vtg", known), bv) require.NoError(t, err) normalizerOutput := String(tree) + require.EqualValues(t, testOne.output, normalizerOutput) if normalizerOutput == "otheradmin" || normalizerOutput == "otherread" { return } diff --git a/go/vt/sqlparser/precedence.go b/go/vt/sqlparser/precedence.go index 58d5fa078ea..1b5576f65b1 100644 --- a/go/vt/sqlparser/precedence.go +++ b/go/vt/sqlparser/precedence.go @@ -38,7 +38,6 @@ const ( P14 P15 P16 - P17 ) // precedenceFor returns the precedence of an expression. @@ -80,7 +79,7 @@ func precedenceFor(in Expr) Precendence { switch node.Operator { case UPlusOp, UMinusOp: return P4 - case BangOp: + default: return P3 } } diff --git a/go/vt/sqlparser/precedence_test.go b/go/vt/sqlparser/precedence_test.go index 6090c436440..d06014369be 100644 --- a/go/vt/sqlparser/precedence_test.go +++ b/go/vt/sqlparser/precedence_test.go @@ -159,6 +159,7 @@ func TestParens(t *testing.T) { {in: "(10 - 2) - 1", expected: "10 - 2 - 1"}, {in: "10 - (2 - 1)", expected: "10 - (2 - 1)"}, {in: "0 <=> (1 and 0)", expected: "0 <=> (1 and 0)"}, + {in: "(~ (1||0)) IS NULL", expected: "~(1 or 0) is null"}, {in: "1 not like ('a' is null)", expected: "1 not like ('a' is null)"}, {in: ":vtg1 not like (:vtg2 is null)", expected: ":vtg1 not like (:vtg2 is null)"}, }