internal refactor: SQL error number handling

This commit refactors code relating to detection of specific server error
numbers. Previously, this logic relied on constants in an external package
github.com/VividCortex/mysqlerr, but that package is obsolete: it is no
longer being updated with new errors from recent server releases.

In this commit, we now track only the relevant error codes internally, and
expose new helper functions which are used by the appropriate callsites.

This refactor was motivated by an overall desire to avoid unnecessary
external module dependencies, which was made more urgent due to MySQL 8.4's
introduction of new error codes (to be handled in a subsequent commit).

This commit also removes tengo.IsAccessError(), which was dead code.

go.mod

@@ -3,7 +3,6 @@ module github.com/skeema/skeema
 go 1.21
 
 require (
-	github.com/VividCortex/mysqlerr v1.0.0
 	github.com/go-sql-driver/mysql v1.8.1
 	github.com/jmoiron/sqlx v1.3.5
 	github.com/mattn/goveralls v0.0.11

go.sum

@@ -1,13 +1,9 @@
 filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
-github.com/VividCortex/mysqlerr v1.0.0 h1:5pZ2TZA+YnzPgzBfiUWGqWmKDVNBdrkf9g+DNe1Tiq8=
-github.com/VividCortex/mysqlerr v1.0.0/go.mod h1:xERx8E4tBhLvpjzdUyQiSfUxeMcATEQrflDAfXsqcAE=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
-github.com/go-sql-driver/mysql v1.8.0 h1:UtktXaU2Nb64z/pLiGIxY4431SJ4/dR5cjMmlVHgnT4=
-github.com/go-sql-driver/mysql v1.8.0/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg=
 github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpvNJ1Y=
 github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg=
 github.com/jmoiron/sqlx v1.3.5 h1:vFFPA71p1o5gAeqtEAwLU4dnX2napprKtHr7PYIcN3g=

internal/fs/dir.go

@@ -12,7 +12,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/VividCortex/mysqlerr"
 	log "github.com/sirupsen/logrus"
 	"github.com/skeema/mybase"
 	"github.com/skeema/skeema/internal/shellout"
@@ -368,14 +367,14 @@ func (dir *Dir) FirstInstance() (*tengo.Instance, error) {
 func (dir *Dir) ValidateInstance(instance *tengo.Instance) error {
 	ok, err := instance.Valid()
 	if !ok {
-		if instance.Password == "" && tengo.IsDatabaseError(err, mysqlerr.ER_ACCESS_DENIED_ERROR) {
+		if instance.Password == "" && tengo.IsAccessDeniedError(err) {
 			err = fmt.Errorf("%w\nNo password was supplied for this login attempt, but the server likely requires a password. For information on how to use Skeema's password option, see https://www.skeema.io/docs/options/#password", err)
 		} else if dir.Config.Changed("connect-options") {
-			if tengo.IsDatabaseError(err, mysqlerr.ER_SPECIFIC_ACCESS_DENIED_ERROR) {
+			if tengo.IsAccessPrivilegeError(err) {
 				err = fmt.Errorf("%w\nCheck your Skeema configuration for connect-options. Typically this error means one of your session variables requires privileges that your user lacks.\nFor more information, see https://www.skeema.io/docs/options/#connect-options", err)
-			} else if tengo.IsDatabaseError(err, mysqlerr.ER_UNKNOWN_SYSTEM_VARIABLE, mysqlerr.ER_INCORRECT_GLOBAL_LOCAL_VAR) {
+			} else if tengo.IsSessionVarNameError(err) {
 				err = fmt.Errorf("%w\nCheck your Skeema configuration for connect-options. Typically this error means one of your session variable names has a typo, or the variable does not exist at the session scope in your specific server version.", err)
-			} else if tengo.IsDatabaseError(err, mysqlerr.ER_WRONG_VALUE_FOR_VAR, mysqlerr.ER_WRONG_TYPE_FOR_VAR) {
+			} else if tengo.IsSessionVarValueError(err) {
 				err = fmt.Errorf("%w\nCheck your Skeema configuration for connect-options. Typically this error means one of your session variable values has a typo, or the value is not supported in your specific server version.", err)
 			}
 		}

internal/tengo/errors.go

@@ -3,14 +3,37 @@ package tengo
 import (
 	"errors"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/go-sql-driver/mysql"
 )
 
+// Constants mapping to database server error numbers
+// Useful reference: https://dev.mysql.com/doc/mysql-errors/8.0/en/server-error-reference.html
+const (
+	ER_PARSE_ERROR  = 1064
+	ER_SYNTAX_ERROR = 1149
+
+	ER_NO_SUCH_TABLE        = 1146
+	ER_SP_DOES_NOT_EXIST    = 1305
+	ER_TRG_DOES_NOT_EXIST   = 1360
+	ER_EVENT_DOES_NOT_EXIST = 1539
+
+	ER_LOCK_DEADLOCK     = 1213
+	ER_LOCK_WAIT_TIMEOUT = 1205
+
+	ER_UNKNOWN_SYSTEM_VARIABLE    = 1193
+	ER_INCORRECT_GLOBAL_LOCAL_VAR = 1238
+	ER_GLOBAL_VARIABLE            = 1229
+	ER_WRONG_VALUE_FOR_VAR        = 1231
+	ER_WRONG_TYPE_FOR_VAR         = 1232
+
+	ER_ACCESS_DENIED_ERROR          = 1045
+	ER_SPECIFIC_ACCESS_DENIED_ERROR = 1227
+)
+
 // IsDatabaseError returns true if err came from a database server, typically
 // as a response to a query or connection attempt.
 // If one or more specificErrors are supplied, IsDatabaseError only returns true
-// if the database error code matched one of those numbers.
+// if the database error code also matched one of those numbers.
 func IsDatabaseError(err error, specificErrors ...uint16) bool {
 	var merr *mysql.MySQLError
 	if errors.As(err, &merr) {
@@ -26,25 +49,47 @@ func IsDatabaseError(err error, specificErrors ...uint16) bool {
 	return false
 }
 
-// IsSyntaxError returns true if err is a SQL syntax error, or false otherwise.
+// IsSyntaxError returns true if err is a SQL syntax or parsing error.
 func IsSyntaxError(err error) bool {
-	return IsDatabaseError(err, mysqlerr.ER_PARSE_ERROR, mysqlerr.ER_SYNTAX_ERROR)
-}
-
-// IsAccessError returns true if err indicates an authentication or authorization
-// problem, at connection time or query time. Can be a problem with credentials,
-// client host, no access to requested default database, missing privilege, etc.
-// There is no sense in immediately retrying the connection or query when
-// encountering this type of error.
-func IsAccessError(err error) bool {
-	authErrors := []uint16{
-		mysqlerr.ER_ACCESS_DENIED_ERROR,
-		mysqlerr.ER_BAD_HOST_ERROR,
-		mysqlerr.ER_DBACCESS_DENIED_ERROR,
-		mysqlerr.ER_BAD_DB_ERROR,
-		mysqlerr.ER_HOST_NOT_PRIVILEGED,
-		mysqlerr.ER_HOST_IS_BLOCKED,
-		mysqlerr.ER_SPECIFIC_ACCESS_DENIED_ERROR,
-	}
-	return IsDatabaseError(err, authErrors...)
+	return IsDatabaseError(err, ER_PARSE_ERROR, ER_SYNTAX_ERROR)
+}
+
+// IsObjectNotFoundError returns true if err is a response from SHOW CREATE
+// which indicates the named object does not exist.
+func IsObjectNotFoundError(err error) bool {
+	return IsDatabaseError(err, ER_NO_SUCH_TABLE, ER_SP_DOES_NOT_EXIST, ER_TRG_DOES_NOT_EXIST, ER_EVENT_DOES_NOT_EXIST)
+}
+
+// IsConcurrentDDLError returns true if err is a type of error that can manifest
+// from running DDL concurrently, indicating the client should retry the DDL
+// serially instead.
+func IsConcurrentDDLError(err error) bool {
+	// * MDL conflicts can result in ER_LOCK_WAIT_TIMEOUT
+	// * Out-of-order CREATE TABLE...LIKE can result in ER_NO_SUCH_TABLE
+	// * FK-related situations can result in ER_LOCK_DEADLOCK
+	return IsDatabaseError(err, ER_LOCK_DEADLOCK, ER_LOCK_WAIT_TIMEOUT, ER_NO_SUCH_TABLE)
+}
+
+// IsSessionVarNameError returns true if err indicates a session variable name
+// does not exist, or is read-only, or only exists at the global scope.
+func IsSessionVarNameError(err error) bool {
+	return IsDatabaseError(err, ER_UNKNOWN_SYSTEM_VARIABLE, ER_INCORRECT_GLOBAL_LOCAL_VAR, ER_GLOBAL_VARIABLE)
+}
+
+// IsSessionVarValueError returns true if err indicates a session variable has an
+// invalid value.
+func IsSessionVarValueError(err error) bool {
+	return IsDatabaseError(err, ER_WRONG_VALUE_FOR_VAR, ER_WRONG_TYPE_FOR_VAR)
+}
+
+// IsAccessDeniedError returns true if err indicates a problem with the username
+// or password upon attempting to authenticate during a new connection.
+func IsAccessDeniedError(err error) bool {
+	return IsDatabaseError(err, ER_ACCESS_DENIED_ERROR)
+}
+
+// IsAccessPrivilegeError returns true if err indicates a lack of a required
+// privilege grant.
+func IsAccessPrivilegeError(err error) bool {
+	return IsDatabaseError(err, ER_SPECIFIC_ACCESS_DENIED_ERROR)
 }

internal/tengo/errors_test.go

@@ -2,7 +2,7 @@ package tengo
 
 import (
 	"errors"
-	"fmt"
+	"strings"
 	"testing"
 )
 
@@ -17,55 +17,77 @@ func (s TengoIntegrationSuite) TestIsDatabaseError(t *testing.T) {
 	}
 }
 
-func (s TengoIntegrationSuite) TestIsSyntaxError(t *testing.T) {
+func (s TengoIntegrationSuite) TestDatabaseErrorTypeFunctions(t *testing.T) {
+	// Test IsSyntaxError
 	err := errors.New("non-db error")
 	if IsSyntaxError(err) {
 		t.Errorf("IsSyntaxError unexpectedly returned true for non-database error type=%T", err)
 	}
-
 	db, err := s.d.ConnectionPool("testing", "")
 	if err != nil {
 		t.Fatalf("Unable to get connection")
 	}
-	_, err = db.Exec("ALTER TAABBEL actor ENGINE=InnoDB")
-	if err == nil {
+	_, syntaxErr := db.Exec("ALTER TAABBEL actor ENGINE=InnoDB")
+	if syntaxErr == nil {
 		t.Error("Bad syntax still returned nil error unexpectedly")
-
-	} else if !IsSyntaxError(err) {
-		t.Errorf("Error of type %T %+v unexpectedly not considered syntax error", err, err)
+	} else if !IsSyntaxError(syntaxErr) {
+		t.Errorf("Error of type %T %+v unexpectedly not considered syntax error", syntaxErr, syntaxErr)
 	}
-	_, err = db.Exec("ALTER TABLE doesnt_exist ENGINE=InnoDB")
-	if err == nil {
+	_, doesntExistErr := db.Exec("ALTER TABLE doesnt_exist ENGINE=InnoDB")
+	if doesntExistErr == nil {
 		t.Error("Bad alter still returned nil error unexpectedly")
-	} else if IsSyntaxError(err) {
-		t.Errorf("Error of type %T %+v unexpectedly considered syntax error", err, err)
+	} else if IsSyntaxError(doesntExistErr) {
+		t.Errorf("Error of type %T %+v unexpectedly considered syntax error", doesntExistErr, doesntExistErr)
 	}
-}
 
-func (s TengoIntegrationSuite) TestIsAccessError(t *testing.T) {
-	err := errors.New("non-db error")
-	if IsAccessError(err) {
-		t.Errorf("IsAccessError unexpectedly returned true for non-database error type=%T", err)
+	// Test IsObjectNotFoundError
+	if !IsObjectNotFoundError(doesntExistErr) {
+		t.Errorf("Error of type %T %+v unexpectedly not considered not-found error", doesntExistErr, doesntExistErr)
+	}
+	if IsObjectNotFoundError(syntaxErr) {
+		t.Errorf("Error of type %T %+v unexpectedly considered not-found error", syntaxErr, syntaxErr)
+	}
+
+	// Test IsSessionVarNameError and IsSessionVarValueError
+	_, invalidVarNameErr := s.d.ConnectionPool("testing", "invalidvar='hello'")
+	_, globalOnlyVarNameErr := s.d.ConnectionPool("", "concurrent_insert=1")
+	_, readOnlyVarNameErr := s.d.ConnectionPool("", "version_comment='hello'")
+	_, invalidVarValueErr := s.d.ConnectionPool("testing", "sql_mode='superduperdb'")
+	_, invalidVarValTypeErr := s.d.ConnectionPool("testing", "wait_timeout='hello'")
+	if !IsSessionVarNameError(invalidVarNameErr) {
+		t.Errorf("Incorrect behavior of IsSessionVarNameError: expected true for %T %+v, but false was returned", invalidVarNameErr, invalidVarNameErr)
+	}
+	if !IsSessionVarNameError(globalOnlyVarNameErr) {
+		t.Errorf("Incorrect behavior of IsSessionVarNameError: expected true for %T %+v, but false was returned", globalOnlyVarNameErr, globalOnlyVarNameErr)
+	}
+	if !IsSessionVarNameError(readOnlyVarNameErr) {
+		t.Errorf("Incorrect behavior of IsSessionVarNameError: expected true for %T %+v, but false was returned", readOnlyVarNameErr, readOnlyVarNameErr)
+	}
+	if IsSessionVarNameError(invalidVarValueErr) || IsSessionVarNameError(invalidVarValTypeErr) {
+		t.Error("Incorrect behavior of IsSessionVarNameError: expected false, but true was returned")
+	}
+	if !IsSessionVarValueError(invalidVarValueErr) {
+		t.Errorf("Incorrect behavior of IsSessionVarValueError: expected true for %T %+v, but false was returned", invalidVarValueErr, invalidVarValueErr)
+	}
+	if !IsSessionVarValueError(invalidVarValTypeErr) {
+		t.Errorf("Incorrect behavior of IsSessionVarValueError: expected true for %T %+v, but false was returned", invalidVarValTypeErr, invalidVarValTypeErr)
+	}
+	if IsSessionVarValueError(invalidVarNameErr) || IsSessionVarValueError(globalOnlyVarNameErr) || IsSessionVarValueError(readOnlyVarNameErr) {
+		t.Error("Incorrect behavior of IsSessionVarValueError: expected false, but true was returned")
 	}
 
+	// Test IsAccessDeniedError
 	// Hack username in DSN to no longer be correct
-	inst := s.d.Instance
-	inst.BaseDSN = fmt.Sprintf("badname%s", inst.BaseDSN)
-	_, err = inst.ConnectionPool("", "")
-	if err == nil {
-		t.Error("ConnectionPool unexpectedly returned nil error")
-	} else if !IsAccessError(err) {
-		t.Errorf("Error of type %T %+v unexpectedly not considered access error", err, err)
-	}
-	inst.BaseDSN = inst.BaseDSN[7:]
-	db, err := inst.ConnectionPool("testing", "")
-	if err != nil {
-		t.Errorf("ConnectionPool unexpectedly returned error: %s", err)
+	s.d.Instance.BaseDSN = strings.Replace(s.d.Instance.BaseDSN, s.d.Instance.Password, "wrongpw", 1)
+	_, accessDeniedErr := s.d.Instance.ConnectionPool("", "")
+	if !IsAccessDeniedError(accessDeniedErr) {
+		t.Errorf("Error of type %T %+v unexpectedly not considered access denied error", accessDeniedErr, accessDeniedErr)
 	}
-	_, err = db.Exec("ALTER TABLE doesnt_exist ENGINE=InnoDB")
-	if err == nil {
-		t.Error("Bad alter still returned nil error unexpectedly")
-	} else if IsAccessError(err) {
-		t.Errorf("Error of type %T %+v unexpectedly considered access error", err, err)
+	s.d.Instance.BaseDSN = strings.Replace(s.d.Instance.BaseDSN, "wrongpw", s.d.Instance.Password, 1)
+	if IsAccessDeniedError(doesntExistErr) {
+		t.Errorf("Error of type %T %+v unexpectedly considered access denied error", doesntExistErr, doesntExistErr)
+	}
+	if IsAccessPrivilegeError(accessDeniedErr) {
+		t.Error("Incorrect behavior of IsAccessPrivilegeError")
 	}
 }

internal/tengo/instance.go

@@ -10,7 +10,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/go-sql-driver/mysql"
 	"github.com/jmoiron/sqlx"
 	"golang.org/x/sync/errgroup"
@@ -728,7 +727,7 @@ func (instance *Instance) DropSchema(schema string, opts BulkDropOptions) error
 	// Now that the tables have been removed, we can drop the schema without
 	// risking a long lock impacting the DB negatively
 	err = dropSchema(db, schema)
-	if IsDatabaseError(err, mysqlerr.ER_LOCK_WAIT_TIMEOUT) {
+	if IsConcurrentDDLError(err) {
 		// we do 1 retry upon seeing a metadata locking conflict, consistent with
 		// logic in DropTablesInSchema
 		err = dropSchema(db, schema)
@@ -881,7 +880,7 @@ func (instance *Instance) DropTablesInSchema(schema string, opts BulkDropOptions
 				}
 			}
 			_, err := db.ExecContext(ctx, "DROP TABLE "+EscapeIdentifier(name))
-			if IsDatabaseError(err, mysqlerr.ER_LOCK_DEADLOCK, mysqlerr.ER_LOCK_WAIT_TIMEOUT) {
+			if IsConcurrentDDLError(err) {
 				// If foreign keys are being used, DROP TABLE can encounter lock wait
 				// timeouts in various situations in MySQL 8.0+ or MariaDB 10.6+, due to
 				// concurrent drops or due to cross-schema FKs. MySQL 8.0+ can also

internal/tengo/routine.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/jmoiron/sqlx"
 	"golang.org/x/sync/errgroup"
 )
@@ -509,7 +508,7 @@ func showCreateRoutine(ctx context.Context, db *sqlx.DB, routine string, ot Obje
 			CreateStatement sql.NullString `db:"Create Procedure"`
 		}
 		err = db.SelectContext(ctx, &createRows, query)
-		if (err == nil && len(createRows) != 1) || IsDatabaseError(err, mysqlerr.ER_SP_DOES_NOT_EXIST) {
+		if (err == nil && len(createRows) != 1) || IsObjectNotFoundError(err) {
 			err = sql.ErrNoRows
 		} else if err == nil {
 			create = createRows[0].CreateStatement.String
@@ -519,7 +518,7 @@ func showCreateRoutine(ctx context.Context, db *sqlx.DB, routine string, ot Obje
 			CreateStatement sql.NullString `db:"Create Function"`
 		}
 		err = db.SelectContext(ctx, &createRows, query)
-		if (err == nil && len(createRows) != 1) || IsDatabaseError(err, mysqlerr.ER_SP_DOES_NOT_EXIST) {
+		if (err == nil && len(createRows) != 1) || IsObjectNotFoundError(err) {
 			err = sql.ErrNoRows
 		} else if err == nil {
 			create = createRows[0].CreateStatement.String

internal/workspace/localdocker.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/jmoiron/sqlx"
 	log "github.com/sirupsen/logrus"
 	"github.com/skeema/skeema/internal/tengo"
@@ -195,7 +194,7 @@ func (ld *LocalDocker) ConnectionPool(params string) (*sqlx.DB, error) {
 	// This can happen if overriding flavor on the command-line, or even
 	// automatically if the real server runs 5.7 but local machine is ARM.
 	// In this case, try conn again with all non-portable sql_mode values removed.
-	if tengo.IsDatabaseError(err, mysqlerr.ER_WRONG_VALUE_FOR_VAR) && strings.Contains(finalParams, "sql_mode") {
+	if tengo.IsSessionVarValueError(err) && strings.Contains(err.Error(), "sql_mode") && strings.Contains(finalParams, "sql_mode") {
 		v, _ := url.ParseQuery(finalParams)
 		sqlMode := v.Get("sql_mode")
 		if len(sqlMode) > 1 {

internal/workspace/workspace.go

@@ -13,7 +13,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/go-sql-driver/mysql"
 	"github.com/jmoiron/sqlx"
 	log "github.com/sirupsen/logrus"
@@ -365,7 +364,7 @@ func ExecLogicalSchema(logicalSchema *fs.LogicalSchema, opts Options) (_ *Schema
 	for n := 0; n < len(logicalSchema.Creates); n++ {
 		if err := <-errs; err != nil {
 			stmterr := err.(*StatementError)
-			if tengo.IsDatabaseError(stmterr.Err, mysqlerr.ER_LOCK_DEADLOCK, mysqlerr.ER_LOCK_WAIT_TIMEOUT, mysqlerr.ER_NO_SUCH_TABLE) {
+			if tengo.IsConcurrentDDLError(stmterr.Err) {
 				sequentialStatements = append(sequentialStatements, stmterr.Statement)
 			} else {
 				wsSchema.Failures = append(wsSchema.Failures, stmterr)

internal/workspace/workspace_test.go

@@ -9,7 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/VividCortex/mysqlerr"
 	"github.com/skeema/mybase"
 	"github.com/skeema/skeema/internal/fs"
 	"github.com/skeema/skeema/internal/tengo"
@@ -125,8 +124,8 @@ func (s WorkspaceIntegrationSuite) TestExecLogicalSchemaErrors(t *testing.T) {
 	if errorText := err.Error(); errorText == "" {
 		t.Error("Unexpectedly found blank error text")
 	}
-	if errNo := wsSchema.Failures[0].ErrorNumber(); errNo != mysqlerr.ER_PARSE_ERROR {
-		t.Errorf("Expected StatementError.ErrorNumber() to return %d, instead found %d", mysqlerr.ER_PARSE_ERROR, errNo)
+	if !tengo.IsSyntaxError(wsSchema.Failures[0].Err) {
+		t.Errorf("Expected StatementError to be a syntax error; instead found %s", wsSchema.Failures[0])
 	}
 
 	// Test handling of fatal error