fix merge conflict

angelamayxie · angelamayxie · commit 6a1307e6dd1b · 2025-06-05T10:40:14.000-07:00
diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go
@@ -1675,7 +1675,7 @@ CREATE TABLE tab3 (
 			"INSERT INTO tab2 VALUES (1, 'b'), (2, 'm'), (3, 'g')",
 			"SELECT m.id, t.s FROM tab1 m JOIN tab2 t on m.id = t.i2 ORDER BY t.s DESC LIMIT 1 INTO @myId, @myText",
 			// TODO: union statement does not handle order by and limit clauses
-			//"SELECT id FROM tab1 UNION select s FROM tab2 LIMIT 1 INTO @myUnion",
+			// "SELECT id FROM tab1 UNION select s FROM tab2 LIMIT 1 INTO @myUnion",
 			"SELECT id FROM tab1 WHERE id > 3 UNION select s FROM tab2 WHERE s < 'f' INTO @mustSingleVar",
 		},
 		Assertions: []ScriptTestAssertion{
@@ -2760,6 +2760,163 @@ CREATE TABLE tab3 (
 			},
 		},
 	},
+	{
+		Name:    "Group Concat with Subquery in ORDER BY",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"CREATE TABLE test_data (id INT PRIMARY KEY, name VARCHAR(50), age INT, category VARCHAR(10))",
+			`INSERT INTO test_data VALUES  
+(1, 'Alice', 25, 'A'),
+(2, 'Bob', 30, 'B'), 
+(3, 'Charlie', 22, 'A'), 
+(4, 'Diana', 28, 'C'), 
+(5, 'Eve', 35, 'B'), 
+(6, 'Frank', 26, 'A')`,
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Skip:     true,
+				Query:    "SELECT category, group_concat(name ORDER BY (SELECT COUNT(*) FROM test_data t2 WHERE t2.category = test_data.category AND t2.age < test_data.age)) FROM test_data GROUP BY category ORDER BY category",
+				Expected: []sql.Row{{"A", "Charlie,Alice,Frank"}, {"B", "Bob,Eve"}, {"C", "Diana"}},
+			},
+			{
+				Skip:     true,
+				Query:    "SELECT group_concat(name ORDER BY (SELECT AVG(age) FROM test_data t2 WHERE t2.category = test_data.category), id) FROM test_data;",
+				Expected: []sql.Row{{"Alice,Charlie,Frank,Diana,Bob,Eve"}},
+			},
+			{
+				Query:    "SELECT category, group_concat(name ORDER BY (SELECT MAX(age) FROM test_data t2 WHERE t2.id <= test_data.id)) FROM test_data GROUP BY category ORDER BY category",
+				Expected: []sql.Row{{"A", "Alice,Charlie,Frank"}, {"B", "Bob,Eve"}, {"C", "Diana"}},
+			},
+		},
+	},
+	{
+		Name:    "Group Concat with Subquery in ORDER BY - Additional Edge Cases",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"CREATE TABLE products (id INT PRIMARY KEY, name VARCHAR(50), price DECIMAL(10,2), category_id INT, supplier_id INT)",
+			"CREATE TABLE categories (id INT PRIMARY KEY, name VARCHAR(50), priority INT)",
+			"CREATE TABLE suppliers (id INT PRIMARY KEY, name VARCHAR(50), rating INT)",
+			"INSERT INTO products VALUES (1, 'Laptop', 999.99, 1, 1), (2, 'Mouse', 25.50, 1, 2), (3, 'Keyboard', 75.00, 1, 1)",
+			"INSERT INTO products VALUES (4, 'Chair', 150.00, 2, 3), (5, 'Desk', 300.00, 2, 3), (6, 'Monitor', 250.00, 1, 2)",
+			"INSERT INTO categories VALUES (1, 'Electronics', 1), (2, 'Furniture', 2)",
+			"INSERT INTO suppliers VALUES (1, 'TechCorp', 5), (2, 'GadgetInc', 4), (3, 'OfficeSupply', 3)",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Skip:     true,
+				Query:    "SELECT category_id, GROUP_CONCAT(name ORDER BY (SELECT rating FROM suppliers WHERE suppliers.id = products.supplier_id) DESC, id ASC) FROM products GROUP BY category_id ORDER BY category_id",
+				Expected: []sql.Row{{1, "Laptop,Keyboard,Mouse,Monitor"}, {2, "Chair,Desk"}},
+			},
+			{
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM products p2 WHERE p2.price < products.price), id) FROM products",
+				Expected: []sql.Row{{"Mouse,Keyboard,Chair,Monitor,Desk,Laptop"}},
+			},
+			{
+				Skip:     true,
+				Query:    "SELECT category_id, GROUP_CONCAT(DISTINCT supplier_id ORDER BY (SELECT rating FROM suppliers WHERE suppliers.id = products.supplier_id)) FROM products GROUP BY category_id",
+				Expected: []sql.Row{{1, "2,1"}, {2, "3"}},
+			},
+			{
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT priority FROM categories WHERE categories.id = products.category_id), price) FROM products",
+				Expected: []sql.Row{{"Mouse,Keyboard,Monitor,Laptop,Chair,Desk"}},
+			},
+			{
+				Query:    "SELECT category_id, GROUP_CONCAT(name ORDER BY (SELECT AVG(price) FROM products p2 WHERE p2.category_id = products.category_id) DESC, name) FROM products GROUP BY category_id ORDER BY category_id",
+				Expected: []sql.Row{{1, "Keyboard,Laptop,Monitor,Mouse"}, {2, "Chair,Desk"}},
+			},
+		},
+	},
+	{
+		Name:    "Group Concat Subquery ORDER BY Error Cases",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"CREATE TABLE test_table (id INT PRIMARY KEY, name VARCHAR(50), value INT)",
+			"INSERT INTO test_table VALUES (1, 'A', 10), (2, 'B', 20), (3, 'C', 30)",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query:       "SELECT GROUP_CONCAT(name ORDER BY (SELECT name, value FROM test_table t2 WHERE t2.id = test_table.id)) FROM test_table",
+				ExpectedErr: sql.ErrInvalidOperandColumns,
+			},
+			{
+				Query:       "SELECT GROUP_CONCAT(name ORDER BY (SELECT value FROM test_table)) FROM test_table",
+				ExpectedErr: sql.ErrExpectedSingleRow,
+			},
+		},
+	},
+	{
+		Name:    "Group Concat Subquery ORDER BY Additional Edge Cases",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"CREATE TABLE complex_test (id INT PRIMARY KEY, name VARCHAR(50), value INT, category VARCHAR(10), created_at DATE)",
+			"INSERT INTO complex_test VALUES (1, 'Alpha', 100, 'X', '2023-01-01')",
+			"INSERT INTO complex_test VALUES (2, 'Beta', 50, 'Y', '2023-01-15')",
+			"INSERT INTO complex_test VALUES (3, 'Gamma', 75, 'X', '2023-02-01')",
+			"INSERT INTO complex_test VALUES (4, 'Delta', 25, 'Z', '2023-02-15')",
+			"INSERT INTO complex_test VALUES (5, 'Epsilon', 90, 'Y', '2023-03-01')",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				// Test with subquery returning NULL values
+				Skip:     true,
+				Query:    "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT CASE WHEN complex_test.value > 80 THEN NULL ELSE complex_test.value END), name) FROM complex_test GROUP BY category ORDER BY category",
+				Expected: []sql.Row{{"X", "Alpha,Gamma"}, {"Y", "Epsilon,Beta"}, {"Z", "Delta"}},
+			},
+			{
+				// Test with correlated subquery using multiple tables
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM complex_test c2 WHERE c2.category = complex_test.category AND c2.value > complex_test.value), name) FROM complex_test",
+				Expected: []sql.Row{{"Alpha,Delta,Epsilon,Beta,Gamma"}},
+			},
+			{
+				// Test with subquery using aggregate functions with HAVING
+				Skip:     true,
+				Query:    "SELECT category, GROUP_CONCAT(name ORDER BY (SELECT AVG(value), name FROM complex_test c2 WHERE c2.id <= complex_test.id HAVING AVG(value) > 50) DESC) FROM complex_test GROUP BY category ORDER BY category",
+				Expected: []sql.Row{{"X", "Alpha,Gamma"}, {"Y", "Beta,Epsilon"}, {"Z", "Delta"}},
+			},
+			{
+				// Test with DISTINCT and complex subquery
+				Query:    "SELECT GROUP_CONCAT(DISTINCT category ORDER BY (SELECT SUM(value) FROM complex_test c2 WHERE c2.category = complex_test.category) DESC SEPARATOR '|') FROM complex_test",
+				Expected: []sql.Row{{"X|Y|Z"}},
+			},
+			{
+				// Test with nested subqueries
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(name ORDER BY (SELECT COUNT(*) FROM complex_test c2 WHERE c2.value > (SELECT MIN(value) FROM complex_test c3 WHERE c3.category = complex_test.category))) FROM complex_test",
+				Expected: []sql.Row{{"Gamma,Alpha,Epsilon,Beta,Delta"}},
+			},
+		},
+	},
+	{
+		Name:    "Group Concat Subquery ORDER BY Performance and Boundary Cases",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"CREATE TABLE perf_test (id INT PRIMARY KEY, data VARCHAR(10), weight DECIMAL(5,2))",
+			"INSERT INTO perf_test VALUES (1, 'A', 1.5), (2, 'B', 2.5), (3, 'C', 0.5), (4, 'D', 3.5), (5, 'E', 2.0)",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				// Test with subquery returning same value for multiple rows (stability)
+				Query:    "SELECT GROUP_CONCAT(data ORDER BY (SELECT 42), id) FROM perf_test",
+				Expected: []sql.Row{{"A,B,C,D,E"}},
+			},
+			{
+				// Test with subquery using LIMIT
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(data ORDER BY (SELECT weight FROM perf_test p2 WHERE p2.id = perf_test.id LIMIT 1)) FROM perf_test",
+				Expected: []sql.Row{{"C,A,E,B,D"}},
+			},
+			{
+				// Test with very small decimal differences in ORDER BY subquery
+				Skip:     true,
+				Query:    "SELECT GROUP_CONCAT(data ORDER BY (SELECT weight + 0.001 * perf_test.id FROM perf_test p2 WHERE p2.id = perf_test.id)) FROM perf_test",
+				Expected: []sql.Row{{"C,A,E,B,D"}},
+			},
+		},
+	},
 	{
 		Name:    "CONVERT USING still converts between incompatible character sets",
 		Dialect: "mysql",
@@ -2998,7 +3155,7 @@ CREATE TABLE tab3 (
 			`CREATE TABLE t1 (pk int PRIMARY KEY, v1 varchar(10))`,
 			`INSERT INTO t1 VALUES (1,"1"), (2,"2"), (3,"3")`,
 			`CREATE TABLE t2 AS SELECT * FROM t1`,
-			//`CREATE TABLE t3(v0 int) AS SELECT pk FROM t1`, // parser problems
+			// `CREATE TABLE t3(v0 int) AS SELECT pk FROM t1`, // parser problems
 			`CREATE TABLE t3 AS SELECT pk FROM t1`,
 			`CREATE TABLE t4 AS SELECT pk, v1 FROM t1`,
 			`CREATE TABLE t5 SELECT * FROM t1 ORDER BY pk LIMIT 1`,
@@ -3163,7 +3320,7 @@ CREATE TABLE tab3 (
 				},
 			},
 		},
-		//todo(max): fix arithmatic on bindvar typing
+		// todo(max): fix arithmatic on bindvar typing
 		SkipPrepared: true,
 	},
 	{
@@ -4193,7 +4350,7 @@ CREATE TABLE tab3 (
 				// but GMS builds GroupBy for any aggregate function.
 				Skip:  true,
 				Query: "select count(*) from numbers having count(*) > val;",
-				//ExpectedErrStr:   "found HAVING clause with no GROUP BY", // not the exact error we want
+				// ExpectedErrStr:   "found HAVING clause with no GROUP BY", // not the exact error we want
 			},
 			{
 				Query:    "select count(*) from numbers group by val having count(*) < val;",
@@ -4205,7 +4362,7 @@ CREATE TABLE tab3 (
 		Name: "using having and group by clauses in subquery ",
 		SetUpScript: []string{
 			"CREATE TABLE t (i int, t varchar(2));",
-			"insert into t values (1, 'a'), (1, 'a2'), (2, 'b'), (3, 'c'), (3, 'c2'), (4, 'd'), (5, 'e'), (5, 'e2');", //, (6, 'f'), (7, 'g'), (7, 'g2')
+			"insert into t values (1, 'a'), (1, 'a2'), (2, 'b'), (3, 'c'), (3, 'c2'), (4, 'd'), (5, 'e'), (5, 'e2');", // , (6, 'f'), (7, 'g'), (7, 'g2')
 		},
 		Assertions: []ScriptTestAssertion{
 			{
diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/dolthub/go-icu-regex v0.0.0-20250327004329-6799764f2dad
 	github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71
 	github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
-	github.com/dolthub/vitess v0.0.0-20250530231040-bfd522856394
+	github.com/dolthub/vitess v0.0.0-20250604183723-cad71bbe21d0
 	github.com/go-kit/kit v0.10.0
 	github.com/go-sql-driver/mysql v1.7.2-0.20231213112541-0004702b931d
 	github.com/gocraft/dbr/v2 v2.7.2
diff --git a/go.sum b/go.sum
@@ -60,6 +60,8 @@ github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9X
 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
 github.com/dolthub/vitess v0.0.0-20250530231040-bfd522856394 h1:sMwntvk7O9dttaJLqnOvy8zgk0ah9qnyWkAahfOgnIo=
 github.com/dolthub/vitess v0.0.0-20250530231040-bfd522856394/go.mod h1:1gQZs/byeHLMSul3Lvl3MzioMtOW1je79QYGyi2fd70=
+github.com/dolthub/vitess v0.0.0-20250604183723-cad71bbe21d0 h1:esaEESlRNds2thjx9mEiDRIp1Cm5nM9K2PbYqPKei5Q=
+github.com/dolthub/vitess v0.0.0-20250604183723-cad71bbe21d0/go.mod h1:1gQZs/byeHLMSul3Lvl3MzioMtOW1je79QYGyi2fd70=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
 github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU=
diff --git a/server/handler.go b/server/handler.go
@@ -453,21 +453,21 @@ func (h *Handler) doQuery(
 	if queryStr != "" {
 		sqlCtx.SetLogger(sqlCtx.GetLogger().WithField("query", queryStr))
 	}
-	sqlCtx.GetLogger().Debugf("Starting query")
+	sqlCtx.GetLogger().WithField(sql.QueryTimeLogKey, time.Now()).Debugf("Starting query")
 
 	finish := observeQuery(sqlCtx, query)
 	defer func() {
 		finish(err)
 	}()
 
-	sqlCtx.GetLogger().Tracef("beginning execution")
+	sqlCtx.GetLogger().WithField(sql.QueryTimeLogKey, time.Now()).Tracef("beginning execution")
 
 	var schema sql.Schema
 	var rowIter sql.RowIter
 	qFlags.Set(sql.QFlagDeferProjections)
 	schema, rowIter, qFlags, err = queryExec(sqlCtx, query, parsed, analyzedPlan, bindings, qFlags)
 	if err != nil {
-		sqlCtx.GetLogger().WithError(err).Warn("error running query")
+		sqlCtx.GetLogger().WithField(sql.QueryTimeLogKey, time.Now()).WithError(err).Warn("error running query")
 		if verboseErrorLogging {
 			fmt.Printf("Err: %+v", err)
 		}
@@ -514,7 +514,7 @@ func (h *Handler) doQuery(
 		sqlCtx.GetLogger().Tracef("returning result %v", r)
 	}
 
-	sqlCtx.GetLogger().Debugf("Query finished in %d ms", time.Since(start).Milliseconds())
+	sqlCtx.GetLogger().WithField(sql.QueryTimeLogKey, time.Now()).Debugf("Query finished in %d ms", time.Since(start).Milliseconds())
 
 	// processedAtLeastOneBatch means we already called callback() at least
 	// once, so no need to call it if RowsAffected == 0.
diff --git a/server/handler_test.go b/server/handler_test.go
@@ -29,6 +29,7 @@ import (
 	"github.com/dolthub/vitess/go/race"
 	"github.com/dolthub/vitess/go/sqltypes"
 	"github.com/dolthub/vitess/go/vt/proto/query"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -1916,3 +1917,79 @@ func TestStatusVariableComUpdate(t *testing.T) {
 	checkSessionStatVar(t, sess1, "Com_update", uint64(5))
 	checkSessionStatVar(t, sess2, "Com_update", uint64(3))
 }
+
+// TestLoggerFieldsSetup tests that handler properly sets up logger fields including query time
+func TestLoggerFieldsSetup(t *testing.T) {
+	e, pro := setupMemDB(require.New(t))
+	dbFunc := pro.Database
+
+	handler := &Handler{
+		e: e,
+		sm: NewSessionManager(
+			sql.NewContext,
+			testSessionBuilder(pro),
+			sql.NoopTracer,
+			dbFunc,
+			sql.NewMemoryManager(nil),
+			sqle.NewProcessList(),
+			"foo",
+		),
+		readTimeout: time.Second,
+	}
+
+	conn := newConn(1)
+	handler.NewConnection(conn)
+	err := handler.ComInitDB(conn, "test")
+	require.NoError(t, err)
+
+	// Execute a query and verify basic logging setup
+	err = handler.ComQuery(context.Background(), conn, "SELECT 1", dummyCb)
+	require.NoError(t, err)
+
+	// Verify that the session's logger has the expected fields
+	session := handler.sm.session(conn)
+	logger := session.GetLogger()
+	require.NotNil(t, logger, "Session should have a logger")
+
+	// Verify that the logger has the expected fields
+	require.Contains(t, logger.Data, sql.ConnectTimeLogKey, "Logger should contain connect time")
+	require.Contains(t, logger.Data, sql.ConnectionIdLogField, "Logger should contain connection ID")
+
+	// Verify that queryTime is actually used in logs by capturing a log entry
+	var capturedFields logrus.Fields
+	hook := &testHook{fields: &capturedFields}
+	logrus.AddHook(hook)
+	defer logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks))
+
+	// Execute a query that will trigger error logging (which includes queryTime)
+	err = handler.ComQuery(context.Background(), conn, "SELECT * FROM nonexistent_table", dummyCb)
+	require.Error(t, err) // This should cause an error log with queryTime
+
+	// Verify that the log entry contained queryTime
+	require.Contains(t, capturedFields, sql.QueryTimeLogKey, "Log entry should contain queryTime field")
+
+	// Verify the values are of correct types
+	connectTime, ok := logger.Data[sql.ConnectTimeLogKey].(time.Time)
+	require.True(t, ok, "Connect time should be a time.Time")
+	require.False(t, connectTime.IsZero(), "Connect time should not be zero")
+
+	connID, ok := logger.Data[sql.ConnectionIdLogField].(uint32)
+	require.True(t, ok, "Connection ID should be a uint32")
+	require.Equal(t, conn.ConnectionID, connID, "Connection ID should match")
+}
+
+// Simple hook to capture log fields for testing
+type testHook struct {
+	fields *logrus.Fields
+}
+
+func (h *testHook) Levels() []logrus.Level {
+	return []logrus.Level{logrus.WarnLevel} // Only capture warning level (error logs)
+}
+
+func (h *testHook) Fire(entry *logrus.Entry) error {
+	if entry.Message == "error running query" {
+		*h.fields = entry.Data
+	}
+	return nil
+}
diff --git a/sql/expression/isnull.go b/sql/expression/isnull.go
@@ -54,6 +54,17 @@ func (e *IsNull) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 		return nil, err
 	}
 
+	// Slices of typed values (e.g. Record and Composite types in Postgres) evaluate
+	// to NULL if all of their entries are NULL.
+	if tupleValue, ok := v.([]types.TupleValue); ok {
+		for _, typedValue := range tupleValue {
+			if typedValue.Value != nil {
+				return false, nil
+			}
+		}
+		return true, nil
+	}
+
 	return v == nil, nil
 }
 
diff --git a/sql/log.go b/sql/log.go
@@ -18,4 +18,5 @@ const (
 	ConnectionIdLogField = "connectionID"
 	ConnectionDbLogField = "connectionDb"
 	ConnectTimeLogKey    = "connectTime"
+	QueryTimeLogKey      = "queryTime"
 )
diff --git a/sql/types/tuple_value.go b/sql/types/tuple_value.go
@@ -0,0 +1,24 @@
+// Copyright 2025 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package types
+
+import "github.com/dolthub/go-mysql-server/sql"
+
+// TupleValue represents a value and its associated type information. TupleValue is used by collections of
+// values where the type information is not consistent across all values (e.g. Records in Postgres).
+type TupleValue struct {
+	Value any
+	Type  sql.Type
+}

Original file line number	Diff line number	Diff line change
`@@ -18,4 +18,5 @@ const (`
`18`	`18`	`ConnectionIdLogField = "connectionID"`
`19`	`19`	`ConnectionDbLogField = "connectionDb"`
`20`	`20`	`ConnectTimeLogKey = "connectTime"`
	`21`	`+ QueryTimeLogKey = "queryTime"`
`21`	`22`	`)`