Skip to content

Commit

Permalink
Merge pull request #6459 from dolthub/fulghum/diff
Browse files Browse the repository at this point in the history
Log a SQL warning when historical values don't fit in `dolt_diff_<tablename>`'s schema
  • Loading branch information
fulghum authored Aug 7, 2023
2 parents 072c487 + fefe7d5 commit ce8beef
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 35 deletions.
6 changes: 4 additions & 2 deletions go/libraries/doltcore/rowconv/row_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ var IdentityConverter = &RowConverter{nil, true, nil}
// to take an extra action when a value cannot be automatically converted to the output data type.
type WarnFunction func(int, string, ...interface{})

var DatatypeCoercionFailureWarning = "unable to coerce value from field '%s' into latest column schema"

const DatatypeCoercionFailureWarning = "unable to coerce value from field '%s' into latest column schema"
const DatatypeCoercionFailureWarningCode int = 1105 // Since this our own custom warning we'll use 1105, the code for an unknown error

const TruncatedOutOfRangeValueWarning = "Truncated %v value: %v"
const TruncatedOutOfRangeValueWarningCode = 1292

// RowConverter converts rows from one schema to another
type RowConverter struct {
// FieldMapping is a mapping from source column to destination column
Expand Down
22 changes: 0 additions & 22 deletions go/libraries/doltcore/sqle/dtables/diff_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ 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 @@ -405,21 +398,6 @@ 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
10 changes: 3 additions & 7 deletions go/libraries/doltcore/sqle/dtables/prolly_row_conv.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,7 @@ func (c ProllyRowConverter) PutConverted(ctx context.Context, key, value val.Tup
return err
}

err = c.putFields(ctx, value, c.valProj, c.valDesc, c.nonPkTargetTypes, dstRow)
if err != nil {
return err
}

return nil
return c.putFields(ctx, value, c.valProj, c.valDesc, c.nonPkTargetTypes, dstRow)
}

func (c ProllyRowConverter) putFields(ctx context.Context, tup val.Tuple, proj val.OrdinalMapping, desc val.TupleDesc, targetTypes []sql.Type, dstRow []interface{}) error {
Expand All @@ -135,7 +130,8 @@ func (c ProllyRowConverter) putFields(ctx context.Context, tup val.Tuple, proj v
dstRow[j] = nil
err = nil
} else if !inRange {
return sql.ErrValueOutOfRange.New(f, t)
c.warnFn(rowconv.TruncatedOutOfRangeValueWarningCode, rowconv.TruncatedOutOfRangeValueWarning, t, f)
dstRow[j] = nil
} else if err != nil {
return err
}
Expand Down
14 changes: 12 additions & 2 deletions go/libraries/doltcore/sqle/enginetest/dolt_queries_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,18 @@ var DiffSystemTableScriptTests = []queries.ScriptTest{
{
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"},
{1, nil, nil, doltCommit, nil, nil, nil, doltCommit, "added"},
{1, "1234567890", 13, doltCommit, 1, nil, nil, doltCommit, "modified"},
},
ExpectedWarningsCount: 4,
},
{
Query: "SHOW WARNINGS;",
Expected: []sql.Row{
{"Warning", 1292, "Truncated tinyint value: 420"},
{"Warning", 1292, "Truncated tinyint value: 420"},
{"Warning", 1292, "Truncated varchar(10) value: 123456789012345"},
{"Warning", 1292, "Truncated varchar(10) value: 123456789012345"},
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/bats/system-tables.bats
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,8 @@ SQL

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
[[ $output =~ "1,,,,,,added" ]] || false
[[ $output =~ "1,1234567890,13,1,,,modified" ]] || false
}

@test "system-tables: query dolt_history_ system table" {
Expand Down

0 comments on commit ce8beef

Please sign in to comment.