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

ddl: fix corrupted default value for bit type column (#18036) #20340

Merged
merged 6 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 6 additions & 2 deletions ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,18 @@ func onDropColumn(t *meta.Meta, job *model.Job) (ver int64, _ error) {
// When the dropping column has not-null flag and it hasn't the default value, we can backfill the column value like "add column".
// NOTE: If the state of StateWriteOnly can be rollbacked, we'd better reconsider the original default value.
// And we need consider the column without not-null flag.
if colInfo.OriginDefaultValue == nil && mysql.HasNotNullFlag(colInfo.Flag) {
if colInfo.GetOriginDefaultValue() == nil && mysql.HasNotNullFlag(colInfo.Flag) {
// If the column is timestamp default current_timestamp, and DDL owner is new version TiDB that set column.Version to 1,
// then old TiDB update record in the column write only stage will uses the wrong default value of the dropping column.
// Because new version of the column default value is UTC time, but old version TiDB will think the default value is the time in system timezone.
// But currently will be ok, because we can't cancel the drop column job when the job is running,
// so the column will be dropped succeed and client will never see the wrong default value of the dropped column.
// More info about this problem, see PR#9115.
colInfo.OriginDefaultValue, err = generateOriginDefaultValue(colInfo)
oldDVal, err := generateOriginDefaultValue(colInfo)
if err != nil {
return ver, errors.Trace(err)
}
err = colInfo.SetOriginDefaultValue(oldDVal)
if err != nil {
return ver, errors.Trace(err)
}
Expand Down
12 changes: 12 additions & 0 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,18 @@ func (s *testIntegrationSuite5) TestBitDefaultValue(c *C) {
tk.MustQuery("select c from t_bit").Check(testkit.Rows("\x19\xb9"))
tk.MustExec("update t_bit set c = b'11100000000111'")
tk.MustQuery("select c from t_bit").Check(testkit.Rows("\x38\x07"))
tk.MustExec("drop table t_bit")

tk.MustExec("create table t_bit (a int)")
tk.MustExec("insert into t_bit value (1)")
tk.MustExec("alter table t_bit add column b bit(1) default b'0';")
tk.MustExec("alter table t_bit modify column b bit(1) default b'1';")
tk.MustQuery("select b from t_bit").Check(testkit.Rows("\x00"))
tk.MustExec("drop table t_bit")

tk.MustExec("create table t_bit (a bit);")
tk.MustExec("insert into t_bit values (null);")
tk.MustQuery("select count(*) from t_bit where a is null;").Check(testkit.Rows("1"))

tk.MustExec(`create table testalltypes1 (
field_1 bit default 1,
Expand Down
19 changes: 12 additions & 7 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2426,7 +2426,11 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab
return errors.Trace(err)
}

col.OriginDefaultValue, err = generateOriginDefaultValue(col.ToInfo())
originDefVal, err := generateOriginDefaultValue(col.ToInfo())
if err != nil {
return errors.Trace(err)
}
err = col.SetOriginDefaultValue(originDefVal)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -2970,12 +2974,13 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
// a new version TiDB builds the DDL job that doesn't be set the column's offset and state,
// and the old version TiDB is the DDL owner, it doesn't get offset and state from the store. Then it will encounter errors.
// So here we set offset and state to support the rolling upgrade.
Offset: col.Offset,
State: col.State,
OriginDefaultValue: col.OriginDefaultValue,
FieldType: *specNewColumn.Tp,
Name: newColName,
Version: col.Version,
Offset: col.Offset,
State: col.State,
OriginDefaultValue: col.OriginDefaultValue,
OriginDefaultValueBit: col.OriginDefaultValueBit,
FieldType: *specNewColumn.Tp,
Name: newColName,
Version: col.Version,
})

var chs, coll string
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ require (
github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989
github.com/pingcap/kvproto v0.0.0-20200907074027-32a3a0accf7d
github.com/pingcap/log v0.0.0-20200828042413-fce0951f1463
github.com/pingcap/parser v0.0.0-20200921063432-e220cfcfd026
github.com/pingcap/parser v0.0.0-20201022083903-fbe80b0c40bb
github.com/pingcap/sysutil v0.0.0-20200715082929-4c47bcac246a
github.com/pingcap/tidb-tools v4.0.6-0.20200828085514-03575b185007+incompatible
github.com/pingcap/tipb v0.0.0-20200618092958-4fad48b4c8c3
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,8 @@ github.com/pingcap/parser v0.0.0-20200803072748-fdf66528323d/go.mod h1:vQdbJqobJ
github.com/pingcap/parser v0.0.0-20200901062802-475ea5e2e0a7/go.mod h1:vQdbJqobJAgFyiRNNtXahpMoGWwPEuWciVEK5A20NS0=
github.com/pingcap/parser v0.0.0-20200921063432-e220cfcfd026 h1:i+r4P7hb4KpW74nPn+P/hqtsW3fu4U9A4JGAYKWMvtw=
github.com/pingcap/parser v0.0.0-20200921063432-e220cfcfd026/go.mod h1:dMMvhqeowLnAsDWspyalgxXoRUnP09cZ7wAnpt2e/S8=
github.com/pingcap/parser v0.0.0-20201022083903-fbe80b0c40bb h1:yT+Y2yiuLOXdVlERMA7vPQ8Shwk920S9LpuS/cFj6uo=
github.com/pingcap/parser v0.0.0-20201022083903-fbe80b0c40bb/go.mod h1:dMMvhqeowLnAsDWspyalgxXoRUnP09cZ7wAnpt2e/S8=
github.com/pingcap/pd/v4 v4.0.0-rc.1.0.20200422143320-428acd53eba2/go.mod h1:s+utZtXDznOiL24VK0qGmtoHjjXNsscJx3m1n8cC56s=
github.com/pingcap/pd/v4 v4.0.0-rc.2.0.20200520083007-2c251bd8f181/go.mod h1:q4HTx/bA8aKBa4S7L+SQKHvjRPXCRV0tA0yRw0qkZSA=
github.com/pingcap/pd/v4 v4.0.5-0.20200817114353-e465cafe8a91/go.mod h1:m9OEkKoPMQWjrbJ9pqjjeCqzqxraZrPEuWa1OI6Wcek=
Expand Down
2 changes: 1 addition & 1 deletion planner/core/plan_to_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func SetPBColumnsDefaultValue(ctx sessionctx.Context, pbColumns []*tipb.ColumnIn
if c.IsGenerated() && !c.GeneratedStored {
pbColumns[i].DefaultVal = []byte{codec.NilFlag}
}
if c.OriginDefaultValue == nil {
if c.GetOriginDefaultValue() == nil {
continue
}

Expand Down
2 changes: 1 addition & 1 deletion statistics/handle/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (h *Handle) insertColStats2KV(physicalID int64, colInfo *model.ColumnInfo)
return
}
count := req.GetRow(0).GetInt64(0)
value := types.NewDatum(colInfo.OriginDefaultValue)
value := types.NewDatum(colInfo.GetOriginDefaultValue())
value, err = value.ConvertTo(h.mu.ctx.GetSessionVars().StmtCtx, &colInfo.FieldType)
if err != nil {
return
Expand Down
8 changes: 1 addition & 7 deletions table/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,7 @@ func CheckNotNull(cols []*Column, row []types.Datum) error {

// GetColOriginDefaultValue gets default value of the column from original default value.
func GetColOriginDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo) (types.Datum, error) {
// If the column type is BIT, both `OriginDefaultValue` and `DefaultValue` of ColumnInfo are corrupted, because
// after JSON marshaling and unmarshaling against the field with type `interface{}`, the content with actual type `[]byte` is changed.
// We need `DefaultValueBit` to restore OriginDefaultValue before reading it.
if col.Tp == mysql.TypeBit && col.DefaultValueBit != nil && col.OriginDefaultValue != nil {
col.OriginDefaultValue = col.DefaultValueBit
}
return getColDefaultValue(ctx, col, col.OriginDefaultValue)
return getColDefaultValue(ctx, col, col.GetOriginDefaultValue())
}

// GetColDefaultValue gets default value of the column.
Expand Down
2 changes: 1 addition & 1 deletion table/tables/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ func (t *TableCommon) IterRecords(ctx sessionctx.Context, startKey kv.Key, cols
// The defaultVals is used to avoid calculating the default value multiple times.
func GetColDefaultValue(ctx sessionctx.Context, col *table.Column, defaultVals []types.Datum) (
colVal types.Datum, err error) {
if col.OriginDefaultValue == nil && mysql.HasNotNullFlag(col.Flag) {
if col.GetOriginDefaultValue() == nil && mysql.HasNotNullFlag(col.Flag) {
return colVal, errors.New("Miss column")
}
if col.State != model.StatePublic {
Expand Down
2 changes: 1 addition & 1 deletion util/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func CheckRecordAndIndex(sessCtx sessionctx.Context, txn kv.Transaction, t table
for i, val := range vals1 {
col := cols[i]
if val.IsNull() {
if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().OriginDefaultValue == nil {
if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().GetOriginDefaultValue() == nil {
return false, errors.Errorf("Column %v define as not null, but can't find the value where handle is %v", col.Name, h1)
}
// NULL value is regarded as its default value.
Expand Down