Skip to content

Commit

Permalink
ddl: fix corrupted default value for bit type column (#18036) (#20339)
Browse files Browse the repository at this point in the history
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
  • Loading branch information
ti-srebot authored Nov 13, 2020
1 parent 68df519 commit 485833e
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 24 deletions.
8 changes: 6 additions & 2 deletions ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,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)
origDefVal, err := generateOriginDefaultValue(colInfo)
if err != nil {
return ver, errors.Trace(err)
}
err = colInfo.SetOriginDefaultValue(origDefVal)
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 @@ -1004,6 +1004,18 @@ func (s *testIntegrationSuite6) 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 @@ -2185,7 +2185,11 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab
return errors.Trace(err)
}

col.OriginDefaultValue, err = generateOriginDefaultValue(col.ToInfo())
origDefVal, err := generateOriginDefaultValue(col.ToInfo())
if err != nil {
return errors.Trace(err)
}
err = col.SetOriginDefaultValue(origDefVal)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -2669,12 +2673,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,
})

// TODO: Remove it when all table versions are greater than or equal to TableInfoVersion1.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e
github.com/pingcap/kvproto v0.0.0-20200311073257-e53d835099b0
github.com/pingcap/log v0.0.0-20190715063458-479153f07ebd
github.com/pingcap/parser v3.0.17-0.20201028064329-c7f5d9fee686+incompatible
github.com/pingcap/parser v3.0.17-0.20201113024240-d9bf17073679+incompatible
github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2
github.com/pingcap/tidb-tools v3.0.6-0.20191119150227-ff0a3c6e5763+incompatible
github.com/pingcap/tipb v0.0.0-20200426072559-d2c068e96eb3
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ github.com/pingcap/kvproto v0.0.0-20200311073257-e53d835099b0 h1:dXXNHvDwAEN1YNg
github.com/pingcap/kvproto v0.0.0-20200311073257-e53d835099b0/go.mod h1:QMdbTAXCHzzygQzqcG9uVUgU2fKeSN1GmfMiykdSzzY=
github.com/pingcap/log v0.0.0-20190715063458-479153f07ebd h1:hWDol43WY5PGhsh3+8794bFHY1bPrmu6bTalpssCrGg=
github.com/pingcap/log v0.0.0-20190715063458-479153f07ebd/go.mod h1:WpHUKhNZ18v116SvGrmjkA9CBhYmuUTKL+p8JC9ANEw=
github.com/pingcap/parser v3.0.17-0.20201028064329-c7f5d9fee686+incompatible h1:OGSmvyHh8eQOogkqUGEAawcM1NAr8wp1rEaEBSEDPYo=
github.com/pingcap/parser v3.0.17-0.20201028064329-c7f5d9fee686+incompatible/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/parser v3.0.17-0.20201113024240-d9bf17073679+incompatible h1:5HEah4ila+lq0/7NqVjywd51aYaU3IDEJ6v3aqM2Bhw=
github.com/pingcap/parser v3.0.17-0.20201113024240-d9bf17073679+incompatible/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2 h1:NL23b8tsg6M1QpSQedK14/Jx++QeyKL2rGiBvXAQVfA=
github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2/go.mod h1:b4gaAPSxaVVtaB+EHamV4Nsv8JmTdjlw0cTKmp4+dRQ=
github.com/pingcap/tidb-tools v3.0.6-0.20191119150227-ff0a3c6e5763+incompatible h1:I8HirWsu1MZp6t9G/g8yKCEjJJxtHooKakEgccvdJ4M=
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 @@ -139,7 +139,7 @@ func (p *PhysicalIndexScan) ToPB(ctx sessionctx.Context) (*tipb.Executor, error)
// SetPBColumnsDefaultValue sets the default values of tipb.ColumnInfos.
func SetPBColumnsDefaultValue(ctx sessionctx.Context, pbColumns []*tipb.ColumnInfo, columns []*model.ColumnInfo) error {
for i, c := range columns {
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 @@ -149,7 +149,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 @@ -363,13 +363,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 @@ -912,7 +912,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
4 changes: 2 additions & 2 deletions util/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,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 Expand Up @@ -672,7 +672,7 @@ func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h
}
ri, ok := rowMap[col.ID]
if !ok {
if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().OriginDefaultValue == nil {
if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().GetOriginDefaultValue() == nil {
return nil, errors.Errorf("Column %v define as not null, but can't find the value where handle is %v", col.Name, h)
}
// NULL value is regarded as its default value.
Expand Down

0 comments on commit 485833e

Please sign in to comment.