diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 74f05f45f6..b14f56b0e6 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -179,6 +179,7 @@ func (cmd DiffCmd) ArgParser() *argparser.ArgParser { ap.SupportsString(DiffMode, "", "diff mode", "Determines how to display modified rows with tabular output. Valid values are row, line, in-place, context. Defaults to context.") ap.SupportsFlag(ReverseFlag, "R", "Reverses the direction of the diff.") ap.SupportsFlag(NameOnlyFlag, "", "Only shows table names.") + ap.SupportsFlag(cli.SystemFlag, "", "Show system tables in addition to user tables") return ap } @@ -199,8 +200,7 @@ func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _ return HandleVErrAndExitCode(verr, usage) } - queryist, oldSqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) - sqlCtx := doltdb.ContextWithDoltCICreateBypassKey(oldSqlCtx) + queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx) if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } @@ -208,13 +208,62 @@ func (cmd DiffCmd) Exec(ctx context.Context, commandStr string, args []string, _ defer closeFunc() } + updateSystemVar, err := setSystemVar(queryist, sqlCtx, apr.Contains(cli.SystemFlag)) + if err != nil { + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) + } + dArgs, err := parseDiffArgs(queryist, sqlCtx, apr) if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } verr = diffUserTables(queryist, sqlCtx, dArgs) - return HandleVErrAndExitCode(verr, usage) + if verr != nil { + return HandleVErrAndExitCode(verr, usage) + } + + if updateSystemVar != nil { + err = updateSystemVar() + } + + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) +} + +// 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) (func() error, error) { + _, rowIter, _, err := queryist.Query(sqlCtx, "SHOW VARIABLES WHERE VARIABLE_NAME='dolt_show_system_tables'") + if err != nil { + return nil, err + } + + row, err := sql.RowIterToRows(sqlCtx, rowIter) + if err != nil { + return nil, err + } + prevVal, err := GetInt8ColAsBool(row[0][1]) + if err != nil { + return nil, err + } + + newVal := false + var update func() error + + if showSystem { + newVal = true + } + if newVal != prevVal { + query := fmt.Sprintf("SET @@dolt_show_system_tables = %t", newVal) + _, _, _, err = queryist.Query(sqlCtx, query) + update = func() error { + query := fmt.Sprintf("SET @@dolt_show_system_tables = %t", prevVal) + _, _, _, err := queryist.Query(sqlCtx, query) + return err + } + } + + return update, err } func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseError { diff --git a/go/cmd/dolt/commands/utils.go b/go/cmd/dolt/commands/utils.go index 8c3c47e19b..44c4db94da 100644 --- a/go/cmd/dolt/commands/utils.go +++ b/go/cmd/dolt/commands/utils.go @@ -363,6 +363,20 @@ 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 != 0, nil + case string: + return v != "0", 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). diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index ed13b11b9d..4fb712f651 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -82,7 +82,7 @@ func DoltCICanBypass(ctx context.Context) bool { // HasDoltPrefix returns a boolean whether or not the provided string is prefixed with the DoltNamespace. Users should // not be able to create tables in this reserved namespace. func HasDoltPrefix(s string) bool { - return strings.HasPrefix(strings.ToLower(s), DoltNamespace) && !HasDoltCIPrefix(s) + return strings.HasPrefix(strings.ToLower(s), DoltNamespace) } // HasDoltCIPrefix returns a boolean whether or not the provided string is prefixed with the DoltCINamespace. Users should diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index 583b74a7ae..7dcc4769dc 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -976,7 +976,7 @@ func (db Database) GetTableNamesAsOf(ctx *sql.Context, time interface{}) ([]stri return nil, err } - return filterDoltInternalTables(ctx, tblNames, db.schemaName), nil + return filterDoltInternalTables(tblNames, db.schemaName, showSystemTables), nil } // getTable returns the user table with the given baseName from the root given @@ -1194,7 +1194,7 @@ func (db Database) GetTableNames(ctx *sql.Context) ([]string, error) { } // TODO: Figure out way to remove filterDoltInternalTables - return filterDoltInternalTables(ctx, tblNames, db.schemaName), nil + return filterDoltInternalTables(tblNames, db.schemaName, showSystemTables), nil } func (db Database) SchemaName() string { @@ -1245,14 +1245,12 @@ func (db Database) getAllTableNames(ctx *sql.Context, root doltdb.RootValue, inc return result, nil } -func filterDoltInternalTables(ctx *sql.Context, tblNames []string, schemaName string) []string { +func filterDoltInternalTables(tblNames []string, schemaName string, includeWriteable bool) []string { result := []string{} for _, tbl := range tblNames { - if doltdb.IsDoltCITable(tbl) { - if doltdb.DoltCICanBypass(ctx) { - result = append(result, tbl) - } + if includeWriteable && !doltdb.IsReadOnlySystemTable(doltdb.TableName{Name: tbl, Schema: schemaName}) { + result = append(result, tbl) } else if !doltdb.IsSystemTable(doltdb.TableName{Name: tbl, Schema: schemaName}) { result = append(result, tbl) } @@ -1435,7 +1433,7 @@ func (db Database) CreateTable(ctx *sql.Context, tableName string, sch sql.Prima return err } - if doltdb.IsSystemTable(doltdb.TableName{Name: tableName, Schema: db.schemaName}) && !doltdb.IsFullTextTable(tableName) { + if doltdb.IsSystemTable(doltdb.TableName{Name: tableName, Schema: db.schemaName}) && !doltdb.IsFullTextTable(tableName) && !doltdb.HasDoltCIPrefix(tableName) { return ErrReservedTableName.New(tableName) } diff --git a/integration-tests/bats/ci.bats b/integration-tests/bats/ci.bats index c8e3330af1..6bbf27a97c 100644 --- a/integration-tests/bats/ci.bats +++ b/integration-tests/bats/ci.bats @@ -117,8 +117,7 @@ get_commit_hash() { dolt ci init last=$(get_commit_hash 1) - dolt diff "$first" "$last" - run dolt diff "$first" "$last" + run dolt diff "$first" "$last" --system [ "$status" -eq 0 ] [[ ${output} == *"dolt_ci"* ]] || false } @@ -224,7 +223,7 @@ jobs: EOF dolt ci import ./workflow_1_updated.yaml updated=$(get_commit_hash 1) - run dolt diff "$original" "$updated" + run dolt diff "$original" "$updated" --system [ "$status" -eq 0 ] [[ ${output} == *"(new)"* ]] || false [[ ${output} == *"dolt_ci_workflow_steps"* ]] || false diff --git a/integration-tests/bats/diff-system.expect b/integration-tests/bats/diff-system.expect new file mode 100755 index 0000000000..1a80d6d0cb --- /dev/null +++ b/integration-tests/bats/diff-system.expect @@ -0,0 +1,30 @@ +#!/usr/bin/expect + +set timeout 5 +set env(NO_COLOR) 1 + +source "$env(BATS_CWD)/helper/common_expect_functions.tcl" + +spawn dolt sql + +expect_with_defaults {dolt-repo-.*>} { send "\\diff HEAD HEAD~1 --system\r"; } + +expect_with_defaults {dolt-repo-.*>} { send "show tables;\r"; } + +expect { + -re {dolt_log} { + puts "\diff did not preserve existing session variable @@dolt_show_system_tables" + exit 1 + } +} + +expect_with_defaults {dolt-repo-.*>} { send "SET @@dolt_show_system_tables = 1;\r"; } + +expect_with_defaults {dolt-repo-.*>} { send "\\diff HEAD HEAD~1 \r"; } + +expect_with_defaults {dolt-repo-.*>} { send "show tables;\r"; } + +expect_with_defaults_2 {dolt_log} {dolt-repo-.*>} { send "quit;\r"; } + +expect eof +exit \ No newline at end of file diff --git a/integration-tests/bats/diff.bats b/integration-tests/bats/diff.bats index e54d785763..d540d2f3ba 100644 --- a/integration-tests/bats/diff.bats +++ b/integration-tests/bats/diff.bats @@ -2183,4 +2183,27 @@ EOF [[ "$output" =~ "$EXPECTED" ]] || false # Count the line numbers to make sure there are no schema changes output [ "${#lines[@]}" -eq 3 ] +} + +@test "diff: diff only shows system tables with --system flag" { + dolt sql -q "insert into dolt_ignore values ('test', 1)" + dolt add . + dolt commit -m "added row to dolt_ignore" + + run dolt diff HEAD HEAD~1 + [ "$status" -eq 0 ] + ! [[ "$output" =~ "dolt_ignore" ]] || false + + run dolt diff HEAD HEAD~1 --system + [ "$status" -eq 0 ] + [[ "$output" =~ "dolt_ignore" ]] || false +} + +# bats test_tags=no_lambda +@test "diff: --system preserves dolt_show_system_tables value in sql-shell" { + skiponwindows "Need to install expect and make this script work on windows." + dolt commit --allow-empty -m "Commit" + + run $BATS_TEST_DIRNAME/diff-system.expect + [ "$status" -eq 0 ] } \ No newline at end of file