Update dep github.com/skeema/tengo, and simplify pull and push

README.md

@@ -19,7 +19,7 @@ Pre-built `skeema` binaries for Linux and macOS will be supplied later in Q2, on
 
 ## Compiling
 
-Requires the [Go programming language toolchain](https://golang.org/dl/). Go version 1.6 or later is needed in order to properly use vendored dependencies.
+Requires the [Go programming language toolchain](https://golang.org/dl/). The latest version, Go version 1.10, is currently required. Older versions may be supported in the near future.
 
 To download, build, and install Skeema, run:
 

cmd_pull.go

@@ -109,87 +109,63 @@ func PullHandler(cfg *mybase.Config) error {
 		if err != nil {
 			return err
 		}
+
 		for _, td := range diff.TableDiffs {
-			stmt, err := td.Statement(mods)
-			if err != nil {
-				return err
+			stmt, stmtErr := td.Statement(mods)
+			if stmtErr != nil && !tengo.IsUnsupportedDiff(stmtErr) {
+				return stmtErr
 			}
 			// skip if mods caused the diff to be a no-op
-			if stmt == "" {
+			if stmt == "" && stmtErr == nil {
 				continue
 			}
-			switch td := td.(type) {
-			case tengo.CreateTable:
-				sf := SQLFile{
-					Dir:      t.Dir,
-					FileName: fmt.Sprintf("%s.sql", td.Table.Name),
-					Contents: stmt,
-				}
-				if length, err := sf.Write(); err != nil {
-					return fmt.Errorf("Unable to write to %s: %s", sf.Path(), err)
-				} else if _, hadErr := t.SQLFileErrors[sf.Path()]; hadErr {
-					// SQL files with syntax errors will result in tengo.CreateTable since
-					// the temp schema will be missing the table, however we can detect this
-					// scenario by looking in the Target's SQLFileErrors
-					log.Infof("Wrote %s (%d bytes) -- updated file to replace invalid SQL", sf.Path(), length)
-				} else {
-					log.Infof("Wrote %s (%d bytes) -- new table", sf.Path(), length)
-				}
-			case tengo.DropTable:
-				table := td.Table
+
+			// For DROP TABLE, we're deleting corresponding table file; vs other
+			// types we're updating/rewriting the file.
+			if td.Type == tengo.TableDiffDrop {
 				sf := SQLFile{
 					Dir:      t.Dir,
-					FileName: fmt.Sprintf("%s.sql", table.Name),
+					FileName: fmt.Sprintf("%s.sql", td.From.Name),
 				}
 				if err := sf.Delete(); err != nil {
 					return fmt.Errorf("Unable to delete %s: %s", sf.Path(), err)
 				}
 				log.Infof("Deleted %s -- table no longer exists", sf.Path())
-			case tengo.AlterTable:
-				table := td.Table
-				createStmt, err := t.Instance.ShowCreateTable(t.SchemaFromInstance.Name, table.Name)
-				if err != nil {
-					return err
-				}
-				sf := SQLFile{
-					Dir:      t.Dir,
-					FileName: fmt.Sprintf("%s.sql", table.Name),
-					Contents: createStmt,
-				}
-				var length int
-				if length, err = sf.Write(); err != nil {
-					return fmt.Errorf("Unable to write to %s: %s", sf.Path(), err)
-				}
-				log.Infof("Wrote %s (%d bytes) -- updated file to reflect table alterations", sf.Path(), length)
-			case tengo.RenameTable:
-				return fmt.Errorf("Table renames not yet supported")
-			default:
-				return fmt.Errorf("Unsupported diff type %T", td)
+				continue
 			}
-		}
 
-		// Tables that use features not supported by tengo diff still need files
-		// updated. Handle same as AlterTable case, since created/dropped tables don't
-		// ever end up in UnsupportedTables since they don't do a diff operation.
-		for _, table := range diff.UnsupportedTables {
-			createStmt := table.CreateStatement
-			if table.HasAutoIncrement() && !t.Dir.Config.GetBool("include-auto-inc") {
-				createStmt, _ = tengo.ParseCreateAutoInc(createStmt)
-			}
+			var reason string
 			sf := SQLFile{
 				Dir:      t.Dir,
-				FileName: fmt.Sprintf("%s.sql", table.Name),
-				Contents: createStmt,
+				FileName: fmt.Sprintf("%s.sql", td.To.Name),
+				Contents: stmt,
 			}
-			var length int
-			if length, err = sf.Write(); err != nil {
-				return fmt.Errorf("Unable to write to %s: %s", sf.Path(), err)
+
+			// For ALTER TABLE, we don't care about the ALTER statement, but we do
+			// need to get the corresponding CREATE TABLE and process auto-inc properly
+			if td.Type == tengo.TableDiffAlter {
+				sf.Contents = td.To.CreateStatement
+				if td.To.HasAutoIncrement() && !t.Dir.Config.GetBool("include-auto-inc") && td.From.NextAutoIncrement <= 1 {
+					sf.Contents, _ = tengo.ParseCreateAutoInc(sf.Contents)
+				}
+				reason = "updated file to reflect table alterations"
+				if tengo.IsUnsupportedDiff(stmtErr) {
+					log.Warnf("Table %s uses unsupported features", td.To.Name)
+					DebugLogUnsupportedDiff(stmtErr.(*tengo.UnsupportedDiffError))
+				}
+			} else if _, hadErr := t.SQLFileErrors[sf.Path()]; hadErr {
+				// SQL files with syntax errors will result in TableDiffCreate since the
+				// temp schema will be missing the table
+				reason = "updated file to replace invalid SQL"
+			} else {
+				reason = "new table"
 			}
-			log.Infof("Wrote %s (%d bytes) -- updated file to reflect (unsupported) table alterations", sf.Path(), length)
-			if t.Dir.Config.GetBool("debug") {
-				log.Warnf("Table %s uses unsupported features", table.Name)
-				t.logUnsupportedTableDiff(table.Name)
+
+			length, err := sf.Write()
+			if err != nil {
+				return fmt.Errorf("Unable to write to %s: %s", sf.Path(), err)
 			}
+			log.Infof("Wrote %s (%d bytes) -- %s", sf.Path(), length, reason)
 		}
 
 		if dir.Config.GetBool("normalize") {

cmd_push.go

@@ -209,15 +209,25 @@ func pushWorker(sps *sharedPushState) {
 			}
 			for n, tableDiff := range diff.TableDiffs {
 				ddl := NewDDLStatement(tableDiff, mods, t)
-				if ddl == nil {
-					// skip blank DDL (due to mods.NextAutoInc, mods.IgnoreTable, etc)
+				if ddl.IsNoop() {
 					continue
 				}
 				targetStmtCount++
+				if err, ok := ddl.Err.(*tengo.UnsupportedDiffError); ok {
+					sps.incrementUnsupportedCount()
+					log.Warnf("Skipping table %s: unable to generate ALTER TABLE due to use of unsupported features. Use --debug for more information.", err.Name)
+					DebugLogUnsupportedDiff(err)
+					continue
+				}
 				sps.incrementDiffCount()
 				if ddl.Err != nil {
-					log.Errorf("%s. The affected DDL statement will be skipped. See --help for more information.", ddl.Err)
 					sps.incrementErrCount(1)
+					if tengo.IsForbiddenDiff(ddl.Err) {
+						log.Errorf("%s due to supplied options. The affected DDL statement will be skipped. See --help for more information.", ddl.Err)
+					} else {
+						log.Errorf("A fatal error occurred with pre-processing a DDL statement: %s. The affected DDL statement will be skipped.", ddl.Err)
+						continue
+					}
 				}
 				sps.syncPrintf(t.Instance, schemaName, "%s\n", ddl.String())
 				if !sps.dryRun && ddl.Err == nil && ddl.Execute() != nil {
@@ -230,16 +240,6 @@ func pushWorker(sps *sharedPushState) {
 					break
 				}
 			}
-			for _, table := range diff.UnsupportedTables {
-				sps.incrementUnsupportedCount()
-				targetStmtCount++
-				if t.Dir.Config.GetBool("debug") {
-					log.Warnf("Skipping table %s: unable to generate ALTER TABLE due to use of unsupported features", table.Name)
-					t.logUnsupportedTableDiff(table.Name)
-				} else {
-					log.Warnf("Skipping table %s: unable to generate ALTER TABLE due to use of unsupported features. Use --debug for more information.", table.Name)
-				}
-			}
 
 			if targetStmtCount == 0 {
 				log.Infof("%s %s: No differences found\n", t.Instance, schemaName)

ddlstatement.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/skeema/tengo"
@@ -15,10 +14,10 @@ import (
 // run directly against a DB.
 type DDLStatement struct {
 	// Err represents errors that occur from applying statement modifiers (which
-	// can forbid destructive DDL), from building an external command string (which
-	// could reference invalid template variables), or from executing the DDL
-	// (errors from the DB directly, or a nonzero exit code from an external
-	// command)
+	// can forbid destructive DDL), from tables using unsupported features,
+	// from building an external command string (which could reference invalid
+	// template variables), or from executing the DDL (errors from the DB directly,
+	// or a nonzero exit code from an external command)
 	Err error
 
 	stmt     string
@@ -28,34 +27,27 @@ type DDLStatement struct {
 	schemaName string
 }
 
-// NewDDLStatement creates and returns a DDLStatement. It may return nil if
-// the StatementModifiers cause it to be a no-op. In the case of an error
+// NewDDLStatement creates and returns a DDLStatement. In the case of an error
 // constructing the statement (mods disallowing destructive DDL, invalid
-// variable interpolation in --alter-wrapper, etc), a non-nil DDLStatement will
-// still be returned, but its Err field will be non-nil, preventing any
-// execution of the DDLStatement.
-func NewDDLStatement(diff tengo.TableDiff, mods tengo.StatementModifiers, target *Target) *DDLStatement {
+// variable interpolation in --alter-wrapper, etc), the returned value's Err
+// field will be non-nil, preventing any execution of the DDLStatement.
+func NewDDLStatement(diff *tengo.TableDiff, mods tengo.StatementModifiers, target *Target) *DDLStatement {
 	ddl := &DDLStatement{
 		instance:   target.Instance,
 		schemaName: target.SchemaFromDir.Name,
 	}
 	var err error
 
-	// Look up size of affected table. This will be 0 for CREATE TABLE statements.
+	// Gather name and size of affected table. Size will be 0 for CREATE TABLE statements.
 	var tableName string
 	var tableSize int64
-	switch diff := diff.(type) {
-	case tengo.AlterTable:
-		tableSize, err = ddl.getTableSize(target, diff.Table)
-		tableName = diff.Table.Name
-	case tengo.DropTable:
-		tableSize, err = ddl.getTableSize(target, diff.Table)
-		tableName = diff.Table.Name
-	case tengo.CreateTable:
-		tableName = diff.Table.Name
-		err = nil
+	if diff.From != nil { // ALTER or DROP
+		tableName = diff.From.Name
+		tableSize, err = ddl.getTableSize(target, diff.From)
+		ddl.setErr(err)
+	} else { // CREATE
+		tableName = diff.To.Name
 	}
-	ddl.setErr(err)
 
 	// If --safe-below-size option in use, enable additional statement modifier
 	// if the table's size is less than the supplied option value
@@ -68,7 +60,7 @@ func NewDDLStatement(diff tengo.TableDiff, mods tengo.StatementModifiers, target
 
 	// Options may indicate some/all DDL gets executed by shelling out to another program.
 	wrapper := target.Dir.Config.Get("ddl-wrapper")
-	if _, isAlter := diff.(tengo.AlterTable); isAlter && target.Dir.Config.Changed("alter-wrapper") {
+	if diff.Type == tengo.TableDiffAlter && target.Dir.Config.Changed("alter-wrapper") {
 		minSize, err := target.Dir.Config.GetBytes("alter-wrapper-min-size")
 		ddl.setErr(err)
 		if tableSize >= int64(minSize) {
@@ -92,14 +84,13 @@ func NewDDLStatement(diff tengo.TableDiff, mods tengo.StatementModifiers, target
 		}
 	}
 
-	// Get the raw DDL statement as a string.
+	// Get the raw DDL statement as a string. This may set stmt to a blank string
+	// if the statement should be skipped due to mods, or if the diff could not
+	// be generated due to use of unsupported table features.
 	ddl.stmt, err = diff.Statement(mods)
 	ddl.setErr(err)
 	if ddl.stmt == "" {
-		// mods may result in a statement that should be skipped, but not due to
-		// error. For example, the only change may be to next-auto-inc value, which
-		// mods specify to ignore. This is represented by a nil DDLStatement.
-		return nil
+		return ddl
 	}
 
 	// Apply wrapper if relevant
@@ -111,28 +102,13 @@ func NewDDLStatement(diff tengo.TableDiff, mods tengo.StatementModifiers, target
 			"DDL":    ddl.stmt,
 			"TABLE":  tableName,
 			"SIZE":   strconv.FormatInt(tableSize, 10),
+			"TYPE":   diff.TypeString(),
 		}
+		extras["CLAUSES"], _ = diff.Clauses(mods)
 		if ddl.instance.SocketPath != "" {
 			delete(extras, "PORT")
 			extras["SOCKET"] = ddl.instance.SocketPath
 		}
-
-		switch diff := diff.(type) {
-		case tengo.AlterTable:
-			prefix := fmt.Sprintf("%s ", diff.Table.AlterStatement())
-			extras["CLAUSES"] = strings.Replace(ddl.stmt, prefix, "", 1)
-			extras["TYPE"] = "ALTER"
-		case tengo.CreateTable:
-			prefix := fmt.Sprintf("CREATE TABLE %s ", tengo.EscapeIdentifier(diff.Table.Name))
-			extras["CLAUSES"] = strings.Replace(ddl.stmt, prefix, "", 1)
-			extras["TYPE"] = "CREATE"
-		case tengo.DropTable:
-			extras["CLAUSES"] = ""
-			extras["TYPE"] = "DROP"
-		default: // currently includes case tengo.RenameTable
-			ddl.setErr(fmt.Errorf("TableDiff type %T not yet supported", diff))
-		}
-
 		ddl.shellOut, err = NewInterpolatedShellOut(wrapper, target.Dir, extras)
 		ddl.setErr(err)
 	}
@@ -147,12 +123,18 @@ func (ddl *DDLStatement) IsShellOut() bool {
 	return (ddl.shellOut != nil)
 }
 
+// IsNoop returns true if the DDL statement should be skipped due to the
+// statement modifiers supplied to the constructor.
+func (ddl *DDLStatement) IsNoop() bool {
+	return ddl.stmt == "" && ddl.Err == nil
+}
+
 // String returns a string representation of ddl. If an external command is in
 // use, the returned string will be prefixed with "\!", the MySQL CLI command
 // shortcut for "system" shellout. If ddl.Err is non-nil, the returned string
 // will be commented-out by wrapping in /* ... */ long-style comment.
 func (ddl *DDLStatement) String() string {
-	if ddl == nil {
+	if ddl.stmt == "" {
 		return ""
 	}
 	var stmt string
@@ -171,17 +153,14 @@ func (ddl *DDLStatement) String() string {
 // or shelling out to an external program, as appropriate.
 func (ddl *DDLStatement) Execute() error {
 	// Refuse to execute no-ops or errors
-	if ddl == nil {
-		return nil
+	if ddl.IsNoop() {
+		return errors.New("Attempted to execute empty DDL statement")
 	} else if ddl.Err != nil {
 		return ddl.Err
 	}
 	if ddl.IsShellOut() {
 		ddl.Err = ddl.shellOut.Run()
 	} else {
-		if ddl.stmt == "" {
-			return errors.New("Attempted to execute empty DDL statement")
-		}
 		if db, err := ddl.instance.Connect(ddl.schemaName, ""); err != nil {
 			ddl.Err = err
 		} else {

ddlstatement_test.go

@@ -61,16 +61,16 @@ func (s *SkeemaIntegrationSuite) TestNewDDLStatement(t *testing.T) {
 			t.Fatalf("Expected this configuration to result in all DDLs being shellouts, but %v is not", ddl)
 		}
 		var expected string
-		switch diff := diff.(type) {
-		case tengo.AlterTable:
-			if diff.Table.Name == "rollups" {
+		switch diff.Type {
+		case tengo.TableDiffAlter:
+			if diff.To.Name == "rollups" {
 				// no rows, so ddl-wrapper used. verify the statement separately.
 				expected = "/bin/echo ddl-wrapper analytics.rollups ALTER"
 				expectedStmt := "ALTER TABLE `rollups` ALGORITHM=INPLACE, LOCK=NONE, ADD COLUMN `value` bigint(20) DEFAULT NULL"
 				if ddl.stmt != expectedStmt {
 					t.Errorf("Expected statement:\n%s\nActual statement:\n%s\n", expectedStmt, ddl.stmt)
 				}
-			} else if diff.Table.Name == "pageviews" {
+			} else if diff.To.Name == "pageviews" {
 				// has 1 row, so alter-wrapper used. verify the execution separately to
 				// sanity-check the quoting rules.
 				expected = "/bin/echo alter-wrapper analytics.pageviews ALTER 'ADD COLUMN `domain` varchar(40) NOT NULL DEFAULT '\"'\"'skeema.net'\"'\"''"
@@ -79,11 +79,11 @@ func (s *SkeemaIntegrationSuite) TestNewDDLStatement(t *testing.T) {
 					t.Errorf("Expected output:\n%sActual output:\n%sErr:\n%v\n", expectedOutput, actualOutput, err)
 				}
 			} else {
-				t.Fatalf("Unexpected AlterTable for %s; perhaps test fixture changed without updating this test?", diff.Table.Name)
+				t.Fatalf("Unexpected AlterTable for %s; perhaps test fixture changed without updating this test?", diff.To.Name)
 			}
-		case tengo.DropTable:
+		case tengo.TableDiffDrop:
 			expected = "/bin/echo ddl-wrapper analytics.widget_counts DROP"
-		case tengo.CreateTable:
+		case tengo.TableDiffCreate:
 			expected = "/bin/echo ddl-wrapper analytics.activity CREATE"
 		}
 		if ddl.shellOut.Command != expected {