slack-15.0: backport vitessio/vitess#16154

go/test/endtoend/utils/cmp.go

@@ -30,12 +30,17 @@ import (
 	"vitess.io/vitess/go/test/utils"
 )
 
+type TestingT interface {
+	require.TestingT
+	Helper()
+}
+
 type MySQLCompare struct {
-	t                 *testing.T
+	t                 TestingT
 	MySQLConn, VtConn *mysql.Conn
 }
 
-func NewMySQLCompare(t *testing.T, vtParams, mysqlParams mysql.ConnParams) (MySQLCompare, error) {
+func NewMySQLCompare(t TestingT, vtParams, mysqlParams mysql.ConnParams) (MySQLCompare, error) {
 	ctx := context.Background()
 	vtConn, err := mysql.Connect(ctx, &vtParams)
 	if err != nil {
@@ -54,6 +59,10 @@ func NewMySQLCompare(t *testing.T, vtParams, mysqlParams mysql.ConnParams) (MySQ
 	}, nil
 }
 
+func (mcmp *MySQLCompare) AsT() *testing.T {
+	return mcmp.t.(*testing.T)
+}
+
 func (mcmp *MySQLCompare) Close() {
 	mcmp.VtConn.Close()
 	mcmp.MySQLConn.Close()

go/test/endtoend/utils/mysql.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"testing"
 
 	"github.com/stretchr/testify/assert"
 
@@ -132,16 +131,16 @@ func prepareMySQLWithSchema(params mysql.ConnParams, sql string) error {
 	return nil
 }
 
-func compareVitessAndMySQLResults(t *testing.T, query string, vtQr, mysqlQr *sqltypes.Result, compareColumns bool) {
+func compareVitessAndMySQLResults(t TestingT, query string, vtQr, mysqlQr *sqltypes.Result, compareColumns bool) {
 	if vtQr == nil && mysqlQr == nil {
 		return
 	}
 	if vtQr == nil {
-		t.Error("Vitess result is 'nil' while MySQL's is not.")
+		t.Errorf("Vitess result is 'nil' while MySQL's is not.")
 		return
 	}
 	if mysqlQr == nil {
-		t.Error("MySQL result is 'nil' while Vitess' is not.")
+		t.Errorf("MySQL result is 'nil' while Vitess' is not.")
 		return
 	}
 	if compareColumns {
@@ -163,7 +162,7 @@ func compareVitessAndMySQLResults(t *testing.T, query string, vtQr, mysqlQr *sql
 	}
 	stmt, err := sqlparser.Parse(query)
 	if err != nil {
-		t.Error(err)
+		t.Errorf(err.Error())
 		return
 	}
 	orderBy := false
@@ -185,13 +184,12 @@ func compareVitessAndMySQLResults(t *testing.T, query string, vtQr, mysqlQr *sql
 	for _, row := range mysqlQr.Rows {
 		errStr += fmt.Sprintf("%s\n", row)
 	}
-	t.Error(errStr)
+	t.Errorf(errStr)
 }
 
-func compareVitessAndMySQLErrors(t *testing.T, vtErr, mysqlErr error) {
+func compareVitessAndMySQLErrors(t TestingT, vtErr, mysqlErr error) {
 	if vtErr != nil && mysqlErr != nil || vtErr == nil && mysqlErr == nil {
 		return
 	}
-	out := fmt.Sprintf("Vitess and MySQL are not erroring the same way.\nVitess error: %v\nMySQL error: %v", vtErr, mysqlErr)
-	t.Error(out)
+	t.Errorf("Vitess and MySQL are not erroring the same way.\nVitess error: %v\nMySQL error: %v", vtErr, mysqlErr)
 }

go/test/endtoend/utils/utils.go

@@ -138,7 +138,7 @@ func ExecCompareMySQL(t *testing.T, vtConn, mysqlConn *mysql.Conn, query string)
 
 // ExecAllowError executes the given query without failing the test if it produces
 // an error. The error is returned to the client, along with the result set.
-func ExecAllowError(t *testing.T, conn *mysql.Conn, query string) (*sqltypes.Result, error) {
+func ExecAllowError(t TestingT, conn *mysql.Conn, query string) (*sqltypes.Result, error) {
 	t.Helper()
 	return conn.ExecuteFetch(query, 1000, true)
 }

go/test/endtoend/vtgate/gen4/gen4_test.go

@@ -495,3 +495,15 @@ func TestFilterOnLeftOuterJoin(t *testing.T) {
 
 	mcmp.AssertMatches(query, "[[INT32(22)] [INT32(33)]]")
 }
+
+func BenchmarkCountStar(b *testing.B) {
+	mcmp, closer := start(b)
+	defer closer()
+
+	// insert some data.
+	mcmp.Exec(`insert into t2(id, tcol1, tcol2) values (1, 'A%B', 'A%B'),(2, 'C_D', 'E'),(3, 'AB', 'C1D'),(4, 'E', 'A%B'),(5, 'A%B', 'AB'),(6, 'C1D', 'E'),(7, 'C_D', 'A%B'),(8, 'E', 'C_D')`)
+
+	for i := 0; i < b.N; i++ {
+		mcmp.Exec(`select Count(*) from t2`)
+	}
+}

go/test/endtoend/vtgate/gen4/main_test.go

@@ -135,7 +135,7 @@ func TestMain(m *testing.M) {
 	os.Exit(exitCode)
 }
 
-func start(t *testing.T) (utils.MySQLCompare, func()) {
+func start(t testing.TB) (utils.MySQLCompare, func()) {
 	mcmp, err := utils.NewMySQLCompare(t, vtParams, mysqlParams)
 	require.NoError(t, err)
 	deleteAll := func() {

go/vt/sqlparser/ast.go

@@ -2707,6 +2707,34 @@ type (
 	}
 
 	CountStar struct {
+		Name string
+		_    bool
+		// TL;DR; This makes sure that reference equality checks works as expected
+		//
+		// You're correct that this might seem a bit strange at first glance.
+		// It's a quirk of Go's handling of empty structs. In Go, two instances of an empty struct are considered
+		// identical, which can be problematic when using these as keys in maps.
+		// They would be treated as the same key and potentially lead to incorrect map behavior.
+		//
+		// Here's a brief example:
+		//
+		// ```golang
+		// func TestWeirdGo(t *testing.T) {
+		// 	type CountStar struct{}
+		//
+		// 	cs1 := &CountStar{}
+		// 	cs2 := &CountStar{}
+		//  if cs1 == cs2 {
+		// 	  panic("what the what!?")
+		//  }
+		// }
+		// ```
+		//
+		// In the above code, cs1 and cs2, despite being distinct variables, would be treated as the same object.
+		//
+		// The solution we employed was to add a dummy field `_ bool` to the otherwise empty struct `CountStar`.
+		// This ensures that each instance of `CountStar` is treated as a separate object,
+		// even in the context of out semantic state which uses these objects as map keys.
 	}
 
 	Avg struct {

go/vt/sqlparser/ast_format.go

@@ -2656,8 +2656,7 @@ func (node *Count) Format(buf *TrackedBuffer) {
 }
 
 func (node *CountStar) Format(buf *TrackedBuffer) {
-	buf.astPrintf(node, "%s(", node.AggrName())
-	buf.WriteString("*)")
+	buf.astPrintf(node, "%s(*)", node.Name)
 }
 
 func (node *Avg) Format(buf *TrackedBuffer) {

go/vt/sqlparser/parse_test.go

@@ -5294,8 +5294,8 @@ func TestOne(t *testing.T) {
 	testOne := struct {
 		input, output string
 	}{
-		input:  "",
-		output: "",
+		input:  "Select Count(*) from t",
+		output: "select Count(*) from t",
 	}
 	if testOne.input == "" {
 		return

go/vt/sqlparser/sql.y

@@ -5755,7 +5755,7 @@ UTC_DATE func_paren_opt
   }
 | COUNT openb '*' closeb
   {
-    $$ = &CountStar{}
+    $$ = &CountStar{Name: $1}
   }
 | COUNT openb distinct_opt expression_list closeb
   {

go/vt/vtgate/planbuilder/select.go

@@ -301,7 +301,7 @@ func buildSQLCalcFoundRowsPlan(
 	sel2.Limit = nil
 
 	countStartExpr := []sqlparser.SelectExpr{&sqlparser.AliasedExpr{
-		Expr: &sqlparser.CountStar{},
+		Expr: &sqlparser.CountStar{Name: "count"},
 	}}
 	if sel2.GroupBy == nil && sel2.Having == nil {
 		// if there is no grouping, we can use the same query and

go/vt/vtgate/planbuilder/testdata/onecase.json

@@ -1,9 +1,68 @@
 [
   {
     "comment": "Add your test case here for debugging and run go test -run=One.",
-    "query": "",
+    "query": "select Count(*) from user join user_extra where user.intcol + user_extra.incol < 5 limit 5",
     "plan": {
-
+      "QueryType": "SELECT",
+      "Original": "select Count(*) from user join user_extra where user.intcol + user_extra.incol < 5 limit 5",
+      "Instructions": {
+        "OperatorType": "Limit",
+        "Count": "5",
+        "Inputs": [
+          {
+            "OperatorType": "Aggregate",
+            "Variant": "Scalar",
+            "Aggregates": "sum_count_star(0) AS Count(*)",
+            "Inputs": [
+              {
+                "OperatorType": "Projection",
+                "Expressions": [
+                  "count(*) * count(*) as Count(*)"
+                ],
+                "Inputs": [
+                  {
+                    "OperatorType": "Join",
+                    "Variant": "Join",
+                    "JoinColumnIndexes": "L:0,R:0",
+                    "JoinVars": {
+                      "user_intcol": 1
+                    },
+                    "TableName": "`user`_user_extra",
+                    "Inputs": [
+                      {
+                        "OperatorType": "Route",
+                        "Variant": "Scatter",
+                        "Keyspace": {
+                          "Name": "user",
+                          "Sharded": true
+                        },
+                        "FieldQuery": "select count(*), `user`.intcol from `user` where 1 != 1 group by `user`.intcol",
+                        "Query": "select count(*), `user`.intcol from `user` group by `user`.intcol",
+                        "Table": "`user`"
+                      },
+                      {
+                        "OperatorType": "Route",
+                        "Variant": "Scatter",
+                        "Keyspace": {
+                          "Name": "user",
+                          "Sharded": true
+                        },
+                        "FieldQuery": "select count(*) from user_extra where 1 != 1 group by .0",
+                        "Query": "select count(*) from user_extra where :user_intcol + user_extra.incol < 5 group by .0",
+                        "Table": "user_extra"
+                      }
+                    ]
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      },
+      "TablesUsed": [
+        "user.user",
+        "user.user_extra"
+      ]
     }
   }
 ]