From 8905ac0bbdc481b7e6c5b4db18a3227a896bb65f Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 7 May 2020 19:30:26 +0530 Subject: [PATCH 1/5] send use statement to vtgate instead of cominitdb method Signed-off-by: Harshit Gangal --- go/mysql/conn.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/go/mysql/conn.go b/go/mysql/conn.go index d515ec6c7a2..57ababf4762 100644 --- a/go/mysql/conn.go +++ b/go/mysql/conn.go @@ -780,10 +780,7 @@ func (c *Conn) handleNextCommand(handler Handler) error { case ComInitDB: db := c.parseComInitDB(data) c.recycleReadPacket() - c.schemaName = db - handler.ComInitDB(c, db) - if err := c.writeOKPacket(0, 0, c.StatusFlags, 0); err != nil { - log.Errorf("Error writing ComInitDB result to %s: %v", c, err) + if err := c.execQuery(fmt.Sprintf("use `%s`", db), handler, false); err != nil { return err } case ComQuery: From 7045c385a7907515e0fbe56b8ba682202b7d1fb3 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 7 May 2020 19:31:31 +0530 Subject: [PATCH 2/5] replace database() with targetstring than keyspace for com_init_db to work Signed-off-by: Harshit Gangal --- go/vt/vtgate/executor.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 9a3810684d3..da8396f336f 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -230,12 +230,7 @@ func (e *Executor) execute(ctx context.Context, safeSession *SafeSession, sql st // addNeededBindVars adds bind vars that are needed by the plan func (e *Executor) addNeededBindVars(bindVarNeeds sqlparser.BindVarNeeds, bindVars map[string]*querypb.BindVariable, session *SafeSession) error { if bindVarNeeds.NeedDatabase { - keyspace, _, _, _ := e.ParseDestinationTarget(session.TargetString) - if keyspace == "" { - bindVars[sqlparser.DBVarName] = sqltypes.NullBindVariable - } else { - bindVars[sqlparser.DBVarName] = sqltypes.StringBindVariable(keyspace) - } + bindVars[sqlparser.DBVarName] = sqltypes.StringBindVariable(session.TargetString) } if bindVarNeeds.NeedLastInsertID { From c825bcaa66bf78ddb9521d83b0f8340d0ec60c04 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 7 May 2020 19:32:31 +0530 Subject: [PATCH 3/5] comm_init_db: fix test with correct database() and error message Signed-off-by: Harshit Gangal --- go/vt/vtgate/executor_select_test.go | 2 +- go/vt/vtgate/executor_test.go | 4 ++-- go/vt/vtgate/mysql_protocol_test.go | 2 +- go/vt/vtgate/plan_executor_select_test.go | 2 +- go/vt/vtgate/plan_executor_test.go | 4 ++-- go/vt/vtgate/plugin_mysql_server.go | 4 ---- go/vt/vtgate/plugin_mysql_server_test.go | 3 --- go/vt/vtgate/vcursor_impl.go | 4 +++- go/vt/vtgate/vcursor_impl_test.go | 2 +- 9 files changed, 11 insertions(+), 16 deletions(-) diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go index c8b90bd2ce3..29e8f9859c0 100644 --- a/go/vt/vtgate/executor_select_test.go +++ b/go/vt/vtgate/executor_select_test.go @@ -389,7 +389,7 @@ func TestSelectDatabase(t *testing.T) { {Name: "database()", Type: sqltypes.VarBinary}, }, Rows: [][]sqltypes.Value{{ - sqltypes.NewVarBinary("TestExecutor"), + sqltypes.NewVarBinary("TestExecutor@master"), }}, } require.NoError(t, err) diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index f35a9cd62c0..120009d0d65 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -207,7 +207,7 @@ func TestDirectTargetRewrites(t *testing.T) { require.NoError(t, err) testQueries(t, "sbclookup", sbclookup, []*querypb.BoundQuery{{ Sql: "select :__vtdbname as `database()` from dual", - BindVariables: map[string]*querypb.BindVariable{"__vtdbname": sqltypes.StringBindVariable("TestUnsharded")}, + BindVariables: map[string]*querypb.BindVariable{"__vtdbname": sqltypes.StringBindVariable("TestUnsharded/0@master")}, }}) } @@ -1038,7 +1038,7 @@ func TestExecutorUse(t *testing.T) { } _, err = executor.Execute(context.Background(), "TestExecute", NewSafeSession(&vtgatepb.Session{}), "use UnexistentKeyspace", nil) - wantErr = "invalid keyspace provided: UnexistentKeyspace" + wantErr = "Unknown database 'UnexistentKeyspace' (errno 1049) (sqlstate 42000)" if err == nil || err.Error() != wantErr { t.Errorf("got: %v, want %v", err, wantErr) } diff --git a/go/vt/vtgate/mysql_protocol_test.go b/go/vt/vtgate/mysql_protocol_test.go index 5a93c224e7e..bff3958773c 100644 --- a/go/vt/vtgate/mysql_protocol_test.go +++ b/go/vt/vtgate/mysql_protocol_test.go @@ -118,7 +118,7 @@ func TestMySQLProtocolExecuteUseStatement(t *testing.T) { // No such keyspace this will fail _, err = c.ExecuteFetch("use InvalidKeyspace", 0, false) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid keyspace provided: InvalidKeyspace") + assert.Contains(t, err.Error(), "Unknown database 'InvalidKeyspace' (errno 1049) (sqlstate 42000)") // That doesn't reset the vitess_target qr, err = c.ExecuteFetch("show vitess_target", 1, false) diff --git a/go/vt/vtgate/plan_executor_select_test.go b/go/vt/vtgate/plan_executor_select_test.go index 01d2a644909..4a8caea274e 100644 --- a/go/vt/vtgate/plan_executor_select_test.go +++ b/go/vt/vtgate/plan_executor_select_test.go @@ -391,7 +391,7 @@ func TestPlanSelectDatabase(t *testing.T) { {Name: "database()", Type: sqltypes.VarBinary}, }, Rows: [][]sqltypes.Value{{ - sqltypes.NewVarBinary("TestExecutor"), + sqltypes.NewVarBinary("TestExecutor@master"), }}, } require.NoError(t, err) diff --git a/go/vt/vtgate/plan_executor_test.go b/go/vt/vtgate/plan_executor_test.go index 222f9709c57..792c762cfdc 100644 --- a/go/vt/vtgate/plan_executor_test.go +++ b/go/vt/vtgate/plan_executor_test.go @@ -205,7 +205,7 @@ func TestPlanDirectTargetRewrites(t *testing.T) { require.NoError(t, err) testQueries(t, "sbclookup", sbclookup, []*querypb.BoundQuery{{ Sql: "select :__vtdbname as `database()` from dual", - BindVariables: map[string]*querypb.BindVariable{"__vtdbname": sqltypes.StringBindVariable("TestUnsharded")}, + BindVariables: map[string]*querypb.BindVariable{"__vtdbname": sqltypes.StringBindVariable("TestUnsharded/0@master")}, }}) } @@ -996,7 +996,7 @@ func TestPlanExecutorUse(t *testing.T) { } _, err = executor.Execute(context.Background(), "TestExecute", NewSafeSession(&vtgatepb.Session{}), "use UnexistentKeyspace", nil) - wantErr = "invalid keyspace provided: UnexistentKeyspace" + wantErr = "Unknown database 'UnexistentKeyspace' (errno 1049) (sqlstate 42000)" if err == nil || err.Error() != wantErr { t.Errorf("got: %v, want %v", err, wantErr) } diff --git a/go/vt/vtgate/plugin_mysql_server.go b/go/vt/vtgate/plugin_mysql_server.go index b83330c34f6..dc5e614b42e 100644 --- a/go/vt/vtgate/plugin_mysql_server.go +++ b/go/vt/vtgate/plugin_mysql_server.go @@ -166,10 +166,6 @@ func startSpan(ctx context.Context, query, label string) (trace.Span, context.Co return startSpanTestable(ctx, query, label, trace.NewSpan, trace.NewFromString) } -func (vh *vtgateHandler) ComInitDB(c *mysql.Conn, schemaName string) { - vh.session(c).TargetString = schemaName -} - func (vh *vtgateHandler) ComQuery(c *mysql.Conn, query string, callback func(*sqltypes.Result) error) error { ctx := context.Background() var cancel context.CancelFunc diff --git a/go/vt/vtgate/plugin_mysql_server_test.go b/go/vt/vtgate/plugin_mysql_server_test.go index e056dda24cd..993ab442792 100644 --- a/go/vt/vtgate/plugin_mysql_server_test.go +++ b/go/vt/vtgate/plugin_mysql_server_test.go @@ -43,9 +43,6 @@ func (th *testHandler) NewConnection(c *mysql.Conn) { func (th *testHandler) ConnectionClosed(c *mysql.Conn) { } -func (th *testHandler) ComInitDB(c *mysql.Conn, schemaName string) { -} - func (th *testHandler) ComQuery(c *mysql.Conn, q string, callback func(*sqltypes.Result) error) error { return nil } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 0d589740377..0e96f2f575b 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -21,6 +21,8 @@ import ( "sync/atomic" "time" + "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/vt/callerid" vschemapb "vitess.io/vitess/go/vt/proto/vschema" "vitess.io/vitess/go/vt/topotools" @@ -345,7 +347,7 @@ func (vc *vcursorImpl) SetTarget(target string) error { return err } if _, ok := vc.vschema.Keyspaces[keyspace]; keyspace != "" && !ok { - return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid keyspace provided: %s", keyspace) + return mysql.NewSQLError(mysql.ERBadDb, "42000", "Unknown database '%s'", keyspace) } if vc.safeSession.InTransaction() && tabletType != topodatapb.TabletType_MASTER { diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index 205bf2fcfa9..f6b36c68fa8 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -197,7 +197,7 @@ func TestSetTarget(t *testing.T) { }, { vschema: vschemaWith2KS, targetString: "ks3", - expectedError: "invalid keyspace provided: ks3", + expectedError: "Unknown database 'ks3' (errno 1049) (sqlstate 42000)", }, { vschema: vschemaWith2KS, targetString: "ks2@replica", From f65943bd7b1f4d820762e92819ade7ac0a8c3e41 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 7 May 2020 19:33:55 +0530 Subject: [PATCH 4/5] remove cominitdb call with comquery with use statement Signed-off-by: Harshit Gangal --- go/mysql/fakesqldb/server.go | 4 ---- go/mysql/server.go | 11 ++++++----- go/mysql/server_test.go | 3 --- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/go/mysql/fakesqldb/server.go b/go/mysql/fakesqldb/server.go index f7c4f904742..ada111ffd47 100644 --- a/go/mysql/fakesqldb/server.go +++ b/go/mysql/fakesqldb/server.go @@ -312,10 +312,6 @@ func (db *DB) ConnectionClosed(c *mysql.Conn) { delete(db.connections, c.ConnectionID) } -// ComInitDB is part of the mysql.Handler interface. -func (db *DB) ComInitDB(c *mysql.Conn, schemaName string) { -} - // ComQuery is part of the mysql.Handler interface. func (db *DB) ComQuery(c *mysql.Conn, query string, callback func(*sqltypes.Result) error) error { return db.Handler.HandleQuery(c, query, callback) diff --git a/go/mysql/server.go b/go/mysql/server.go index 8eb72a2d87e..3155655106f 100644 --- a/go/mysql/server.go +++ b/go/mysql/server.go @@ -18,6 +18,7 @@ package mysql import ( "crypto/tls" + "fmt" "io" "net" "strings" @@ -90,10 +91,6 @@ type Handler interface { // ConnectionClosed is called when a connection is closed. ConnectionClosed(c *Conn) - // InitDB is called once at the beginning to set db name, - // and subsequently for every ComInitDB event. - ComInitDB(c *Conn, schemaName string) - // ComQuery is called when a connection receives a query. // Note the contents of the query slice may change after // the first call to callback. So the Handler should not @@ -458,7 +455,11 @@ func (l *Listener) handle(conn net.Conn, connectionID uint32, acceptTime time.Ti } // Set initial db name. - l.handler.ComInitDB(c, c.schemaName) + if c.schemaName != "" { + l.handler.ComQuery(c, fmt.Sprintf("use `%s`", c.schemaName), func(result *sqltypes.Result) error { + return nil + }) + } for { err := c.handleNextCommand(l.handler) diff --git a/go/mysql/server_test.go b/go/mysql/server_test.go index 863223296ce..4c256c8dc12 100644 --- a/go/mysql/server_test.go +++ b/go/mysql/server_test.go @@ -113,9 +113,6 @@ func (th *testHandler) NewConnection(c *Conn) { func (th *testHandler) ConnectionClosed(c *Conn) { } -func (th *testHandler) ComInitDB(c *Conn, schemaName string) { -} - func (th *testHandler) ComQuery(c *Conn, query string, callback func(*sqltypes.Result) error) error { if result := th.Result(); result != nil { callback(result) From d90138eeec3cc5cea49cc43c1f92e2ec6f72aad0 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 7 May 2020 21:55:02 +0530 Subject: [PATCH 5/5] return unknown db message on mysql connect to vtgate Signed-off-by: Harshit Gangal --- go/mysql/server.go | 18 +++++++++++------- go/vt/vtgate/mysql_protocol_test.go | 14 ++------------ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/go/mysql/server.go b/go/mysql/server.go index 3155655106f..fb6a7388839 100644 --- a/go/mysql/server.go +++ b/go/mysql/server.go @@ -438,6 +438,17 @@ func (l *Listener) handle(conn net.Conn, connectionID uint32, acceptTime time.Ti defer connCountPerUser.Add(c.User, -1) } + // Set initial db name. + if c.schemaName != "" { + err = l.handler.ComQuery(c, fmt.Sprintf("use `%s`", c.schemaName), func(result *sqltypes.Result) error { + return nil + }) + if err != nil { + c.writeErrorPacketFromError(err) + return + } + } + // Negotiation worked, send OK packet. if err := c.writeOKPacket(0, 0, c.StatusFlags, 0); err != nil { log.Errorf("Cannot write OK packet to %s: %v", c, err) @@ -454,13 +465,6 @@ func (l *Listener) handle(conn net.Conn, connectionID uint32, acceptTime time.Ti log.Warningf("Slow connection from %s: %v", c, connectTime) } - // Set initial db name. - if c.schemaName != "" { - l.handler.ComQuery(c, fmt.Sprintf("use `%s`", c.schemaName), func(result *sqltypes.Result) error { - return nil - }) - } - for { err := c.handleNextCommand(l.handler) if err != nil { diff --git a/go/vt/vtgate/mysql_protocol_test.go b/go/vt/vtgate/mysql_protocol_test.go index bff3958773c..ff4c938e19c 100644 --- a/go/vt/vtgate/mysql_protocol_test.go +++ b/go/vt/vtgate/mysql_protocol_test.go @@ -135,18 +135,8 @@ func TestMySQLProtocolExecuteUseStatement(t *testing.T) { } func TestMysqlProtocolInvalidDB(t *testing.T) { - c, err := mysqlConnect(&mysql.ConnParams{DbName: "invalidDB"}) - if err != nil { - t.Fatal(err) - } - defer c.Close() - - _, err = c.ExecuteFetch("select id from t1", 10, true /* wantfields */) - c.Close() - want := "vtgate: : keyspace invalidDB not found in vschema (errno 1105) (sqlstate HY000) during query: select id from t1" - if err == nil || err.Error() != want { - t.Errorf("exec with db:\n%v, want\n%s", err, want) - } + _, err := mysqlConnect(&mysql.ConnParams{DbName: "invalidDB"}) + require.EqualError(t, err, "vtgate: : Unknown database 'invalidDB' (errno 1049) (sqlstate 42000) (errno 1049) (sqlstate 42000)") } func TestMySQLProtocolClientFoundRows(t *testing.T) {