mirror/dolt
1
0
Fork 0
mirror of https://github.com/dolthub/dolt.git synced 2026-01-25 18:49:36 -06:00
Code Issues Packages Projects Releases 123 Wiki Activity

Address reviewer comments

Browse Source
This commit is contained in:
Tan Yong Zhi
2022-09-10 20:26:42 +08:00
parent 74edb799fa
commit 5ab5f499fc
2 changed files with 121 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
go/cmd/dolt/commands/diff.go
View File

@@ -558,7 +558,6 @@ func diffRows(
if err != nil {
return errhand.VerboseErrorFromError(err)
}
defer rowWriter.Close(ctx)
// can't diff
if !diffable {
@@ -618,9 +617,11 @@ func diffRows(
}
defer rowIter.Close(sqlCtx)
defer rowWriter.Close(ctx)
var modifiedColNames map[string]bool
if dArgs.skinny {
modifiedColNames, err := getModifiedCols(sqlCtx, rowIter, unionSch, sch)
modifiedColNames, err = getModifiedCols(sqlCtx, rowIter, unionSch, sch)
if err != nil {
return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build()
}
@@ -629,7 +630,7 @@ func diffRows(
var filteredUnionSch sql.Schema
for _, s := range unionSch {
for colName := range modifiedColNames {
if strings.Contains(s.Name, colName) {
if s.Name == colName {
filteredUnionSch = append(filteredUnionSch, s)
}
}
@@ -643,17 +644,20 @@ func diffRows(
defer rowWriter.Close(ctx)
// reset the row iterator
rowIter.Close(sqlCtx)
err = rowIter.Close(sqlCtx)
if err != nil {
return errhand.BuildDError("Error closing row iterator:\n%s", query).AddCause(err).Build()
}
_, rowIter, err = se.Query(sqlCtx, query)
defer rowIter.Close(sqlCtx)
if sql.ErrSyntaxError.Is(err) {
return errhand.BuildDError("Failed to parse diff query. Invalid where clause?\nDiff query: %s", query).AddCause(err).Build()
} else if err != nil {
return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build()
}
defer rowIter.Close(sqlCtx)
}
err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter, dArgs.skinny)
err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter, modifiedColNames, dArgs.skinny)
if err != nil {
return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build()
}
@@ -697,6 +701,7 @@ func writeDiffResults(
targetSch sql.Schema,
iter sql.RowIter,
writer diff.SqlRowDiffWriter,
modifiedColNames map[string]bool,
filterChangedCols bool,
) error {
ds, err := newDiffSplitter(diffQuerySch, targetSch)
@@ -719,15 +724,19 @@ func writeDiffResults(
if filterChangedCols {
var filteredOldRow, filteredNewRow rowDiff
for i, changeType := range oldRow.colDiffs {
if changeType != diff.None || targetSch[i].PrimaryKey {
filteredOldRow.row = append(filteredOldRow.row, oldRow.row[i])
filteredOldRow.colDiffs = append(filteredOldRow.colDiffs, oldRow.colDiffs[i])
filteredOldRow.rowDiff = oldRow.rowDiff
for i, changeType := range newRow.colDiffs {
if (changeType == diff.Added|diff.Removed) || modifiedColNames[targetSch[i].Name] {
if i < len(oldRow.row) {
filteredOldRow.row = append(filteredOldRow.row, oldRow.row[i])
filteredOldRow.colDiffs = append(filteredOldRow.colDiffs, oldRow.colDiffs[i])
filteredOldRow.rowDiff = oldRow.rowDiff
}
filteredNewRow.row = append(filteredNewRow.row, newRow.row[i])
filteredNewRow.colDiffs = append(filteredNewRow.colDiffs, newRow.colDiffs[i])
filteredNewRow.rowDiff = newRow.rowDiff
if i < len(newRow.row) {
filteredNewRow.row = append(filteredNewRow.row, newRow.row[i])
filteredNewRow.colDiffs = append(filteredNewRow.colDiffs, newRow.colDiffs[i])
filteredNewRow.rowDiff = newRow.rowDiff
}
}
}
@@ -751,6 +760,10 @@ func writeDiffResults(
}
}
// getModifiedCols returns a set of the names of columns that are modified, as well as the name of the primary key for a particular row iterator and schema.
// In the case where rows are added or removed, all columns will be included
// unionSch refers to a joint schema between the schema before and after any schema changes pertaining to the diff,
// while diffQuerySch refers to the schema returned by the "dolt_diff" sql query.
func getModifiedCols(
ctx *sql.Context,
iter sql.RowIter,
@@ -769,11 +782,17 @@ func getModifiedCols(
return modifiedColNames, err
}
oldRow, _, err := ds.splitDiffResultRow(r)
oldRow, newRow, err := ds.splitDiffResultRow(r)
if err != nil {
return modifiedColNames, err
}
for i, changeType := range newRow.colDiffs {
if changeType != diff.None || unionSch[i].PrimaryKey {
modifiedColNames[unionSch[i].Name] = true
}
}
for i, changeType := range oldRow.colDiffs {
if changeType != diff.None || unionSch[i].PrimaryKey {
modifiedColNames[unionSch[i].Name] = true

91
integration-tests/bats/diff.bats
View File

@@ -720,16 +720,22 @@ SQL
[[ "$output" = 'UPDATE `t` SET `val1`=30,`val3`=4 WHERE `pk`=1;' ]] || false
}
@test "diff: skinny flag only shows row changed" {
dolt sql -q "create table t(pk int primary key, val1 int, val2 int)"
@test "diff: skinny flag only shows row changed without schema changes" {
dolt sql -q "CREATE TABLE t(pk int primary key, val1 int, val2 int)"
dolt add .
dolt sql -q "INSERT INTO t VALUES (1, 1, 1)"
dolt commit -am "cm1"
dolt sql -q "UPDATE t SET val1=2 where pk=1"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 'val2' ]] || false
dolt sql -q "UPDATE t SET val1=2 WHERE pk=1"
dolt commit -am "cm2"
dolt sql -q "UPDATE t SET val1=3 where pk=1"
dolt sql -q "UPDATE t SET val1=3 WHERE pk=1"
dolt commit -am "cm3"
run dolt diff --skinny HEAD~1
@@ -738,6 +744,83 @@ SQL
[[ "$output" =~ 'val1' ]] || false
}
@test "diff: skinny flag only shows row changed when both schema (column added) and data is changed (row updated)" {
dolt sql -q "create table t(pk int primary key, val1 int, val2 int)"
dolt add .
dolt sql -q "INSERT INTO t VALUES (1, 1, 1)"
dolt sql -q "INSERT INTO t VALUES (2, 2, 2)"
dolt commit -am "cm1"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 'val2' ]] || false
dolt sql -q "UPDATE t SET val1=3 WHERE pk=1"
dolt sql -q "ALTER TABLE t ADD val3 int "
dolt sql -q "UPDATE t SET val3=4 WHERE pk=1"
dolt commit -am "cm2"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 'val3' ]] || false
[[ ! "$output" =~ 'val2' ]] || false
}
@test "diff: skinny flag only shows row changed when both schema (column dropped) and data is changed (row updated)" {
dolt sql -q "create table t(pk int primary key, val1 int, s varchar(255))"
dolt add .
dolt sql -q "INSERT INTO t VALUES (1, 1, 'bla')"
dolt sql -q "INSERT INTO t VALUES (2, 2, 'bla2')"
dolt commit -am "cm1"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 's' ]] || false
dolt sql -q "ALTER TABLE t DROP COLUMN s"
dolt sql -q "UPDATE t SET val1=3 WHERE pk=1"
dolt sql -q "UPDATE t SET val1=4 WHERE pk=2"
dolt commit -am "cm2"
test=$(dolt diff --skinny --data HEAD~1)
echo $test
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 's' ]] || false
[[ ! "$output" =~ 'val3' ]] || false
}
@test "diff: skinny flag only shows row changed when data is changed (row deleted)" {
dolt sql -q "create table t(pk int primary key, val1 int, val2 int)"
dolt add .
dolt sql -q "INSERT INTO t VALUES (1, 1, 1)"
dolt sql -q "INSERT INTO t VALUES (2, 2, 2)"
dolt commit -am "cm1"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 'val2' ]] || false
dolt sql -q "DELETE FROM t WHERE pk=1"
dolt commit -am "cm2"
run dolt diff --skinny --data HEAD~1
[ $status -eq 0 ]
[[ "$output" =~ 'pk' ]] || false
[[ "$output" =~ 'val1' ]] || false
[[ "$output" =~ 'val2' ]] || false
}
@test "diff: keyless sql diffs" {
dolt sql -q "create table t(pk int, val int)"