Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update sql status to not report changes for unstaged tables that have changed but not visibly. #8624

Merged
merged 2 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions go/libraries/doltcore/diff/table_deltas.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,81 @@ func (td TableDelta) HasSchemaChanged(ctx context.Context) (bool, error) {
return !fromSchemaHash.Equal(toSchemaHash), nil
}

func (td TableDelta) HasChangesIgnoringColumnTags(ctx context.Context) (bool, error) {

if td.FromTable == nil && td.ToTable == nil {
return true, nil
}

if td.IsAdd() || td.IsDrop() {
return true, nil
}

if td.IsRename() {
return true, nil
}

if td.HasFKChanges() {
return true, nil
}

fromRowDataHash, err := td.FromTable.GetRowDataHash(ctx)
if err != nil {
return false, err
}

toRowDataHash, err := td.ToTable.GetRowDataHash(ctx)
if err != nil {
return false, err
}

// Any change to the table data counts as a change
if !fromRowDataHash.Equal(toRowDataHash) {
return true, nil
}

fromTableHash, err := td.FromTable.HashOf()
if err != nil {
return false, err
}

toTableHash, err := td.FromTable.HashOf()
if err != nil {
return false, err
}

// If the data hashes have changed, the table has obviously changed.
if !fromTableHash.Equal(toTableHash) {
return true, nil
}

fromSchemaHash, err := td.FromTable.GetSchemaHash(ctx)
if err != nil {
return false, err
}

toSchemaHash, err := td.ToTable.GetSchemaHash(ctx)
if err != nil {
return false, err
}

// If neither data nor schema hashes have changed, the table is obviously the same.
if fromSchemaHash.Equal(toSchemaHash) {
return false, nil
}

// The schema hash has changed but the data has remained the same. We must inspect the schema to determine
// whether the change is observable or if only column tags have changed.

fromSchema, toSchema, err := td.GetSchemas(ctx)
if err != nil {
return false, err
}

// SchemasAreEqual correctly accounts for tags
return !schema.SchemasAreEqual(fromSchema, toSchema), nil
}

func (td TableDelta) HasDataChanged(ctx context.Context) (bool, error) {
// Database collation change is not a data change
if td.FromTable == nil && td.ToTable == nil {
Expand Down Expand Up @@ -445,6 +520,9 @@ func (td TableDelta) HasPrimaryKeySetChanged() bool {
}

func (td TableDelta) HasChanges() (bool, error) {
fromString := td.FromTable.DebugString(context.Background(), td.FromTable.NodeStore())
toString := td.ToTable.DebugString(context.Background(), td.ToTable.NodeStore())
_, _ = fromString, toString
hashChanged, err := td.HasHashChanged()
if err != nil {
return false, err
Expand Down
14 changes: 14 additions & 0 deletions go/libraries/doltcore/sqle/dtables/status_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ func newStatusItr(ctx *sql.Context, st *StatusTable) (*StatusItr, error) {
return nil, err
}

// Some tables may differ only in column tags and/or recorded conflicts.
// We try to make such changes invisible to users and shouldn't display them for unstaged tables.
changedUnstagedTables := make([]diff.TableDelta, 0, len(unstagedTables))
for _, unstagedTableDiff := range unstagedTables {
changed, err := unstagedTableDiff.HasChangesIgnoringColumnTags(ctx)
if err != nil {
return nil, err
}
if changed {
changedUnstagedTables = append(changedUnstagedTables, unstagedTableDiff)
}
}
unstagedTables = changedUnstagedTables

stagedSchemas, unstagedSchemas, err := diff.GetStagedUnstagedDatabaseSchemaDeltas(ctx, roots)
if err != nil {
return nil, err
Expand Down
31 changes: 29 additions & 2 deletions integration-tests/bats/sql-status.bats
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,32 @@ teardown() {
run dolt sql -r csv -q "select * from dolt_status ORDER BY status"
[ "$status" -eq 0 ]
[[ "$output" =~ 'dolt_docs,false,conflict' ]] || false
[[ "$output" =~ 'dolt_docs,false,modified' ]] || false
}
}

@test "sql-status: tables with no observable changes don't show in status but can be staged" {
dolt sql <<SQL
create table t1 (id int primary key);
insert into t1 values (1);
SQL
dolt add -A && dolt commit -m "create table t1"
dolt sql <<SQL
create table temp__t1 (id int primary key);
insert into temp__t1 values (1);
drop table t1;
rename table temp__t1 to t1;
SQL
dolt sql -q "select * from dolt_status;"
run dolt sql -q "select * from dolt_status;"
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 0 ]

run dolt diff t1
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 0 ]

dolt add t1

run dolt sql -r csv -q "select * from dolt_status;"
[ "$status" -eq 0 ]
[[ "$output" =~ "t1,true,modified" ]] || false
}
28 changes: 28 additions & 0 deletions integration-tests/bats/status.bats
Original file line number Diff line number Diff line change
Expand Up @@ -552,3 +552,31 @@ SQL
[[ "${lines[1]}" = "Your branch is ahead of 'origin/main' by 1 commit." ]] || false
[[ "${lines[2]}" = " (use \"dolt push\" to publish your local commits)" ]] || false
}

@test "status: tables with no observable changes don't show in status but can be staged" {
dolt sql <<SQL
create table t1 (id int primary key);
insert into t1 values (1);
SQL
dolt add -A && dolt commit -m "create table t1"
dolt sql <<SQL
create table temp__t1 (id int primary key);
insert into temp__t1 values (1);
drop table t1;
rename table temp__t1 to t1;
SQL
run dolt status
[ "$status" -eq 0 ]
[[ "$output" =~ "nothing to commit, working tree clean" ]] || false
[[ ! $output =~ "t1" ]] || false

run dolt diff t1
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 0 ]

dolt add t1

run dolt status
[ "$status" -eq 0 ]
[[ "$output" =~ "modified: t1" ]] || false
}
Loading