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 f3b1fc10340..905d4deaa08 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 07c3421988b..290c57f9d97 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1435,10 +1435,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 1be3124aa24..bce0cb32cba 100644 --- a/go/vt/sqlparser/constants.go +++ b/go/vt/sqlparser/constants.go @@ -155,19 +155,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 = "+" @@ -718,8 +716,6 @@ const ( ModOp ShiftLeftOp ShiftRightOp - JSONExtractOp - JSONUnquoteExtractOp ) // Constant for Enum Type - UnaryExprOperator diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go index a65913b6a57..3097e0161bc 100644 --- a/go/vt/sqlparser/normalizer_test.go +++ b/go/vt/sqlparser/normalizer_test.go @@ -405,6 +405,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), + }, }} for _, tc := range testcases { t.Run(tc.in, func(t *testing.T) { @@ -491,6 +498,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/parse_test.go b/go/vt/sqlparser/parse_test.go index aa007febe9a..fa189def53c 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/go/vt/sqlparser/parse_test.go @@ -994,9 +994,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()", }, { @@ -5675,7 +5677,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\tINDEX zips ((cast(info -> '$.field' as unsigned array)))\n)", + output: "create table t (\n\tid int,\n\tinfo JSON,\n\tINDEX zips ((cast(json_extract(info, '$.field') as unsigned array)))\n)", }, } for _, test := range createTableQueries { diff --git a/go/vt/sqlparser/precedence.go b/go/vt/sqlparser/precedence.go index ec590b23f95..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. @@ -58,10 +57,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: @@ -83,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 a6cbffee351..3905212ff78 100644 --- a/go/vt/sqlparser/precedence_test.go +++ b/go/vt/sqlparser/precedence_test.go @@ -156,6 +156,9 @@ 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)"}, } for _, tc := range tests { diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go index 51d3c77cd92..9ea4d0bc595 100644 --- a/go/vt/sqlparser/sql.go +++ b/go/vt/sqlparser/sql.go @@ -17840,7 +17840,7 @@ yydefault: var yyLOCAL Expr //line sql.y:5487 { - 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 1063: @@ -17848,7 +17848,7 @@ yydefault: var yyLOCAL Expr //line sql.y:5491 { - 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 1064: diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y index a3f7a2e1a82..9cdd14cfdcf 100644 --- a/go/vt/sqlparser/sql.y +++ b/go/vt/sqlparser/sql.y @@ -5485,11 +5485,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 6924bf11911..378e386192f 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\tINDEX `zips` ((CAST(`info` -> '$.field' AS unsigned array)))\n)", + "CREATE TABLE `t` (\n\t`id` int,\n\t`info` JSON,\n\tINDEX `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/expr.go b/go/vt/vtgate/evalengine/expr.go index dfa8491391e..0be178ee149 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 b723609160c..efc5fdee04a 100644 --- a/go/vt/vtgate/evalengine/expr_compare.go +++ b/go/vt/vtgate/evalengine/expr_compare.go @@ -578,13 +578,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 @@ -593,18 +598,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 } // typeof implements the ComparisonOp interface @@ -620,12 +616,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.cfg.Collation, nil) @@ -678,6 +676,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 cd52631c00c..d25415ae674 100644 --- a/go/vt/vtgate/evalengine/testcases/cases.go +++ b/go/vt/vtgate/evalengine/testcases/cases.go @@ -1060,24 +1060,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 37a4037d21e..b379812c949 100644 --- a/go/vt/vtgate/evalengine/translate.go +++ b/go/vt/vtgate/evalengine/translate.go @@ -84,7 +84,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()) } } @@ -298,10 +298,6 @@ func (ast *astCompiler) translateBinaryExpr(binary *sqlparser.BinaryExpr) (Expr, 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 0b086021d88..2d3a82f0b9b 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -2290,7 +2290,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\"INT64(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 d8f0d09a64e..016cfdb0fd1 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -2800,8 +2800,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 ba5510c5bf5..13823e5351f 100644 --- a/go/vt/vtgate/semantics/early_rewriter_test.go +++ b/go/vt/vtgate/semantics/early_rewriter_test.go @@ -823,6 +823,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) {