From 01b4f8d3199458fc0cb946fe1b2cd83a793c3234 Mon Sep 17 00:00:00 2001 From: xia Date: Mon, 19 Jun 2017 23:00:48 +0800 Subject: [PATCH 1/3] *: fix add index after add column with default value --- ddl/ddl_db_test.go | 12 ++++++++ ddl/index.go | 13 ++++++++- table/tables/tables.go | 65 ++++++++++++++++++++++++------------------ 3 files changed, 61 insertions(+), 29 deletions(-) diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index 428791186ecda..ef7929f188f44 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -157,6 +157,18 @@ func (s *testDBSuite) TestMySQLErrorCode(c *C) { s.testErrorCode(c, sql, tmysql.ErrWrongTableName) } +func (s *testDBSuite) TestAddIndexAfterAddColumn(c *C) { + defer testleak.AfterTest(c)() + s.tk = testkit.NewTestKit(c, s.store) + s.tk.MustExec("use " + s.schemaName) + + s.tk.MustExec("create table test_add_index_after_add_col(a int, b int not null default '0')") + s.tk.MustExec("insert into test_add_index_after_add_col values(1, 2),(2,2)") + s.tk.MustExec("alter table test_add_index_after_add_col add column c int not null default '0'") + sql := "alter table test_add_index_after_add_col add unique index cc(c) " + s.testErrorCode(c, sql, tmysql.ErrDupEntry) +} + func (s *testDBSuite) TestIndex(c *C) { defer testleak.AfterTest(c)() s.tk = testkit.NewTestKit(c, s.store) diff --git a/ddl/index.go b/ddl/index.go index 380a642113851..f03d1497b947b 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -421,7 +421,9 @@ func (d *ddl) fetchRowColVals(txn kv.Transaction, t table.Table, taskOpInfo *ind } cols := t.Cols() + ctx := d.newContext() idxInfo := taskOpInfo.tblIndex.Meta() + defaultVals := make([]types.Datum, len(cols)) for i, idxRecord := range idxRecords { rowMap, err := tablecodec.DecodeRow(rawRecords[i], taskOpInfo.colMap, time.UTC) if err != nil { @@ -431,7 +433,16 @@ func (d *ddl) fetchRowColVals(txn kv.Transaction, t table.Table, taskOpInfo *ind idxVal := make([]types.Datum, 0, len(idxInfo.Columns)) for _, v := range idxInfo.Columns { col := cols[v.Offset] - idxVal = append(idxVal, rowMap[col.ID]) + idxColumnVal := rowMap[col.ID] + if _, ok := rowMap[col.ID]; ok { + idxVal = append(idxVal, idxColumnVal) + continue + } + idxColumnVal, ret.err = tables.GetColDefaultValue(ctx, col, defaultVals) + if ret.err != nil { + ret.err = errors.Trace(err) + } + idxVal = append(idxVal, idxColumnVal) } idxRecord.vals = idxVal } diff --git a/table/tables/tables.go b/table/tables/tables.go index 715e6800416a6..262db0e38dafb 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -459,10 +459,11 @@ func (t *Table) RowWithCols(ctx context.Context, h int64, cols []*table.Column) } colTps[col.ID] = &col.FieldType } - row, err := tablecodec.DecodeRow(value, colTps, ctx.GetSessionVars().GetTimeZone()) + rowMap, err := tablecodec.DecodeRow(value, colTps, ctx.GetSessionVars().GetTimeZone()) if err != nil { return nil, errors.Trace(err) } + defaultVals := make([]types.Datum, len(cols)) for i, col := range cols { if col == nil { continue @@ -470,23 +471,18 @@ func (t *Table) RowWithCols(ctx context.Context, h int64, cols []*table.Column) if col.IsPKHandleColumn(t.meta) { continue } - ri, ok := row[col.ID] + ri, ok := rowMap[col.ID] if ok { v[i] = ri continue } - - if col.OriginDefaultValue != nil && col.State == model.StatePublic { - ri, err = table.GetColOriginDefaultValue(ctx, col.ToInfo()) - if err != nil { - return nil, errors.Trace(err) - } - v[i] = ri - continue - } if mysql.HasNotNullFlag(col.Flag) { return nil, errors.New("Miss column") } + v[i], err = GetColDefaultValue(ctx, col, defaultVals) + if err != nil { + return nil, errors.Trace(err) + } } return v, nil } @@ -633,29 +629,21 @@ func (t *Table) IterRecords(ctx context.Context, startKey kv.Key, cols []*table. } data := make([]types.Datum, len(cols)) for _, col := range cols { - if col.IsPKHandleColumn(t.Meta()) { - data[col.Offset] = types.NewIntDatum(handle) + if col.IsPKHandleColumn(t.meta) { + if mysql.HasUnsignedFlag(col.Flag) { + data[col.Offset].SetUint64(uint64(handle)) + } else { + data[col.Offset].SetInt64(handle) + } continue } if _, ok := rowMap[col.ID]; ok { data[col.Offset] = rowMap[col.ID] continue } - if col.OriginDefaultValue == nil && mysql.HasNotNullFlag(col.Flag) { - return errors.New("Miss column") - } - if col.State != model.StatePublic { - continue - } - if defaultVals[col.Offset].IsNull() { - d, err := table.GetColOriginDefaultValue(ctx, col.ToInfo()) - if err != nil { - return errors.Trace(err) - } - data[col.Offset] = d - defaultVals[col.Offset] = d - } else { - data[col.Offset] = defaultVals[col.Offset] + data[col.Offset], err = GetColDefaultValue(ctx, col, defaultVals) + if err != nil { + return errors.Trace(err) } } more, err := fn(handle, data, cols) @@ -673,6 +661,27 @@ func (t *Table) IterRecords(ctx context.Context, startKey kv.Key, cols []*table. return nil } +func GetColDefaultValue(ctx context.Context, col *table.Column, defaultVals []types.Datum) ( + colVal types.Datum, err error) { + if col.OriginDefaultValue == nil && mysql.HasNotNullFlag(col.Flag) { + return colVal, errors.New("Miss column") + } + if col.State != model.StatePublic { + return colVal, nil + } + if defaultVals[col.Offset].IsNull() { + colVal, err = table.GetColOriginDefaultValue(ctx, col.ToInfo()) + if err != nil { + return colVal, errors.Trace(err) + } + defaultVals[col.Offset] = colVal + } else { + colVal = defaultVals[col.Offset] + } + + return colVal, nil +} + // AllocAutoID implements table.Table AllocAutoID interface. func (t *Table) AllocAutoID() (int64, error) { return t.alloc.Alloc(t.ID) From 5dee15afa8ff8cd479a22bfcb0b0f56165312208 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 20 Jun 2017 11:00:58 +0800 Subject: [PATCH 2/3] *: address comments --- ddl/index.go | 1 + table/tables/tables.go | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ddl/index.go b/ddl/index.go index f03d1497b947b..280080e416baa 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -441,6 +441,7 @@ func (d *ddl) fetchRowColVals(txn kv.Transaction, t table.Table, taskOpInfo *ind idxColumnVal, ret.err = tables.GetColDefaultValue(ctx, col, defaultVals) if ret.err != nil { ret.err = errors.Trace(err) + return nil, ret } idxVal = append(idxVal, idxColumnVal) } diff --git a/table/tables/tables.go b/table/tables/tables.go index 262db0e38dafb..cea22320b3402 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -476,9 +476,6 @@ func (t *Table) RowWithCols(ctx context.Context, h int64, cols []*table.Column) v[i] = ri continue } - if mysql.HasNotNullFlag(col.Flag) { - return nil, errors.New("Miss column") - } v[i], err = GetColDefaultValue(ctx, col, defaultVals) if err != nil { return nil, errors.Trace(err) @@ -661,6 +658,8 @@ func (t *Table) IterRecords(ctx context.Context, startKey kv.Key, cols []*table. return nil } +// GetColDefaultValue gets a column default value. +// The defaultVals is used to avoid calculating the default value multiple times. func GetColDefaultValue(ctx context.Context, col *table.Column, defaultVals []types.Datum) ( colVal types.Datum, err error) { if col.OriginDefaultValue == nil && mysql.HasNotNullFlag(col.Flag) { From c4a3e38a1daecb9dc0d3dedf1cead261ad46f662 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 20 Jun 2017 11:48:18 +0800 Subject: [PATCH 3/3] ddl: tiny update --- ddl/index.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/index.go b/ddl/index.go index 280080e416baa..7069e5a2e8f27 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -440,7 +440,7 @@ func (d *ddl) fetchRowColVals(txn kv.Transaction, t table.Table, taskOpInfo *ind } idxColumnVal, ret.err = tables.GetColDefaultValue(ctx, col, defaultVals) if ret.err != nil { - ret.err = errors.Trace(err) + ret.err = errors.Trace(ret.err) return nil, ret } idxVal = append(idxVal, idxColumnVal)