diff --git a/pkg/ddl/util/util.go b/pkg/ddl/util/util.go index 397355bf0b..6f088e0646 100644 --- a/pkg/ddl/util/util.go +++ b/pkg/ddl/util/util.go @@ -423,5 +423,5 @@ func GenKeyExistsErr(key, value []byte, idxInfo *model.IndexInfo, tblInfo *model zap.String("key", hex.EncodeToString(key)), zap.String("value", hex.EncodeToString(value)), zap.Error(err)) return errors.Trace(kv.ErrKeyExists.FastGenByArgs(key, indexName)) } - return kv.ErrKeyExists.FastGenByArgs(strings.Join(valueStr, "-"), indexName) + return kv.GenKeyExistsErr(valueStr, indexName) } diff --git a/pkg/executor/batch_checker.go b/pkg/executor/batch_checker.go index d7f83edeb4..7a7f8204f9 100644 --- a/pkg/executor/batch_checker.go +++ b/pkg/executor/batch_checker.go @@ -17,7 +17,6 @@ package executor import ( "context" "fmt" - "strings" "github.com/pingcap/errors" "github.com/pingcap/tidb/pkg/errno" @@ -33,7 +32,8 @@ import ( "github.com/pingcap/tidb/pkg/types" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/pingcap/tidb/pkg/util/codec" - "github.com/pingcap/tidb/pkg/util/stringutil" + "github.com/pingcap/tidb/pkg/util/logutil" + "go.uber.org/zap" ) type keyValueWithDupInfo struct { @@ -125,26 +125,37 @@ func getKeysNeedCheckOneRow(ctx sessionctx.Context, t table.Table, row []types.D } var handleKey *keyValueWithDupInfo if handle != nil { - fn := func() string { - var str string - var err error - if t.Meta().IsCommonHandle { - data := make([]types.Datum, len(handleCols)) - for i, col := range handleCols { - data[i] = row[col.Offset] - } - str, err = formatDataForDupError(data) - } else { - str, err = row[handleCols[0].Offset].ToString() - } - if err != nil { - return kv.GetDuplicateErrorHandleString(handle) - } - return str - } handleKey = &keyValueWithDupInfo{ newKey: tablecodec.EncodeRecordKey(t.RecordPrefix(), handle), - dupErr: kv.ErrKeyExists.FastGenByArgs(stringutil.MemoizeStr(fn), t.Meta().Name.String()+".PRIMARY"), + } + + var keyCols []string + var err error + if t.Meta().IsCommonHandle { + data := make([]types.Datum, len(handleCols)) + for i, col := range handleCols { + data[i] = row[col.Offset] + } + keyCols, err = dataToStrings(data) + } else { + var s string + s, err = row[handleCols[0].Offset].ToString() + keyCols = []string{s} + } + if err != nil { + var handleData []types.Datum + handleData, err = handle.Data() + if err == nil { + keyCols, err = dataToStrings(handleData) + } + } + + if err == nil { + handleKey.dupErr = kv.GenKeyExistsErr(keyCols, t.Meta().Name.String()+".PRIMARY") + } else { + logutil.BgLogger().Warn("get key string failed", + zap.Error(err), zap.Stringer("handle", handle)) + handleKey.dupErr = kv.ErrKeyExists } } @@ -207,13 +218,13 @@ func getKeysNeedCheckOneRow(ctx sessionctx.Context, t table.Table, row []types.D if v.Meta().State != model.StatePublic && v.Meta().BackfillState != model.BackfillStateInapplicable { _, key, _ = tables.GenTempIdxKeyByState(v.Meta(), key) } - colValStr, err1 := formatDataForDupError(colVals) + colStrVals, err1 := dataToStrings(colVals) if err1 != nil { return nil, err1 } uniqueKeys = append(uniqueKeys, &keyValueWithDupInfo{ newKey: key, - dupErr: kv.ErrKeyExists.FastGenByArgs(colValStr, fmt.Sprintf("%s.%s", v.TableMeta().Name.String(), v.Meta().Name.String())), + dupErr: kv.GenKeyExistsErr(colStrVals, fmt.Sprintf("%s.%s", v.TableMeta().Name.String(), v.Meta().Name.String())), }) } } @@ -248,16 +259,16 @@ func buildHandleFromDatumRow(sctx *stmtctx.StatementContext, row []types.Datum, return handle, nil } -func formatDataForDupError(data []types.Datum) (string, error) { +func dataToStrings(data []types.Datum) ([]string, error) { strs := make([]string, 0, len(data)) for _, datum := range data { str, err := datum.ToString() if err != nil { - return "", errors.Trace(err) + return nil, errors.Trace(err) } strs = append(strs, str) } - return strings.Join(strs, "-"), nil + return strs, nil } // getOldRow gets the table record row from storage for batch check. diff --git a/pkg/executor/insert.go b/pkg/executor/insert.go index 86895458e9..0878dabf8b 100644 --- a/pkg/executor/insert.go +++ b/pkg/executor/insert.go @@ -56,7 +56,7 @@ type InsertExec struct { func (e *InsertExec) exec(ctx context.Context, rows [][]types.Datum) error { defer trace.StartRegion(ctx, "InsertExec").End() - logutil.Eventf(ctx, "insert %d rows into table `%s`", len(rows), stringutil.MemoizeStr(func() string { + logutil.Eventf(ctx, "insert %d rows into table `%s`", len(rows), stringutil.StringerFunc(func() string { var tblName string if meta := e.Table.Meta(); meta != nil { tblName = meta.Name.L diff --git a/pkg/kv/error.go b/pkg/kv/error.go index 8aa3f4b900..c8e1bc77a4 100644 --- a/pkg/kv/error.go +++ b/pkg/kv/error.go @@ -47,7 +47,8 @@ var ( ErrTxnTooLarge = dbterror.ClassKV.NewStd(mysql.ErrTxnTooLarge) // ErrEntryTooLarge is the error when a key value entry is too large. ErrEntryTooLarge = dbterror.ClassKV.NewStd(mysql.ErrEntryTooLarge) - // ErrKeyExists returns when key is already exist. + // ErrKeyExists returns when key is already exist. Caller should try to use + // GenKeyExistsErr to generate this error for correct format. ErrKeyExists = dbterror.ClassKV.NewStd(mysql.ErrDupEntry) // ErrNotImplemented returns when a function is not implemented yet. ErrNotImplemented = dbterror.ClassKV.NewStd(mysql.ErrNotImplemented) @@ -91,23 +92,8 @@ func IsErrNotFound(err error) bool { return ErrNotExist.Equal(err) } -// GetDuplicateErrorHandleString is used to concat the handle columns data with '-'. -// This is consistent with MySQL. -func GetDuplicateErrorHandleString(handle Handle) string { - dt, err := handle.Data() - if err != nil { - return err.Error() - } - var sb strings.Builder - for i, d := range dt { - if i != 0 { - sb.WriteString("-") - } - s, err := d.ToString() - if err != nil { - return err.Error() - } - sb.WriteString(s) - } - return sb.String() +// GenKeyExistsErr generates a ErrKeyExists, it concat the handle columns data +// with '-'. This is consistent with MySQL. +func GenKeyExistsErr(keyCols []string, keyName string) error { + return ErrKeyExists.FastGenByArgs(strings.Join(keyCols, "-"), keyName) } diff --git a/pkg/store/driver/txn/error.go b/pkg/store/driver/txn/error.go index dddbb4dd0f..58f3aabac8 100644 --- a/pkg/store/driver/txn/error.go +++ b/pkg/store/driver/txn/error.go @@ -39,6 +39,8 @@ import ( "go.uber.org/zap" ) +// genKeyExistsError is the fallback path when can't extract key columns for +// kv.GenKeyExistsErr. func genKeyExistsError(name string, value string, err error) error { if err != nil { logutil.BgLogger().Info("extractKeyExistsErr meets error", zap.Error(err)) @@ -58,10 +60,10 @@ func ExtractKeyExistsErrFromHandle(key kv.Key, value []byte, tblInfo *model.Tabl if pkInfo := tblInfo.GetPkColInfo(); pkInfo != nil { if mysql.HasUnsignedFlag(pkInfo.GetFlag()) { handleStr := strconv.FormatUint(uint64(handle.IntValue()), 10) - return genKeyExistsError(name, handleStr, nil) + return kv.GenKeyExistsErr([]string{handleStr}, name) } } - return genKeyExistsError(name, handle.String(), nil) + return kv.GenKeyExistsErr([]string{handle.String()}, name) } if len(value) == 0 { @@ -107,7 +109,7 @@ func ExtractKeyExistsErrFromHandle(key kv.Key, value []byte, tblInfo *model.Tabl } valueStr = append(valueStr, str) } - return genKeyExistsError(name, strings.Join(valueStr, "-"), nil) + return kv.GenKeyExistsErr(valueStr, name) } // ExtractKeyExistsErrFromIndex returns a ErrKeyExists error from a index key. diff --git a/pkg/table/tables/tables.go b/pkg/table/tables/tables.go index ae071e396f..fa9d477dc4 100644 --- a/pkg/table/tables/tables.go +++ b/pkg/table/tables/tables.go @@ -988,8 +988,8 @@ func (t *TableCommon) AddRecord(sctx table.MutateContext, r []types.Datum, opts _, err = txn.Get(ctx, key) } if err == nil { - handleStr := getDuplicateErrorHandleString(t.Meta(), recordID, r) - return recordID, kv.ErrKeyExists.FastGenByArgs(handleStr, t.Meta().Name.String()+".PRIMARY") + dupErr := getDuplicateError(t.Meta(), recordID, r) + return recordID, dupErr } else if !kv.ErrNotExist.Equal(err) { return recordID, err } @@ -1085,8 +1085,8 @@ func (t *TableCommon) AddRecord(sctx table.MutateContext, r []types.Datum, opts return recordID, nil } -// genIndexKeyStr generates index content string representation. -func genIndexKeyStr(colVals []types.Datum) (string, error) { +// genIndexKeyStrs generates index content strings representation. +func genIndexKeyStrs(colVals []types.Datum) ([]string, error) { // Pass pre-composed error to txn. strVals := make([]string, 0, len(colVals)) for _, cv := range colVals { @@ -1095,12 +1095,12 @@ func genIndexKeyStr(colVals []types.Datum) (string, error) { if !cv.IsNull() { cvs, err = types.ToString(cv.GetValue()) if err != nil { - return "", err + return nil, err } } strVals = append(strVals, cvs) } - return strings.Join(strVals, "-"), nil + return strVals, nil } // addIndices adds data into indices. If any key is duplicated, returns the original handle. @@ -1123,11 +1123,11 @@ func (t *TableCommon) addIndices(sctx table.MutateContext, recordID kv.Handle, r if !skipCheck && v.Meta().Unique { // Make error message consistent with MySQL. tablecodec.TruncateIndexValues(t.meta, v.Meta(), indexVals) - entryKey, err := genIndexKeyStr(indexVals) + colStrVals, err := genIndexKeyStrs(indexVals) if err != nil { return nil, err } - dupErr = kv.ErrKeyExists.FastGenByArgs(entryKey, fmt.Sprintf("%s.%s", v.TableMeta().Name.String(), v.Meta().Name.String())) + dupErr = kv.GenKeyExistsErr(colStrVals, fmt.Sprintf("%s.%s", v.TableMeta().Name.String(), v.Meta().Name.String())) } rsData := TryGetHandleRestoredDataWrapper(t.meta, r, nil, v.Meta()) if dupHandle, err := v.Create(sctx, txn, indexVals, recordID, rsData, opts...); err != nil { @@ -1517,13 +1517,13 @@ func (t *TableCommon) buildIndexForRow(ctx table.MutateContext, h kv.Handle, val if kv.ErrKeyExists.Equal(err) { // Make error message consistent with MySQL. tablecodec.TruncateIndexValues(t.meta, idx.Meta(), vals) - entryKey, err1 := genIndexKeyStr(vals) + colStrVals, err1 := genIndexKeyStrs(vals) if err1 != nil { - // if genIndexKeyStr failed, return the original error. + // if genIndexKeyStrs failed, return the original error. return err } - return kv.ErrKeyExists.FastGenByArgs(entryKey, fmt.Sprintf("%s.%s", idx.TableMeta().Name.String(), idx.Meta().Name.String())) + return kv.GenKeyExistsErr(colStrVals, fmt.Sprintf("%s.%s", idx.TableMeta().Name.String(), idx.Meta().Name.String())) } return err } @@ -1801,25 +1801,35 @@ func FindIndexByColName(t table.Table, name string) table.Index { return nil } -func getDuplicateErrorHandleString(tblInfo *model.TableInfo, handle kv.Handle, row []types.Datum) string { +func getDuplicateError(tblInfo *model.TableInfo, handle kv.Handle, row []types.Datum) error { + keyName := tblInfo.Name.String() + ".PRIMARY" + if handle.IsInt() { - return kv.GetDuplicateErrorHandleString(handle) + return kv.GenKeyExistsErr([]string{handle.String()}, keyName) } pkIdx := FindPrimaryIndex(tblInfo) if pkIdx == nil { - return kv.GetDuplicateErrorHandleString(handle) + handleData, err := handle.Data() + if err != nil { + return kv.ErrKeyExists.FastGenByArgs(handle.String(), keyName) + } + colStrVals, err := genIndexKeyStrs(handleData) + if err != nil { + return kv.ErrKeyExists.FastGenByArgs(handle.String(), keyName) + } + return kv.GenKeyExistsErr(colStrVals, keyName) } pkDts := make([]types.Datum, 0, len(pkIdx.Columns)) for _, idxCol := range pkIdx.Columns { pkDts = append(pkDts, row[idxCol.Offset]) } tablecodec.TruncateIndexValues(tblInfo, pkIdx, pkDts) - entryKey, err := genIndexKeyStr(pkDts) + colStrVals, err := genIndexKeyStrs(pkDts) if err != nil { - // if genIndexKeyStr failed, return DuplicateErrorHandleString. - return kv.GetDuplicateErrorHandleString(handle) + // if genIndexKeyStrs failed, return ErrKeyExists with handle.String(). + return kv.ErrKeyExists.FastGenByArgs(handle.String(), keyName) } - return entryKey + return kv.GenKeyExistsErr(colStrVals, keyName) } func init() { diff --git a/pkg/util/stringutil/string_util.go b/pkg/util/stringutil/string_util.go index c4f8600659..91ca12d60e 100644 --- a/pkg/util/stringutil/string_util.go +++ b/pkg/util/stringutil/string_util.go @@ -360,10 +360,18 @@ func (l StringerFunc) String() string { return l() } -// MemoizeStr returns memoized version of stringFunc. +// MemoizeStr returns memoized version of stringFunc. When the result of l is not +// "", it will be cached and returned directly next time. +// +// MemoizeStr is not concurrency safe. func MemoizeStr(l func() string) fmt.Stringer { + var result string return StringerFunc(func() string { - return l() + if result != "" { + return result + } + result = l() + return result }) } diff --git a/pkg/util/stringutil/string_util_test.go b/pkg/util/stringutil/string_util_test.go index 8915b700bf..4b5b429dfe 100644 --- a/pkg/util/stringutil/string_util_test.go +++ b/pkg/util/stringutil/string_util_test.go @@ -206,6 +206,18 @@ func TestEscapeGlobQuestionMark(t *testing.T) { } } +func TestMemoizeStr(t *testing.T) { + cnt := 0 + slowStringFn := func() string { + cnt++ + return "slow" + } + stringer := MemoizeStr(slowStringFn) + require.Equal(t, "slow", stringer.String()) + require.Equal(t, "slow", stringer.String()) + require.Equal(t, 1, cnt) +} + func BenchmarkDoMatch(b *testing.B) { escape := byte('\\') tbl := []struct {