From 15bfa7b3d9067ab16502de65b3990fdcb27d0b57 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Fri, 7 Jul 2023 17:17:11 +0300 Subject: [PATCH] resolved conflict (#13457) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/mysql/constants.go | 1 + go/vt/mysqlctl/schema.go | 16 +++++++- go/vt/vttablet/tabletserver/schema/engine.go | 38 +++++++++++++++---- .../tabletserver/schema/engine_test.go | 8 ++-- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/go/mysql/constants.go b/go/mysql/constants.go index bcbfd6ae931..54c92a9b4b5 100644 --- a/go/mysql/constants.go +++ b/go/mysql/constants.go @@ -536,6 +536,7 @@ const ( ERDataTooLong = 1406 ErrWrongValueForType = 1411 ERWarnDataTruncated = 1265 + ERNoSuchUser = 1449 ERForbidSchemaChange = 1450 ERDataOutOfRange = 1690 ERInvalidJSONText = 3140 diff --git a/go/vt/mysqlctl/schema.go b/go/vt/mysqlctl/schema.go index c5c10afdc7f..273cbca0f2c 100644 --- a/go/vt/mysqlctl/schema.go +++ b/go/vt/mysqlctl/schema.go @@ -41,6 +41,14 @@ import ( var autoIncr = regexp.MustCompile(` AUTO_INCREMENT=\d+`) +type EmptyColumnsErr struct { + dbName, tableName, query string +} + +func (e EmptyColumnsErr) Error() string { + return fmt.Sprintf("unable to get columns for table %s.%s using query %s", e.dbName, e.tableName, e.query) +} + // executeSchemaCommands executes some SQL commands, using the mysql // command line tool. It uses the dba connection parameters, with credentials. func (mysqld *Mysqld) executeSchemaCommands(sql string) error { @@ -287,6 +295,10 @@ const ( GetFieldsQuery = "SELECT %s FROM %s WHERE 1 != 1" ) +// GetColumnsList returns the column names for a given table/view, using a query generating function. +// Returned values: +// - selectColumns: a string of comma delimited qualified names to be used in a SELECT query. e.g. "`id`, `name`, `val`" +// - err: error func GetColumnsList(dbName, tableName string, exec func(string, int, bool) (*sqltypes.Result, error)) (string, error) { var dbName2 string if dbName == "" { @@ -300,8 +312,8 @@ func GetColumnsList(dbName, tableName string, exec func(string, int, bool) (*sql return "", err } if qr == nil || len(qr.Rows) == 0 { - err = fmt.Errorf("unable to get columns for table %s.%s using query %s", dbName, tableName, query) - log.Errorf("%s", fmt.Errorf("unable to get columns for table %s.%s using query %s", dbName, tableName, query)) + err := &EmptyColumnsErr{dbName: dbName, tableName: tableName, query: query} + log.Error(err.Error()) return "", err } selectColumns := "" diff --git a/go/vt/vttablet/tabletserver/schema/engine.go b/go/vt/vttablet/tabletserver/schema/engine.go index 06e469de0f0..1cb15abb74b 100644 --- a/go/vt/vttablet/tabletserver/schema/engine.go +++ b/go/vt/vttablet/tabletserver/schema/engine.go @@ -18,26 +18,29 @@ package schema import ( "bytes" + "context" "encoding/json" + "errors" "fmt" "net/http" + "strings" "sync" "time" - "vitess.io/vitess/go/stats" - "vitess.io/vitess/go/vt/dbconnpool" - "vitess.io/vitess/go/vt/schema" - "vitess.io/vitess/go/vt/vtgate/evalengine" - - "context" - "vitess.io/vitess/go/acl" "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/stats" "vitess.io/vitess/go/timer" + "vitess.io/vitess/go/vt/concurrency" "vitess.io/vitess/go/vt/dbconfigs" + "vitess.io/vitess/go/vt/dbconnpool" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/mysqlctl" + "vitess.io/vitess/go/vt/mysqlctl/tmutils" + "vitess.io/vitess/go/vt/schema" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" @@ -341,6 +344,7 @@ func (se *Engine) reload(ctx context.Context) error { return err } + rec := concurrency.AllErrorRecorder{} // curTables keeps track of tables in the new snapshot so we can detect what was dropped. curTables := map[string]bool{"dual": true} // changedTables keeps track of tables that have changed so we can reload their pk info. @@ -376,9 +380,24 @@ func (se *Engine) reload(ctx context.Context) error { } log.V(2).Infof("Reading schema for table: %s", tableName) + tableType := row[1].String() table, err := LoadTable(conn, se.cp.DBName(), tableName, row[3].ToString()) if err != nil { - log.Warningf("Failed reading schema for the table: %s, error: %v", tableName, err) + isView := strings.Contains(tableType, tmutils.TableView) + var emptyColumnsError mysqlctl.EmptyColumnsErr + if errors.As(err, &emptyColumnsError) && isView { + log.Warningf("Failed reading schema for the table: %s, error: %v", tableName, err) + continue + } + sqlErr, isSQLErr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError) + if isSQLErr && sqlErr != nil && sqlErr.Number() == mysql.ERNoSuchUser && isView { + // A VIEW that has an invalid DEFINER, leading to: + // ERROR 1449 (HY000): The user specified as a definer (...) does not exist + log.Warningf("Failed reading schema for the table: %s, error: %v", tableName, err) + continue + } + // Non recoverable error: + rec.RecordError(vterrors.Wrapf(err, "in Engine.reload(), reading table %s", tableName)) continue } table.FileSize = fileSize @@ -391,6 +410,9 @@ func (se *Engine) reload(ctx context.Context) error { created = append(created, tableName) } } + if rec.HasErrors() { + return rec.Error() + } // Compute and handle dropped tables. var dropped []string diff --git a/go/vt/vttablet/tabletserver/schema/engine_test.go b/go/vt/vttablet/tabletserver/schema/engine_test.go index 5339887b5c2..29a74a28021 100644 --- a/go/vt/vttablet/tabletserver/schema/engine_test.go +++ b/go/vt/vttablet/tabletserver/schema/engine_test.go @@ -444,18 +444,16 @@ func TestOpenFailedDueToLoadTableErr(t *testing.T) { db.AddQueryPattern(fmt.Sprintf(mysql.GetColumnNamesQueryPatternForTable, "test_view"), sqltypes.MakeTestResult(sqltypes.MakeTestFields("column_name", "varchar"), "")) // rejecting the impossible query - db.AddRejectedQuery("SELECT * FROM `fakesqldb`.`test_view` WHERE 1 != 1", errors.New("The user specified as a definer ('root'@'%') does not exist (errno 1449) (sqlstate HY000)")) + db.AddRejectedQuery("SELECT * FROM `fakesqldb`.`test_view` WHERE 1 != 1", mysql.NewSQLErrorFromError(errors.New("The user specified as a definer ('root'@'%') does not exist (errno 1449) (sqlstate HY000)"))) AddFakeInnoDBReadRowsResult(db, 0) se := newEngine(10, 1*time.Second, 1*time.Second, db) err := se.Open() - // failed load should not return any error, instead should be logged. - require.NoError(t, err) + // failed load should return an error because of test_table + assert.ErrorContains(t, err, "Row count exceeded") logs := tl.GetAllLogs() logOutput := strings.Join(logs, ":::") - assert.Contains(t, logOutput, "WARNING:Failed reading schema for the table: test_table") - assert.Contains(t, logOutput, "Row count exceeded") assert.Contains(t, logOutput, "WARNING:Failed reading schema for the table: test_view") assert.Contains(t, logOutput, "The user specified as a definer ('root'@'%') does not exist (errno 1449) (sqlstate HY000)") }