schema.Reload(): ignore column reading errors for views only, error f…

…or tables (vitessio#13442) * schema.Reload(): ignore column reading errors for views only, error for tables Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * restore to pre-PR Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * add ERNoSuchUser error (view's definer not found) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Error Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * ignore either missing columns or unknown definer -- for views Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * adapt tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * cleanup Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * update comment Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
slackhq · Jul 7, 2023 · 105ae38 · 105ae38
1 parent 1b5640f
commit 105ae38
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 10 deletions.
diff --git a/go/mysql/constants.go b/go/mysql/constants.go
@@ -554,6 +554,7 @@ const (
 	ERIllegalValueForType          = ErrorCode(1367)
 	ERDataTooLong                  = ErrorCode(1406)
 	ErrWrongValueForType           = ErrorCode(1411)
+	ERNoSuchUser                   = ErrorCode(1449)
 	ERForbidSchemaChange           = ErrorCode(1450)
 	ERWrongValue                   = ErrorCode(1525)
 	ERDataOutOfRange               = ErrorCode(1690)

diff --git 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. It uses the dba connection parameters, with credentials.
 func (mysqld *Mysqld) executeSchemaCommands(ctx context.Context, sql string) error {
 	params, err := mysqld.dbcfgs.DbaConnector().MysqlParams()
@@ -283,6 +291,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 == "" {
@@ -296,8 +308,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
@@ -20,14 +20,19 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
 	"golang.org/x/exp/maps"
 
 	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/concurrency"
+	"vitess.io/vitess/go/vt/mysqlctl"
+	"vitess.io/vitess/go/vt/mysqlctl/tmutils"
 	"vitess.io/vitess/go/vt/sidecardb"
 
 	"vitess.io/vitess/go/acl"
@@ -441,6 +446,7 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) 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.
@@ -491,9 +497,24 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) error {
 		}
 
 		log.V(2).Infof("Reading schema for table: %s", tableName)
-		table, err := LoadTable(conn, se.cp.DBName(), tableName, row[1].String(), row[3].ToString())
+		tableType := row[1].String()
+		table, err := LoadTable(conn, se.cp.DBName(), tableName, tableType, 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
 		}
 		if includeStats {
@@ -508,6 +529,9 @@ func (se *Engine) reload(ctx context.Context, includeStats bool) error {
 			created = append(created, table)
 		}
 	}
+	if rec.HasErrors() {
+		return rec.Error()
+	}
 
 	dropped := se.getDroppedTables(curTables, changedViews, mismatchTables)
 

diff --git a/go/vt/vttablet/tabletserver/schema/engine_test.go b/go/vt/vttablet/tabletserver/schema/engine_test.go
@@ -447,18 +447,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, 0, 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)")
 }

diff --git a/go/vt/vttablet/tabletserver/schema/load_table.go b/go/vt/vttablet/tabletserver/schema/load_table.go
@@ -26,6 +26,7 @@ import (
 	"vitess.io/vitess/go/mysql/collations"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/mysqlctl"
+	"vitess.io/vitess/go/vt/mysqlctl/tmutils"
 	querypb "vitess.io/vitess/go/vt/proto/query"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vtgate/evalengine"
@@ -48,7 +49,7 @@ func LoadTable(conn *connpool.DBConn, databaseName, tableName, tableType string,
 			return nil, err
 		}
 		ta.Type = Message
-	case strings.Contains(tableType, "VIEW"):
+	case strings.Contains(tableType, tmutils.TableView):
 		ta.Type = View
 	}
 	return ta, nil