Skip to content

Commit

Permalink
Merge pull request #6399 from dolthub/fulghum/dolt-6391
Browse files Browse the repository at this point in the history
Bug fix: `dolt_diff_<tablename>` correctly shows historical values after narrowing types
  • Loading branch information
fulghum authored Jul 27, 2023
2 parents 282c298 + 19cd6f2 commit c6a88c8
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 11 deletions.
33 changes: 22 additions & 11 deletions go/libraries/doltcore/sqle/dtables/diff_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@ func NewDiffTable(ctx *sql.Context, tblName string, ddb *doltdb.DoltDB, root *do
return nil, err
}

// Process sch to widen all the fields so that any previous values (in the same type family) can be
// shown in the diff table, even if the current schema's type has been changed to a smaller type.
sch, err = widenSchemaColumns(sch)
if err != nil {
return nil, err
}

diffTableSchema, j, err := GetDiffTableSchemaAndJoiner(ddb.Format(), sch, sch)
if err != nil {
return nil, err
Expand Down Expand Up @@ -398,6 +405,21 @@ func (dt *DiffTable) reverseIterForChild(ctx *sql.Context, parent hash.Hash) (*d
}
}

// widenSchemaColumns takes a schema, |sch|, and returns a new schema with all the non-PK columns
// promoted to their widest type in the same family (e.g. tinyint -> bigint, varchar(10) -> TEXT).
// This function is used so that when a table's schema has changed throughout its history, we can
// still show any previous values in the diff table, even if those previous values are larger than
// the current schema's type.
func widenSchemaColumns(sch schema.Schema) (schema.Schema, error) {
widenedCols := make([]schema.Column, 0, sch.GetAllCols().Size())
for _, col := range sch.GetAllCols().GetColumns() {
col.TypeInfo = col.TypeInfo.Promote()
widenedCols = append(widenedCols, col)
}
return schema.NewSchema(schema.NewColCollection(widenedCols...),
[]int{}, sch.GetCollation(), sch.Indexes(), sch.Checks())
}

func tableInfoForCommit(ctx context.Context, table string, cm *doltdb.Commit, hs hash.Hash) (TblInfoAtCommit, error) {
r, err := cm.GetRootValue(ctx)
if err != nil {
Expand Down Expand Up @@ -706,17 +728,6 @@ type DiffPartitions struct {
fromSch schema.Schema
}

func NewDiffPartitions(tblName string, cmItr doltdb.CommitItr, cmHashToTblInfo map[hash.Hash]TblInfoAtCommit, selectFunc partitionSelectFunc, toSch, fromSch schema.Schema) *DiffPartitions {
return &DiffPartitions{
tblName: tblName,
cmItr: cmItr,
cmHashToTblInfo: cmHashToTblInfo,
selectFunc: selectFunc,
toSch: toSch,
fromSch: fromSch,
}
}

// processCommit is called in a commit iteration loop. Adds partitions when it finds a commit and its parent that have
// different values for the hash of the table being looked at.
func (dps *DiffPartitions) processCommit(ctx *sql.Context, cmHash hash.Hash, cm *doltdb.Commit, root *doltdb.RootValue, tbl *doltdb.Table) (*DiffPartition, error) {
Expand Down
23 changes: 23 additions & 0 deletions go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,29 @@ var DiffSystemTableScriptTests = []queries.ScriptTest{
},
},
},
{
// https://github.com/dolthub/dolt/issues/6391
Name: "columns modified to narrower types",
SetUpScript: []string{
"create table t (pk int primary key, col1 varchar(20), col2 int);",
"call dolt_commit('-Am', 'new table t');",
"insert into t values (1, '123456789012345', 420);",
"call dolt_commit('-am', 'inserting data');",
"update t set col1='1234567890', col2=13;",
"alter table t modify column col1 varchar(10);",
"alter table t modify column col2 tinyint;",
"call dolt_commit('-am', 'narrowing types');",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "select to_pk, to_col1, to_col2, to_commit, from_pk, from_col1, from_col2, from_commit, diff_type from dolt_diff_t order by diff_type ASC;",
Expected: []sql.Row{
{1, "123456789012345", 420, doltCommit, nil, nil, nil, doltCommit, "added"},
{1, "1234567890", 13, doltCommit, 1, "123456789012345", 420, doltCommit, "modified"},
},
},
},
},
{
Name: "multiple column renames",
SetUpScript: []string{
Expand Down
16 changes: 16 additions & 0 deletions integration-tests/bats/system-tables.bats
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,22 @@ SQL
[[ "$output" =~ "$EXPECTED" ]] || false
}

@test "system-tables: query dolt_diff_ system table when column types have been narrowed" {
dolt sql -q "create table t (pk int primary key, col1 varchar(20), col2 int);"
dolt commit -Am "new table t"
dolt sql -q "insert into t values (1, '123456789012345', 420);"
dolt commit -am "inserting a row"
dolt sql -q "update t set col1='1234567890', col2=13;"
dolt sql -q "alter table t modify column col1 varchar(10);"
dolt sql -q "alter table t modify column col2 tinyint;"
dolt commit -am "narrowing types"

run dolt sql -r csv -q "select to_pk, to_col1, to_col2, from_pk, from_col1, from_col2, diff_type from dolt_diff_t order by from_commit_date ASC;"
[ $status -eq 0 ]
[[ $output =~ "1,123456789012345,420,,,,added" ]] || false
[[ $output =~ "1,1234567890,13,1,123456789012345,420,modified" ]] || false
}

@test "system-tables: query dolt_history_ system table" {
dolt sql -q "create table test (pk int, c1 int, primary key(pk))"
dolt add test
Expand Down

0 comments on commit c6a88c8

Please sign in to comment.