From 90a2cef5c0c46d1e96a769b4076d3dc68e734e65 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Fri, 30 May 2025 16:31:00 -0700 Subject: [PATCH 1/8] set default hostname to os.Hostname --- sql/variables/system_variables.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sql/variables/system_variables.go b/sql/variables/system_variables.go index 188d3bc5ec..6cf1431d9f 100644 --- a/sql/variables/system_variables.go +++ b/sql/variables/system_variables.go @@ -17,6 +17,7 @@ package variables import ( "fmt" "math" + "os" "strings" "sync" "time" @@ -187,6 +188,11 @@ func init() { InitSystemVariables() } +func getHostname() string { + hostname, _ := os.Hostname() + return hostname +} + // systemVars is the internal collection of all MySQL system variables according to the following pages: // https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html // https://dev.mysql.com/doc/refman/8.0/en/replication-options-gtids.html @@ -1009,7 +1015,7 @@ var systemVars = map[string]sql.SystemVariable{ Dynamic: false, SetVarHintApplies: false, Type: types.NewSystemStringType("hostname"), - Default: "", + Default: getHostname(), }, "immediate_server_version": &sql.MysqlSystemVariable{ Name: "immediate_server_version", From 46366b5af0f6ff9332a341601d5fd00ed091f207 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Fri, 30 May 2025 17:43:30 -0700 Subject: [PATCH 2/8] set port and max conns when server is created --- server/server.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/server.go b/server/server.go index 205147c1bb..f1de217aba 100644 --- a/server/server.go +++ b/server/server.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "net" + "strconv" "time" "github.com/dolthub/vitess/go/mysql" @@ -118,6 +119,15 @@ func portInUse(hostPort string) bool { return false } +func updateSystemVariables(cfg mysql.ListenerConfig) { + _, port, _ := net.SplitHostPort(cfg.Listener.Addr().String()) + portInt, _ := strconv.ParseInt(port, 10, 64) + sql.SystemVariables.AssignValues(map[string]interface{}{ + "max_connections": cfg.MaxConns, + "port": portInt, + }) +} + func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handler mysql.Handler, sel ServerEventListener) (*Server, error) { if cfg.ConnReadTimeout < 0 { cfg.ConnReadTimeout = 0 @@ -172,6 +182,8 @@ func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handle return nil, err } + updateSystemVariables(listenerCfg) + return &Server{ Listener: protocolListener, handler: handler, From 217752b84f19257d6d2769ceebb8927d7cc12dc1 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Mon, 2 Jun 2025 09:16:44 -0700 Subject: [PATCH 3/8] moved unknown sys variable test into VariableErrorsTests --- enginetest/queries/variable_queries.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/enginetest/queries/variable_queries.go b/enginetest/queries/variable_queries.go index 173be4222a..61b46932cb 100644 --- a/enginetest/queries/variable_queries.go +++ b/enginetest/queries/variable_queries.go @@ -22,12 +22,6 @@ import ( ) var VariableQueries = []ScriptTest{ - { - Name: "use string name for foreign_key checks", - SetUpScript: []string{}, - Query: "select @@GLOBAL.unknown", - ExpectedErr: sql.ErrUnknownSystemVariable, - }, { Name: "use string name for foreign_key checks", SetUpScript: []string{}, @@ -649,6 +643,10 @@ var VariableQueries = []ScriptTest{ } var VariableErrorTests = []QueryErrorTest{ + { + Query: "select @@GLOBAL.unknown", + ExpectedErr: sql.ErrUnknownSystemVariable, + }, { Query: "set @@does_not_exist = 100", ExpectedErr: sql.ErrUnknownSystemVariable, From 7df5fb41c4ff6867f9811a5b5056f897f1e6c98f Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 4 Jun 2025 16:40:26 -0700 Subject: [PATCH 4/8] temporary tests --- enginetest/memory_engine_test.go | 16 ++---- enginetest/server_engine_test.go | 85 +++++++++++++++++++++++++++++++ server/server.go | 45 +++++++++++++--- sql/variables/system_variables.go | 8 +-- 4 files changed, 128 insertions(+), 26 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index cb5455eed8..a59e308ca6 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -202,23 +202,15 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() + //t.Skip() var scripts = []queries.ScriptTest{ { - Name: "AS OF propagates to nested CALLs", + Name: "test", SetUpScript: []string{}, Assertions: []queries.ScriptTestAssertion{ { - Query: "create procedure create_proc() create table t (i int primary key, j int);", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, - }, - { - Query: "call create_proc()", - Expected: []sql.Row{ - {types.NewOkResult(0)}, - }, + Query: "select @@hostname, @@port, @@max_connections", + Expected: []sql.Row{}, }, }, }, diff --git a/enginetest/server_engine_test.go b/enginetest/server_engine_test.go index c95f60b1c3..b6bd55ddf0 100644 --- a/enginetest/server_engine_test.go +++ b/enginetest/server_engine_test.go @@ -375,3 +375,88 @@ func TestServerPreparedStatements(t *testing.T) { }) } } + +func TestServerQueries(t *testing.T) { + port, perr := findEmptyPort() + require.NoError(t, perr) + + s, serr := initTestServer(port) + require.NoError(t, serr) + + go s.Start() + defer s.Close() + + tests := []serverScriptTest{ + { + name: "test that config variables are properly set", + setup: []string{}, + assertions: []serverScriptTestAssertion{ + { + query: "select @@hostname, @@port", + //query: "select @@hostname, @@port, @@max_connections", + isExec: false, + expectedRows: []any{ + sql.Row{"macbook.local", port}, + }, + checkRows: func(t *testing.T, rows *gosql.Rows, expectedRows []any) (bool, error) { + var resHostname string + var resPort int + var rowNum int + for rows.Next() { + if err := rows.Scan(&resHostname, &resPort); err != nil { + return false, err + } + if rowNum >= len(expectedRows) { + return false, nil + } + expectedRow := expectedRows[rowNum].(sql.Row) + require.Equal(t, expectedRow[0].(string), resHostname) + require.Equal(t, expectedRow[1].(int), resPort) + } + return true, nil + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + conn, cerr := dbr.Open("mysql", fmt.Sprintf(noUserFmt, address, port), nil) + require.NoError(t, cerr) + defer conn.Close() + commonSetup := []string{ + "create database test_db;", + "use test_db;", + } + commonTeardown := []string{ + "drop database test_db", + } + for _, stmt := range append(commonSetup, test.setup...) { + _, err := conn.Exec(stmt) + require.NoError(t, err) + } + for _, assertion := range test.assertions { + t.Run(assertion.query, func(t *testing.T) { + if assertion.skip { + t.Skip() + } + rows, err := conn.Query(assertion.query, assertion.args...) + if assertion.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + ok, err := assertion.checkRows(t, rows, assertion.expectedRows) + require.NoError(t, err) + require.True(t, ok) + }) + } + for _, stmt := range append(commonTeardown) { + _, err := conn.Exec(stmt) + require.NoError(t, err) + } + }) + } +} diff --git a/server/server.go b/server/server.go index f1de217aba..0a6f8ff919 100644 --- a/server/server.go +++ b/server/server.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "net" + "os" "strconv" "time" @@ -119,13 +120,40 @@ func portInUse(hostPort string) bool { return false } -func updateSystemVariables(cfg mysql.ListenerConfig) { - _, port, _ := net.SplitHostPort(cfg.Listener.Addr().String()) - portInt, _ := strconv.ParseInt(port, 10, 64) - sql.SystemVariables.AssignValues(map[string]interface{}{ - "max_connections": cfg.MaxConns, - "port": portInt, +func getHostname() (string, error) { + hostname, err := os.Hostname() + if err != nil { + return "", err + } + return hostname, nil +} + +func updateSystemVariables(cfg mysql.ListenerConfig) error { + hostname, err := getHostname() + if err != nil { + return err + } + _, port, err := net.SplitHostPort(cfg.Listener.Addr().String()) + if err != nil { + return err + } + print(port) + portInt, err := strconv.ParseInt(port, 10, 64) + if err != nil { + return err + } + if portInt == 0 { + } + err = sql.SystemVariables.AssignValues(map[string]interface{}{ + "port": portInt, + "hostname": hostname, + // TODO: the rest + //"max_connections": cfg.MaxConns, }) + if err != nil { + return err + } + return nil } func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handler mysql.Handler, sel ServerEventListener) (*Server, error) { @@ -182,7 +210,10 @@ func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handle return nil, err } - updateSystemVariables(listenerCfg) + err = updateSystemVariables(listenerCfg) + if err != nil { + return nil, err + } return &Server{ Listener: protocolListener, diff --git a/sql/variables/system_variables.go b/sql/variables/system_variables.go index 6cf1431d9f..188d3bc5ec 100644 --- a/sql/variables/system_variables.go +++ b/sql/variables/system_variables.go @@ -17,7 +17,6 @@ package variables import ( "fmt" "math" - "os" "strings" "sync" "time" @@ -188,11 +187,6 @@ func init() { InitSystemVariables() } -func getHostname() string { - hostname, _ := os.Hostname() - return hostname -} - // systemVars is the internal collection of all MySQL system variables according to the following pages: // https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html // https://dev.mysql.com/doc/refman/8.0/en/replication-options-gtids.html @@ -1015,7 +1009,7 @@ var systemVars = map[string]sql.SystemVariable{ Dynamic: false, SetVarHintApplies: false, Type: types.NewSystemStringType("hostname"), - Default: getHostname(), + Default: "", }, "immediate_server_version": &sql.MysqlSystemVariable{ Name: "immediate_server_version", From 0ce96502f12fa55b6c95d73ddbee3d475cf8e0c3 Mon Sep 17 00:00:00 2001 From: James Cor Date: Fri, 6 Jun 2025 10:13:29 -0700 Subject: [PATCH 5/8] fix --- enginetest/memory_engine_test.go | 16 ++++++++++++---- server/server.go | 6 ++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index a59e308ca6..cb5455eed8 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -202,15 +202,23 @@ func TestSingleQueryPrepared(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - //t.Skip() + t.Skip() var scripts = []queries.ScriptTest{ { - Name: "test", + Name: "AS OF propagates to nested CALLs", SetUpScript: []string{}, Assertions: []queries.ScriptTestAssertion{ { - Query: "select @@hostname, @@port, @@max_connections", - Expected: []sql.Row{}, + Query: "create procedure create_proc() create table t (i int primary key, j int);", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, + }, + { + Query: "call create_proc()", + Expected: []sql.Row{ + {types.NewOkResult(0)}, + }, }, }, }, diff --git a/server/server.go b/server/server.go index 0a6f8ff919..061e7dbc6b 100644 --- a/server/server.go +++ b/server/server.go @@ -137,17 +137,15 @@ func updateSystemVariables(cfg mysql.ListenerConfig) error { if err != nil { return err } - print(port) portInt, err := strconv.ParseInt(port, 10, 64) if err != nil { return err } - if portInt == 0 { - } + // TODO: add the rest of the config variables err = sql.SystemVariables.AssignValues(map[string]interface{}{ "port": portInt, "hostname": hostname, - // TODO: the rest + // TODO: this causes an error because max_connections is 0? //"max_connections": cfg.MaxConns, }) if err != nil { From d8ac48613a082cc19b237ce8feed64fba628a514 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Mon, 9 Jun 2025 16:07:10 -0700 Subject: [PATCH 6/8] set timeout sys vars, updated tests --- enginetest/server_engine_test.go | 18 +++++++++----- server/server.go | 41 +++++++++++++------------------- server/server_config.go | 4 ++-- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/enginetest/server_engine_test.go b/enginetest/server_engine_test.go index b6bd55ddf0..eb3263826b 100644 --- a/enginetest/server_engine_test.go +++ b/enginetest/server_engine_test.go @@ -6,6 +6,7 @@ import ( "fmt" "math" "net" + "os" "testing" "github.com/dolthub/vitess/go/mysql" @@ -376,7 +377,10 @@ func TestServerPreparedStatements(t *testing.T) { } } -func TestServerQueries(t *testing.T) { +func TestServerVariables(t *testing.T) { + hostname, herr := os.Hostname() + require.NoError(t, herr) + port, perr := findEmptyPort() require.NoError(t, perr) @@ -388,22 +392,24 @@ func TestServerQueries(t *testing.T) { tests := []serverScriptTest{ { - name: "test that config variables are properly set", + name: "test that config system variables are properly set", setup: []string{}, assertions: []serverScriptTestAssertion{ { - query: "select @@hostname, @@port", - //query: "select @@hostname, @@port, @@max_connections", + query: "select @@hostname, @@port, @@max_connections, @@net_read_timeout, @@net_write_timeout", isExec: false, expectedRows: []any{ - sql.Row{"macbook.local", port}, + sql.Row{hostname, port, 1, 1, 1}, }, checkRows: func(t *testing.T, rows *gosql.Rows, expectedRows []any) (bool, error) { var resHostname string var resPort int + var resMaxConnections int + var resNetReadTimeout int + var resNetWriteTimeout int var rowNum int for rows.Next() { - if err := rows.Scan(&resHostname, &resPort); err != nil { + if err := rows.Scan(&resHostname, &resPort, &resMaxConnections, &resNetReadTimeout, &resNetWriteTimeout); err != nil { return false, err } if rowNum >= len(expectedRows) { diff --git a/server/server.go b/server/server.go index 061e7dbc6b..9b8abbbaa3 100644 --- a/server/server.go +++ b/server/server.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "net" - "os" "strconv" "time" @@ -120,19 +119,7 @@ func portInUse(hostPort string) bool { return false } -func getHostname() (string, error) { - hostname, err := os.Hostname() - if err != nil { - return "", err - } - return hostname, nil -} - func updateSystemVariables(cfg mysql.ListenerConfig) error { - hostname, err := getHostname() - if err != nil { - return err - } _, port, err := net.SplitHostPort(cfg.Listener.Addr().String()) if err != nil { return err @@ -143,10 +130,10 @@ func updateSystemVariables(cfg mysql.ListenerConfig) error { } // TODO: add the rest of the config variables err = sql.SystemVariables.AssignValues(map[string]interface{}{ - "port": portInt, - "hostname": hostname, - // TODO: this causes an error because max_connections is 0? - //"max_connections": cfg.MaxConns, + "port": portInt, + "max_connections": cfg.MaxConns, + "net_read_timeout": cfg.ConnReadTimeout.Seconds(), + "net_write_timeout": cfg.ConnWriteTimeout.Seconds(), }) if err != nil { return err @@ -155,14 +142,18 @@ func updateSystemVariables(cfg mysql.ListenerConfig) error { } func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handler mysql.Handler, sel ServerEventListener) (*Server, error) { - if cfg.ConnReadTimeout < 0 { - cfg.ConnReadTimeout = 0 - } - if cfg.ConnWriteTimeout < 0 { - cfg.ConnWriteTimeout = 0 - } - if cfg.MaxConnections < 0 { - cfg.MaxConnections = 0 + oneSecond := time.Duration(1) * time.Second + if cfg.ConnReadTimeout < oneSecond { + // TODO set to MySQL default value + cfg.ConnReadTimeout = oneSecond + } + if cfg.ConnWriteTimeout < oneSecond { + // TODO set to MySQL default value + cfg.ConnWriteTimeout = oneSecond + } + if cfg.MaxConnections < 1 { + // TODO set to MySQL default value + cfg.MaxConnections = 1 } for _, opt := range cfg.Options { diff --git a/server/server_config.go b/server/server_config.go index 853d622616..d0af2c26ac 100644 --- a/server/server_config.go +++ b/server/server_config.go @@ -109,14 +109,14 @@ func (c Config) NewConfig() (Config, error) { if !ok { return Config{}, sql.ErrUnknownSystemVariable.New("net_write_timeout") } - c.ConnWriteTimeout = time.Duration(timeout) * time.Millisecond + c.ConnWriteTimeout = time.Duration(timeout) * time.Second } if _, val, ok := sql.SystemVariables.GetGlobal("net_read_timeout"); ok { timeout, ok := val.(int64) if !ok { return Config{}, sql.ErrUnknownSystemVariable.New("net_read_timeout") } - c.ConnReadTimeout = time.Duration(timeout) * time.Millisecond + c.ConnReadTimeout = time.Duration(timeout) * time.Second } return c, nil } From 11be5705b7d16a89e35004b4ecfe150157757f56 Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Mon, 9 Jun 2025 16:24:24 -0700 Subject: [PATCH 7/8] mysql sys vars use seconds, not milliseconds --- server/server_config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/server_config_test.go b/server/server_config_test.go index c1d7b2b2dc..a2a8319a20 100644 --- a/server/server_config_test.go +++ b/server/server_config_test.go @@ -49,14 +49,14 @@ func TestConfigWithDefaults(t *testing.T) { Type: types.NewSystemIntType("net_write_timeout", 1, 9223372036854775807, false), ConfigField: "ConnWriteTimeout", Default: int64(76), - ExpectedCmp: int64(76000000), + ExpectedCmp: int64(76000000000), }, { Name: "net_read_timeout", Scope: sql.SystemVariableScope_Both, Type: types.NewSystemIntType("net_read_timeout", 1, 9223372036854775807, false), ConfigField: "ConnReadTimeout", Default: int64(67), - ExpectedCmp: int64(67000000), + ExpectedCmp: int64(67000000000), }, } From b05c9dd75c7849a8a5d68fc27a188902889d98cc Mon Sep 17 00:00:00 2001 From: Angela Xie Date: Mon, 9 Jun 2025 17:11:03 -0700 Subject: [PATCH 8/8] set default port when port is not available (like with bufconn) --- server/server.go | 18 +++++++++++++----- server/server_test.go | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/server/server.go b/server/server.go index 9b8abbbaa3..f124a4894d 100644 --- a/server/server.go +++ b/server/server.go @@ -119,18 +119,26 @@ func portInUse(hostPort string) bool { return false } -func updateSystemVariables(cfg mysql.ListenerConfig) error { +func getPortOrDefault(cfg mysql.ListenerConfig) int64 { + // TODO read this values from systemVars + defaultPort := int64(3606) _, port, err := net.SplitHostPort(cfg.Listener.Addr().String()) if err != nil { - return err + return defaultPort } portInt, err := strconv.ParseInt(port, 10, 64) if err != nil { - return err + return defaultPort } + return portInt +} + +func updateSystemVariables(cfg mysql.ListenerConfig) error { + port := getPortOrDefault(cfg) + // TODO: add the rest of the config variables - err = sql.SystemVariables.AssignValues(map[string]interface{}{ - "port": portInt, + err := sql.SystemVariables.AssignValues(map[string]interface{}{ + "port": port, "max_connections": cfg.MaxConns, "net_read_timeout": cfg.ConnReadTimeout.Seconds(), "net_write_timeout": cfg.ConnWriteTimeout.Seconds(), diff --git a/server/server_test.go b/server/server_test.go index 0929f5e6de..3d229789ae 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -18,8 +18,8 @@ import ( gsql "github.com/dolthub/go-mysql-server/sql" ) -// TestSeverCustomListener verifies a caller can provide their own net.Conn implementation for the server to use -func TestSeverCustomListener(t *testing.T) { +// TestServerCustomListener verifies a caller can provide their own net.Conn implementation for the server to use +func TestServerCustomListener(t *testing.T) { dbName := "mydb" // create a net.Conn thats based on a golang buffer buffer := 1024