From 0ec6cea9c57f3a6097dc9ef755864a0cd00a66be Mon Sep 17 00:00:00 2001 From: winkyao Date: Fri, 7 Dec 2018 18:46:00 +0800 Subject: [PATCH 1/3] ddl: fix panic when add index of generated column. --- ddl/db_integration_test.go | 18 ++++++++++++++++++ ddl/index.go | 16 +++++++++++++--- util/admin/admin.go | 4 ++-- util/rowDecoder/decoder.go | 10 +++++----- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index effcb92e2d832..62ffc866f27e8 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -152,6 +152,24 @@ func (s *testIntegrationSuite) TestEndIncluded(c *C) { tk.MustExec("admin check table t") } +func (s *testIntegrationSuite) TestNullGeneratedColumn(c *C) { + tk := testkit.NewTestKit(c, s.store) + + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + defer tk.MustExec("drop table if exists t") + tk.MustExec("CREATE TABLE `t` (" + + "`a` int(11) DEFAULT NULL," + + "`b` int(11) DEFAULT NULL," + + "`c` int(11) GENERATED ALWAYS AS (`a` + `b`) VIRTUAL DEFAULT NULL," + + "`h` varchar(10) DEFAULT NULL," + + "`m` int(11) DEFAULT NULL" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin") + + tk.MustExec("insert into t values()") + tk.MustExec("alter table t add index idx_c(c)") +} + func newStoreWithBootstrap() (kv.Storage, *domain.Domain, error) { store, err := mockstore.NewMockTikvStore() if err != nil { diff --git a/ddl/index.go b/ddl/index.go index a97a1c32cb337..b04c0199fedd4 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -456,7 +456,7 @@ type addIndexWorker struct { defaultVals []types.Datum idxRecords []*indexRecord rowMap map[int64]types.Datum - rowDecoder decoder.RowDecoder + rowDecoder *decoder.RowDecoder idxKeyBufs [][]byte batchCheckKeys []kv.Key distinctCheckFlags []bool @@ -542,8 +542,8 @@ func (w *addIndexWorker) getIndexRecord(handle int64, recordKey []byte, rawRecor } continue } - idxColumnVal := w.rowMap[col.ID] - if _, ok := w.rowMap[col.ID]; ok { + idxColumnVal, ok := w.rowMap[col.ID] + if ok { idxVal[j] = idxColumnVal // Make sure there is no dirty data. delete(w.rowMap, col.ID) @@ -566,10 +566,19 @@ func (w *addIndexWorker) getIndexRecord(handle int64, recordKey []byte, rawRecor } idxVal[j] = idxColumnVal } + // If there are generated column, rowDecoder will use column value that not in idxInfo.Columns to calculate + // the generated value, so we need to clear up the reusing map. + w.cleanRowMap() idxRecord := &indexRecord{handle: handle, key: recordKey, vals: idxVal} return idxRecord, nil } +func (w *addIndexWorker) cleanRowMap() { + for id := range w.rowMap { + delete(w.rowMap, id) + } +} + // getNextHandle gets next handle of entry that we are going to process. func (w *addIndexWorker) getNextHandle(taskRange reorgIndexTask, taskDone bool) (nextHandle int64) { if !taskDone { @@ -788,6 +797,7 @@ func (w *addIndexWorker) handleBackfillTask(d *ddlCtx, task *reorgIndexTask) *ad // we should check whether this ddl job is still runnable. err = w.ddlWorker.isReorgRunnable(d) } + if err != nil { result.err = err return result diff --git a/util/admin/admin.go b/util/admin/admin.go index 1c4fa7107a0fd..ae3da9ad91c2f 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -553,7 +553,7 @@ func CompareTableRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table. return nil } -func makeRowDecoder(t table.Table, decodeCol []*table.Column, genExpr map[model.TableColumnID]expression.Expression) decoder.RowDecoder { +func makeRowDecoder(t table.Table, decodeCol []*table.Column, genExpr map[model.TableColumnID]expression.Expression) *decoder.RowDecoder { cols := t.Cols() tblInfo := t.Meta() decodeColsMap := make(map[int64]decoder.Column, len(decodeCol)) @@ -578,7 +578,7 @@ func makeRowDecoder(t table.Table, decodeCol []*table.Column, genExpr map[model. } // genExprs use to calculate generated column value. -func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h int64, cols []*table.Column, rowDecoder decoder.RowDecoder) ([]types.Datum, error) { +func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h int64, cols []*table.Column, rowDecoder *decoder.RowDecoder) ([]types.Datum, error) { key := t.RecordKey(h) value, err := txn.Get(key) if err != nil { diff --git a/util/rowDecoder/decoder.go b/util/rowDecoder/decoder.go index bd4d778b91b81..8d1f68c2b00d8 100644 --- a/util/rowDecoder/decoder.go +++ b/util/rowDecoder/decoder.go @@ -42,7 +42,7 @@ type RowDecoder struct { } // NewRowDecoder returns a new RowDecoder. -func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) RowDecoder { +func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) *RowDecoder { colFieldMap := make(map[int64]*types.FieldType, len(decodeColMap)) haveGenCol := false for id, col := range decodeColMap { @@ -52,7 +52,7 @@ func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) RowDecod } } if !haveGenCol { - return RowDecoder{ + return &RowDecoder{ colTypes: colFieldMap, } } @@ -61,7 +61,7 @@ func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) RowDecod for _, col := range cols { tps[col.Offset] = &col.FieldType } - return RowDecoder{ + return &RowDecoder{ mutRow: chunk.MutRowFromTypes(tps), columns: decodeColMap, colTypes: colFieldMap, @@ -70,8 +70,8 @@ func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) RowDecod } // DecodeAndEvalRowWithMap decodes a byte slice into datums and evaluates the generated column value. -func (rd RowDecoder) DecodeAndEvalRowWithMap(ctx sessionctx.Context, b []byte, decodeLoc, sysLoc *time.Location, row map[int64]types.Datum) (map[int64]types.Datum, error) { - row, err := tablecodec.DecodeRowWithMap(b, rd.colTypes, decodeLoc, row) +func (rd *RowDecoder) DecodeAndEvalRowWithMap(ctx sessionctx.Context, b []byte, decodeLoc, sysLoc *time.Location, row map[int64]types.Datum) (map[int64]types.Datum, error) { + _, err := tablecodec.DecodeRowWithMap(b, rd.colTypes, decodeLoc, row) if err != nil { return nil, errors.Trace(err) } From ea2401908e8bd02d0eb758cf48d8fd415ac42448 Mon Sep 17 00:00:00 2001 From: winkyao Date: Fri, 7 Dec 2018 19:04:18 +0800 Subject: [PATCH 2/3] fix ci --- ddl/db_integration_test.go | 2 +- tablecodec/tablecodec.go | 4 ++-- util/rowDecoder/decoder.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 6e4cdd648cc4a..185b3c774067e 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -157,7 +157,6 @@ func (s *testIntegrationSuite) TestNullGeneratedColumn(c *C) { tk.MustExec("use test") tk.MustExec("drop table if exists t") - defer tk.MustExec("drop table if exists t") tk.MustExec("CREATE TABLE `t` (" + "`a` int(11) DEFAULT NULL," + "`b` int(11) DEFAULT NULL," + @@ -168,6 +167,7 @@ func (s *testIntegrationSuite) TestNullGeneratedColumn(c *C) { tk.MustExec("insert into t values()") tk.MustExec("alter table t add index idx_c(c)") + tk.MustExec("drop table t") } func (s *testIntegrationSuite) TestCaseInsensitiveCharsetAndCollate(c *C) { diff --git a/tablecodec/tablecodec.go b/tablecodec/tablecodec.go index de9b31c31210e..7eeea8e57cb59 100644 --- a/tablecodec/tablecodec.go +++ b/tablecodec/tablecodec.go @@ -305,10 +305,10 @@ func DecodeRowWithMap(b []byte, cols map[int64]*types.FieldType, loc *time.Locat row = make(map[int64]types.Datum, len(cols)) } if b == nil { - return nil, nil + return row, nil } if len(b) == 1 && b[0] == codec.NilFlag { - return nil, nil + return row, nil } cnt := 0 var ( diff --git a/util/rowDecoder/decoder.go b/util/rowDecoder/decoder.go index 8d1f68c2b00d8..4ea12d1394299 100644 --- a/util/rowDecoder/decoder.go +++ b/util/rowDecoder/decoder.go @@ -71,7 +71,7 @@ func NewRowDecoder(cols []*table.Column, decodeColMap map[int64]Column) *RowDeco // DecodeAndEvalRowWithMap decodes a byte slice into datums and evaluates the generated column value. func (rd *RowDecoder) DecodeAndEvalRowWithMap(ctx sessionctx.Context, b []byte, decodeLoc, sysLoc *time.Location, row map[int64]types.Datum) (map[int64]types.Datum, error) { - _, err := tablecodec.DecodeRowWithMap(b, rd.colTypes, decodeLoc, row) + row, err := tablecodec.DecodeRowWithMap(b, rd.colTypes, decodeLoc, row) if err != nil { return nil, errors.Trace(err) } From 23880f7f0291739ce4b6fd46c15057a907613aa5 Mon Sep 17 00:00:00 2001 From: winkyao Date: Fri, 7 Dec 2018 19:08:20 +0800 Subject: [PATCH 3/3] fix ci --- tablecodec/tablecodec_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tablecodec/tablecodec_test.go b/tablecodec/tablecodec_test.go index a68f26d6ef873..9467d926128d9 100644 --- a/tablecodec/tablecodec_test.go +++ b/tablecodec/tablecodec_test.go @@ -137,7 +137,7 @@ func (s *testTableCodecSuite) TestRowCodec(c *C) { r, err = DecodeRow(bs, colMap, time.UTC) c.Assert(err, IsNil) - c.Assert(r, IsNil) + c.Assert(len(r), Equals, 0) } func (s *testTableCodecSuite) TestTimeCodec(c *C) {