Skip to content

Commit

Permalink
mounter(ticdc): fix mounter default value panic and data inconsistency (
Browse files Browse the repository at this point in the history
  • Loading branch information
maxshuang authored and overvenus committed Feb 22, 2022
1 parent 7519d6c commit 93b9dd0
Show file tree
Hide file tree
Showing 3 changed files with 257 additions and 31 deletions.
30 changes: 25 additions & 5 deletions cdc/entry/mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ func UnmarshalDDL(raw *model.RawKVEntry) (*timodel.Job, error) {
if err != nil {
return nil, errors.Trace(err)
}
log.Debug("get new DDL job", zap.String("detail", job.String()))
if !job.IsDone() && !job.IsSynced() {
return nil, nil
}
Expand Down Expand Up @@ -449,25 +450,41 @@ func formatColVal(datum types.Datum, tp byte) (value interface{}, warn string, e
}
return v, warn, nil
default:
// FIXME: GetValue() may return some types that go sql not support, which will cause sink DML fail
// NOTICE: GetValue() may return some types that go sql not support, which will cause sink DML fail
// Make specified convert upper if you need
// Go sql support type ref to: https://github.com/golang/go/blob/go1.17.4/src/database/sql/driver/types.go#L236
return datum.GetValue(), "", nil
}
}

// Scenarios when call this function:
// (1) column define default null at creating + insert without explicit column
// (2) alter table add column default xxx + old existing data
// (3) amend + insert without explicit column + alter table add column default xxx
// (4) online DDL drop column + data insert at state delete-only
//
// getDefaultOrZeroValue return interface{} need to meet to require type in
// https://github.com/golang/go/blob/go1.17.4/src/database/sql/driver/types.go#L236
// Supported type is: nil, basic type(Int, Int8,..., Float32, Float64, String), Slice(uint8), other types not support
func getDefaultOrZeroValue(col *timodel.ColumnInfo) (interface{}, string, error) {
var d types.Datum
// NOTICE: SHOULD use OriginDefaultValue here, more info pls ref to
// https://github.com/pingcap/tiflow/issues/4048
// FIXME: Too many corner cases may hit here, like type truncate, timezone
// (1) If this column is uk(no pk), will cause data inconsistency in Scenarios(2)
// (2) If not fix here, will cause data inconsistency in Scenarios(3) directly
// Ref: https://github.com/pingcap/tidb/blob/d2c352980a43bb593db81fd1db996f47af596d91/table/column.go#L489
if col.GetOriginDefaultValue() != nil {
d = types.NewDatum(col.GetOriginDefaultValue())
return d.GetValue(), "", nil
}

if !mysql.HasNotNullFlag(col.Flag) {
// see https://github.com/pingcap/tidb/issues/9304
// NOTICE: NotNullCheck need do after OriginDefaultValue check, as when TiDB meet "amend + add column default xxx",
// ref: https://github.com/pingcap/ticdc/issues/3929
// must use null if TiDB not write the column value when default value is null
// and the value is null
// and the value is null, see https://github.com/pingcap/tidb/issues/9304
d = types.NewDatum(nil)
} else if col.GetDefaultValue() != nil {
d = types.NewDatum(col.GetDefaultValue())
} else {
switch col.Tp {
case mysql.TypeEnum:
Expand All @@ -478,6 +495,9 @@ func getDefaultOrZeroValue(col *timodel.ColumnInfo) (interface{}, string, error)
return emptyBytes, "", nil
default:
d = table.GetZeroValue(col)
if d.IsNull() {
log.Error("meet unsupported column type", zap.String("column info", col.String()))
}
}
}

Expand Down
Loading

0 comments on commit 93b9dd0

Please sign in to comment.