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

Log a SQL warning when historical values don't fit in dolt_diff_<tablename>'s schema #6459

Merged
merged 4 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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 incorrect %v value: %v"
fulghum marked this conversation as resolved.
Show resolved Hide resolved
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 incorrect tinyint value: 420"},
{"Warning", 1292, "Truncated incorrect tinyint value: 420"},
{"Warning", 1292, "Truncated incorrect varchar(10) value: 123456789012345"},
{"Warning", 1292, "Truncated incorrect varchar(10) value: 123456789012345"},
},
},
},
Expand Down
Loading