Merge branch 'release-18.0' of https://github.com/github/vitess-gh in…

…to release-18.0-github

Signed-off-by: Arthur Schreiber <arthurschreiber@github.com>

go/mysql/server_flaky_test.go

@@ -1429,7 +1429,7 @@ func TestListenerShutdown(t *testing.T) {
 
 	l.Shutdown()
 
-	assert.EqualValues(t, 1, connRefuse.Get(), "connRefuse")
+	waitForConnRefuse(t, 1)
 
 	err = conn.Ping()
 	require.EqualError(t, err, "Server shutdown in progress (errno 1053) (sqlstate 08S01)")
@@ -1441,6 +1441,24 @@ func TestListenerShutdown(t *testing.T) {
 	require.Equal(t, "Server shutdown in progress", sqlErr.Message)
 }
 
+func waitForConnRefuse(t *testing.T, valWanted int64) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	tick := time.NewTicker(100 * time.Millisecond)
+	defer tick.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			require.FailNow(t, "connRefuse did not reach %v", valWanted)
+		case <-tick.C:
+			if connRefuse.Get() == valWanted {
+				return
+			}
+		}
+	}
+}
+
 func TestParseConnAttrs(t *testing.T) {
 	expected := map[string]string{
 		"_client_version": "8.0.11",

go/test/endtoend/vtgate/queries/subquery/subquery_test.go

@@ -173,3 +173,16 @@ func TestSubqueryInAggregation(t *testing.T) {
 	// This fails as the planner adds `weight_string` method which make the query fail on MySQL.
 	// mcmp.Exec(`SELECT max((select min(id2) from t1 where t1.id1 = t.id1)) FROM t1 t`)
 }
+
+// TestSubqueryInDerivedTable tests that subqueries and derived tables
+// are handled correctly when there are joins inside the derived table
+func TestSubqueryInDerivedTable(t *testing.T) {
+	utils.SkipIfBinaryIsBelowVersion(t, 18, "vtgate")
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec("INSERT INTO t1 (id1, id2) VALUES (1, 100), (2, 200), (3, 300), (4, 400), (5, 500);")
+	mcmp.Exec("INSERT INTO t2 (id3, id4) VALUES (10, 1), (20, 2), (30, 3), (40, 4), (50, 99)")
+	mcmp.Exec(`select t.a from (select t1.id2, t2.id3, (select id2 from t1 order by id2 limit 1) as a from t1 join t2 on t1.id1 = t2.id4) t`)
+	mcmp.Exec(`SELECT COUNT(*) FROM (SELECT DISTINCT t1.id1 FROM t1 JOIN t2 ON t1.id1 = t2.id4) dt`)
+}

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;

go/vt/mysqlctl/fakemysqldaemon.go

@@ -277,13 +277,6 @@ func (fmd *FakeMysqlDaemon) GetServerUUID(ctx context.Context) (string, error) {
 	return "000000", nil
 }
 
-// CurrentPrimaryPositionLocked is thread-safe
-func (fmd *FakeMysqlDaemon) CurrentPrimaryPositionLocked(pos replication.Position) {
-	fmd.mu.Lock()
-	defer fmd.mu.Unlock()
-	fmd.CurrentPrimaryPosition = pos
-}
-
 // ReplicationStatus is part of the MysqlDaemon interface
 func (fmd *FakeMysqlDaemon) ReplicationStatus() (replication.ReplicationStatus, error) {
 	if fmd.ReplicationStatusError != nil {
@@ -307,6 +300,8 @@ func (fmd *FakeMysqlDaemon) ReplicationStatus() (replication.ReplicationStatus,
 
 // PrimaryStatus is part of the MysqlDaemon interface
 func (fmd *FakeMysqlDaemon) PrimaryStatus(ctx context.Context) (replication.PrimaryStatus, error) {
+	fmd.mu.Lock()
+	defer fmd.mu.Unlock()
 	if fmd.PrimaryStatusError != nil {
 		return replication.PrimaryStatus{}, fmd.PrimaryStatusError
 	}
@@ -372,7 +367,21 @@ func (fmd *FakeMysqlDaemon) GetPreviousGTIDs(ctx context.Context, binlog string)
 
 // PrimaryPosition is part of the MysqlDaemon interface
 func (fmd *FakeMysqlDaemon) PrimaryPosition() (replication.Position, error) {
-	return fmd.CurrentPrimaryPosition, nil
+	return fmd.GetPrimaryPositionLocked(), nil
+}
+
+// GetPrimaryPositionLocked gets the primary position while holding the lock.
+func (fmd *FakeMysqlDaemon) GetPrimaryPositionLocked() replication.Position {
+	fmd.mu.Lock()
+	defer fmd.mu.Unlock()
+	return fmd.CurrentPrimaryPosition
+}
+
+// SetPrimaryPositionLocked is thread-safe.
+func (fmd *FakeMysqlDaemon) SetPrimaryPositionLocked(pos replication.Position) {
+	fmd.mu.Lock()
+	defer fmd.mu.Unlock()
+	fmd.CurrentPrimaryPosition = pos
 }
 
 // IsReadOnly is part of the MysqlDaemon interface

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

go/vt/sqlparser/ast_funcs.go

@@ -1445,10 +1445,6 @@ func (op BinaryExprOperator) ToString() string {
 		return ShiftLeftStr
 	case ShiftRightOp:
 		return ShiftRightStr
-	case JSONExtractOp:
-		return JSONExtractOpStr
-	case JSONUnquoteExtractOp:
-		return JSONUnquoteExtractOpStr
 	default:
 		return "Unknown BinaryExprOperator"
 	}

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    = "+"
@@ -728,8 +726,6 @@ const (
 	ModOp
 	ShiftLeftOp
 	ShiftRightOp
-	JSONExtractOp
-	JSONUnquoteExtractOp
 )
 
 // Constant for Enum Type - UnaryExprOperator

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
 	}

go/vt/sqlparser/parse_test.go

@@ -1004,9 +1004,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()",
 	}, {
@@ -5685,7 +5687,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 {

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
 		}
 	}

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 {

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:

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'",

go/vt/vtctl/grpcvtctldserver/server_test.go

@@ -12030,7 +12030,7 @@ func TestTabletExternallyReparented(t *testing.T) {
 				defer func() {
 					topofactory.SetError(nil)
 
-					ctx, cancel := context.WithTimeout(ctx, time.Millisecond*10)
+					ctx, cancel := context.WithTimeout(ctx, time.Second*10)
 					defer cancel()
 
 					resp, err := vtctld.GetTablets(ctx, &vtctldatapb.GetTabletsRequest{})

go/vt/vtgate/engine/route.go

@@ -166,11 +166,12 @@ func (route *Route) SetTruncateColumnCount(count int) {
 func (route *Route) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) {
 	ctx, cancelFunc := addQueryTimeout(ctx, vcursor, route.QueryTimeout)
 	defer cancelFunc()
-	qr, err := route.executeInternal(ctx, vcursor, bindVars, wantfields)
+	rss, bvs, err := route.findRoute(ctx, vcursor, bindVars)
 	if err != nil {
 		return nil, err
 	}
-	return qr.Truncate(route.TruncateColumnCount), nil
+
+	return route.executeShards(ctx, vcursor, bindVars, wantfields, rss, bvs)
 }
 
 // addQueryTimeout adds a query timeout to the context it receives and returns the modified context along with the cancel function.
@@ -188,20 +189,6 @@ const (
 	IgnoreReserveTxn cxtKey = iota
 )
 
-func (route *Route) executeInternal(
-	ctx context.Context,
-	vcursor VCursor,
-	bindVars map[string]*querypb.BindVariable,
-	wantfields bool,
-) (*sqltypes.Result, error) {
-	rss, bvs, err := route.findRoute(ctx, vcursor, bindVars)
-	if err != nil {
-		return nil, err
-	}
-
-	return route.executeShards(ctx, vcursor, bindVars, wantfields, rss, bvs)
-}
-
 func (route *Route) executeShards(
 	ctx context.Context,
 	vcursor VCursor,
@@ -255,11 +242,15 @@ func (route *Route) executeShards(
 		}
 	}
 
-	if len(route.OrderBy) == 0 {
-		return result, nil
+	if len(route.OrderBy) != 0 {
+		var err error
+		result, err = route.sort(result)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	return route.sort(result)
+	return result.Truncate(route.TruncateColumnCount), nil
 }
 
 func filterOutNilErrors(errs []error) []error {
@@ -441,7 +432,7 @@ func (route *Route) sort(in *sqltypes.Result) (*sqltypes.Result, error) {
 		return 0
 	})
 
-	return out.Truncate(route.TruncateColumnCount), err
+	return out, err
 }
 
 func (route *Route) description() PrimitiveDescription {