ddl: fix panic when add index of generated column. (#8620)

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")
+	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)")
+	tk.MustExec("drop table t")
+}
+
 func (s *testIntegrationSuite) TestCaseInsensitiveCharsetAndCollate(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 

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

executor/executor.go

@@ -410,7 +410,7 @@ type CheckTableExec struct {
 	done   bool
 	is     infoschema.InfoSchema
 
-	genExprs map[string]expression.Expression
+	genExprs map[model.TableColumnID]expression.Expression
 }
 
 // Open implements the Executor Open interface.

model/model.go

@@ -15,7 +15,6 @@ package model
 
 import (
 	"encoding/json"
-	"fmt"
 	"strings"
 	"time"
 
@@ -556,7 +555,8 @@ func collationToProto(c string) int32 {
 	return int32(mysql.DefaultCollationID)
 }
 
-// GetTableColumnID gets a ID of a column with table ID
-func GetTableColumnID(tableInfo *TableInfo, col *ColumnInfo) string {
-	return fmt.Sprintf("%d_%d", tableInfo.ID, col.ID)
+// TableColumnID is composed by table ID and column ID.
+type TableColumnID struct {
+	TableID  int64
+	ColumnID int64
 }

planner/core/common_plans.go

@@ -73,7 +73,7 @@ type CheckTable struct {
 
 	Tables []*ast.TableName
 
-	GenExprs map[string]expression.Expression
+	GenExprs map[model.TableColumnID]expression.Expression
 }
 
 // RecoverIndex is used for backfilling corrupted index data.

planner/core/planbuilder.go

@@ -32,7 +32,6 @@ import (
 	"github.com/pingcap/tidb/table"
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/types/parser_driver"
-	"github.com/pingcap/tidb/util/admin"
 	"github.com/pingcap/tidb/util/ranger"
 )
 
@@ -530,7 +529,7 @@ func (b *planBuilder) buildAdmin(as *ast.AdminStmt) (Plan, error) {
 
 func (b *planBuilder) buildAdminCheckTable(as *ast.AdminStmt) (*CheckTable, error) {
 	p := &CheckTable{Tables: as.Tables}
-	p.GenExprs = make(map[string]expression.Expression)
+	p.GenExprs = make(map[model.TableColumnID]expression.Expression, len(p.Tables))
 
 	mockTablePlan := LogicalTableDual{}.init(b.ctx)
 	for _, tbl := range p.Tables {
@@ -562,8 +561,7 @@ func (b *planBuilder) buildAdminCheckTable(as *ast.AdminStmt) (*CheckTable, erro
 				return nil, errors.Trace(err)
 			}
 			expr = expression.BuildCastFunction(b.ctx, expr, colExpr.GetType())
-			genColumnName := admin.GetTableColumnID(tableInfo, column.ColumnInfo)
-			p.GenExprs[genColumnName] = expr
+			p.GenExprs[model.TableColumnID{TableID: tableInfo.ID, ColumnID: column.ColumnInfo.ID}] = expr
 		}
 	}
 	return p, nil

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 (

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) {

util/admin/admin.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"io"
 	"sort"
+	"time"
 
 	"github.com/pingcap/errors"
 	"github.com/pingcap/parser/model"
@@ -32,7 +33,7 @@ import (
 	"github.com/pingcap/tidb/tablecodec"
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/util"
-	"github.com/pingcap/tidb/util/chunk"
+	"github.com/pingcap/tidb/util/rowDecoder"
 	"github.com/pingcap/tidb/util/sqlexec"
 	log "github.com/sirupsen/logrus"
 )
@@ -294,7 +295,7 @@ func ScanIndexData(sc *stmtctx.StatementContext, txn kv.Transaction, kvIndex tab
 // It returns nil if the data from the index is equal to the data from the table columns,
 // otherwise it returns an error with a different set of records.
 // genExprs is use to calculate the virtual generate column.
-func CompareIndexData(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[string]expression.Expression) error {
+func CompareIndexData(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[model.TableColumnID]expression.Expression) error {
 	err := checkIndexAndRecord(sessCtx, txn, t, idx, genExprs)
 	if err != nil {
 		return errors.Trace(err)
@@ -336,7 +337,7 @@ func adjustDatumKind(vals1, vals2 []types.Datum) {
 	}
 }
 
-func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[string]expression.Expression) error {
+func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[model.TableColumnID]expression.Expression) error {
 	it, err := idx.SeekFirst(txn)
 	if err != nil {
 		return errors.Trace(err)
@@ -352,6 +353,7 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table
 	if err != nil {
 		return errors.Trace(err)
 	}
+	rowDecoder := makeRowDecoder(t, cols, genExprs)
 	sc := sessCtx.GetSessionVars().StmtCtx
 	for {
 		vals1, h, err := it.Next()
@@ -365,7 +367,7 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table
 		if err != nil {
 			return errors.Trace(err)
 		}
-		vals2, err := rowWithCols(sessCtx, txn, t, h, cols, genExprs)
+		vals2, err := rowWithCols(sessCtx, txn, t, h, cols, rowDecoder)
 		vals2 = tables.TruncateIndexValuesIfNeeded(t.Meta(), idx.Meta(), vals2)
 		if kv.ErrNotExist.Equal(err) {
 			record := &RecordData{Handle: h, Values: vals1}
@@ -399,7 +401,7 @@ func compareDatumSlice(sc *stmtctx.StatementContext, val1s, val2s []types.Datum)
 }
 
 // CheckRecordAndIndex is exported for testing.
-func CheckRecordAndIndex(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[string]expression.Expression) error {
+func CheckRecordAndIndex(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[model.TableColumnID]expression.Expression) error {
 	sc := sessCtx.GetSessionVars().StmtCtx
 	cols := make([]*table.Column, len(idx.Meta().Columns))
 	for i, col := range idx.Meta().Columns {
@@ -551,16 +553,38 @@ 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 {
+	cols := t.Cols()
+	tblInfo := t.Meta()
+	decodeColsMap := make(map[int64]decoder.Column, len(decodeCol))
+	for _, v := range decodeCol {
+		col := cols[v.Offset]
+		tpExpr := decoder.Column{
+			Info: col.ToInfo(),
+		}
+		if col.IsGenerated() && !col.GeneratedStored {
+			for _, c := range cols {
+				if _, ok := col.Dependences[c.Name.L]; ok {
+					decodeColsMap[c.ID] = decoder.Column{
+						Info: c.ToInfo(),
+					}
+				}
+			}
+			tpExpr.GenExpr = genExpr[model.TableColumnID{TableID: tblInfo.ID, ColumnID: col.ID}]
+		}
+		decodeColsMap[col.ID] = tpExpr
+	}
+	return decoder.NewRowDecoder(cols, decodeColsMap)
+}
+
 // genExprs use to calculate generated column value.
-func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h int64, cols []*table.Column, genExprs map[string]expression.Expression) ([]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)
-	genColFlag := false
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 	v := make([]types.Datum, len(cols))
-	colTps := make(map[int64]*types.FieldType, len(cols))
 	for i, col := range cols {
 		if col == nil {
 			continue
@@ -576,34 +600,13 @@ func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h
 			}
 			continue
 		}
-		// If have virtual generate column , decode all columns.
-		if col.IsGenerated() && col.GeneratedStored == false {
-			genColFlag = true
-		}
-		colTps[col.ID] = &col.FieldType
-	}
-	// if have virtual generate column, decode all columns
-	if genColFlag {
-		for _, c := range t.Cols() {
-			if c.State != model.StatePublic {
-				continue
-			}
-			colTps[c.ID] = &c.FieldType
-		}
 	}
 
-	rowMap, err := tablecodec.DecodeRow(value, colTps, sessCtx.GetSessionVars().Location())
+	rowMap, err := rowDecoder.DecodeAndEvalRowWithMap(sessCtx, value, sessCtx.GetSessionVars().Location(), time.UTC, nil)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
-	if genColFlag && genExprs != nil {
-		err = fillGenColData(sessCtx, rowMap, t, cols, genExprs)
-		if err != nil {
-			return v, errors.Trace(err)
-		}
-	}
-
 	for i, col := range cols {
 		if col == nil {
 			continue
@@ -635,7 +638,7 @@ func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h
 
 // genExprs use to calculate generated column value.
 func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Table, startKey kv.Key, cols []*table.Column,
-	fn table.RecordIterFunc, genExprs map[string]expression.Expression) error {
+	fn table.RecordIterFunc, genExprs map[model.TableColumnID]expression.Expression) error {
 	prefix := t.RecordPrefix()
 	keyUpperBound := prefix.PrefixNext()
 
@@ -650,22 +653,7 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab
 	}
 
 	log.Debugf("startKey:%q, key:%q, value:%q", startKey, it.Key(), it.Value())
-
-	genColFlag := false
-	colMap := make(map[int64]*types.FieldType, len(cols))
-	for _, col := range cols {
-		if col.IsGenerated() && col.GeneratedStored == false {
-			genColFlag = true
-			break
-		}
-		colMap[col.ID] = &col.FieldType
-	}
-	if genColFlag {
-		for _, col := range t.Cols() {
-			colMap[col.ID] = &col.FieldType
-		}
-	}
-
+	rowDecoder := makeRowDecoder(t, cols, genExprs)
 	for it.Valid() && it.Key().HasPrefix(prefix) {
 		// first kv pair is row lock information.
 		// TODO: check valid lock
@@ -675,18 +663,10 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab
 			return errors.Trace(err)
 		}
 
-		rowMap, err := tablecodec.DecodeRow(it.Value(), colMap, sessCtx.GetSessionVars().Location())
+		rowMap, err := rowDecoder.DecodeAndEvalRowWithMap(sessCtx, it.Value(), sessCtx.GetSessionVars().Location(), time.UTC, nil)
 		if err != nil {
 			return errors.Trace(err)
 		}
-
-		if genColFlag && genExprs != nil {
-			err = fillGenColData(sessCtx, rowMap, t, cols, genExprs)
-			if err != nil {
-				return errors.Trace(err)
-			}
-		}
-
 		data := make([]types.Datum, 0, len(cols))
 		for _, col := range cols {
 			if col.IsPKHandleColumn(t.Meta()) {
@@ -714,44 +694,6 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab
 	return nil
 }
 
-// genExprs use to calculate generated column value.
-func fillGenColData(sessCtx sessionctx.Context, rowMap map[int64]types.Datum, t table.Table, cols []*table.Column, genExprs map[string]expression.Expression) error {
-	tableInfo := t.Meta()
-	row := make([]types.Datum, len(t.Cols()))
-	for _, col := range t.Cols() {
-		ri, ok := rowMap[col.ID]
-		if ok {
-			row[col.Offset] = ri
-		}
-	}
-
-	var err error
-	for _, col := range cols {
-		if !col.IsGenerated() || col.GeneratedStored == true {
-			continue
-		}
-		genColumnName := GetTableColumnID(tableInfo, col.ColumnInfo)
-		if expr, ok := genExprs[genColumnName]; ok {
-			var val types.Datum
-			val, err = expr.Eval(chunk.MutRowFromDatums(row).ToRow())
-			if err != nil {
-				return errors.Trace(err)
-			}
-			val, err = table.CastValue(sessCtx, val, col.ToInfo())
-			if err != nil {
-				return errors.Trace(err)
-			}
-			rowMap[col.ID] = val
-		}
-	}
-	return nil
-}
-
-// GetTableColumnID gets a ID of a column with table ID	// TableColumnID is composed by table ID and column ID.
-func GetTableColumnID(tableInfo *model.TableInfo, col *model.ColumnInfo) string {
-	return fmt.Sprintf("%d_%d", tableInfo.ID, col.ID)
-}
-
 // admin error codes.
 const (
 	codeDataNotEqual       terror.ErrCode = 1