Requested changes

go/cmd/dolt/commands/diff.go

@@ -208,7 +208,7 @@ func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _
 		defer closeFunc()
 	}
 
-	systemVarVal, err := setSystemVar(queryist, sqlCtx, apr.Contains(cli.SystemFlag))
+	updateSystemVar, systemVarVal, err := setSystemVar(queryist, sqlCtx, apr.Contains(cli.SystemFlag))
 
 	dArgs, err := parseDiffArgs(queryist, sqlCtx, apr)
 	if err != nil {
@@ -217,34 +217,46 @@ func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _
 
 	verr = diffUserTables(queryist, sqlCtx, dArgs)
 
-	query := fmt.Sprintf("SET @@dolt_show_system_tables = %d", systemVarVal)
-	_, _, _, err = queryist.Query(sqlCtx, query)
-	if err != nil {
-		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	if updateSystemVar {
+		query := fmt.Sprintf("SET @@dolt_show_system_tables = %t", systemVarVal)
+		_, _, _, err = queryist.Query(sqlCtx, query)
+		if err != nil {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+		}
 	}
 
 	return HandleVErrAndExitCode(verr, usage)
 }
 
-func setSystemVar(queryist cli.Queryist, sqlCtx *sql.Context, showSystem bool) (int8, error) {
+// setSystemVar sets the @@dolt_show_system_tables variable if necessary, and returns the value it must be
+// set to after the commands completion, if necessary.
+func setSystemVar(queryist cli.Queryist, sqlCtx *sql.Context, showSystem bool) (bool, bool, error) {
 	_, rowIter, _, err := queryist.Query(sqlCtx, "SHOW VARIABLES WHERE VARIABLE_NAME='dolt_show_system_tables'")
+	updateSystemVar := false
 	if err != nil {
-		return 0, err
+		return false, false, err
 	}
 
 	row, err := sql.RowIterToRows(sqlCtx, rowIter)
 	if err != nil {
-		return 0, err
+		return false, false, err
+	}
+	prevVal, err := GetInt8ColAsBool(row[0][1])
+	if err != nil {
+		return false, false, err
 	}
-	prevVal := row[0][1].(int8)
 
-	newVal := 0
+	newVal := false
 	if showSystem {
-		newVal = 1
+		newVal = true
 	}
-	query := fmt.Sprintf("SET @@dolt_show_system_tables = %d", newVal)
-	_, _, _, err = queryist.Query(sqlCtx, query)
-	return prevVal, err
+	if newVal != prevVal {
+		query := fmt.Sprintf("SET @@dolt_show_system_tables = %t", newVal)
+		_, _, _, err = queryist.Query(sqlCtx, query)
+		updateSystemVar = true
+	}
+
+	return updateSystemVar, prevVal, err
 }
 
 func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseError {

go/cmd/dolt/commands/utils.go

@@ -363,6 +363,18 @@ func GetTinyIntColAsBool(col interface{}) (bool, error) {
 	}
 }
 
+// GetInt8ColAsBool returns the value of an int8 column as a bool
+// This is necessary because Queryist may return an int8 column as a bool (when using SQLEngine)
+// or as a string (when using ConnectionQueryist).
+func GetInt8ColAsBool(col interface{}) (bool, error) {
+	switch v := col.(type) {
+	case int8:
+		return v == 1, nil
+	default:
+		return false, fmt.Errorf("unexpected type %T, was expecting int8", v)
+	}
+}
+
 // getInt64ColAsInt64 returns the value of an int64 column as a string
 // This is necessary because Queryist may return an int64 column as an int64 (when using SQLEngine)
 // or as a string (when using ConnectionQueryist).

integration-tests/bats/diff-system.expect

@@ -13,7 +13,7 @@ expect_with_defaults                                                   {dolt-rep
 
 expect {
     -re {dolt_log} {
-        puts "\diff did not maintain system tables variables"
+        puts "\diff did not preserve existing session variable @@dolt_show_system_tables"
         exit 1
     }
 }