From 4097f690a25d74559d849bb95e54c088a90965ad Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Thu, 11 Apr 2019 15:47:26 +0800 Subject: [PATCH 1/8] executor: correct point calculatation for CHAR[N] index column --- executor/point_get.go | 47 ++++++++++++++++++++++++++++++++++---- executor/point_get_test.go | 39 +++++++++++++++++++++++++++++++ util/testkit/testkit.go | 9 ++++++++ 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 1d170b8dfe087..ebded466178e6 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -15,6 +15,7 @@ package executor import ( "context" + "strings" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" @@ -22,6 +23,7 @@ import ( "github.com/pingcap/tidb/kv" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" @@ -85,7 +87,7 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.RecordBatch) err } if e.idxInfo != nil { idxKey, err1 := e.encodeIndexKey() - if err1 != nil { + if err1 != nil && !kv.ErrNotExist.Equal(err1) { return err1 } handleVal, err1 := e.get(idxKey) @@ -115,22 +117,57 @@ func (e *PointGetExecutor) Next(ctx context.Context, req *chunk.RecordBatch) err return e.decodeRowValToChunk(val, req.Chunk) } -func (e *PointGetExecutor) encodeIndexKey() ([]byte, error) { +func (e *PointGetExecutor) encodeIndexKey() (_ []byte, err error) { + sc := e.ctx.GetSessionVars().StmtCtx for i := range e.idxVals { colInfo := e.tblInfo.Columns[e.idxInfo.Columns[i].Offset] - casted, err := table.CastValue(e.ctx, e.idxVals[i], colInfo) + if colInfo.Tp == mysql.TypeString || colInfo.Tp == mysql.TypeVarString || colInfo.Tp == mysql.TypeVarchar { + e.idxVals[i], err = handlePadCharToFullLength(sc, &colInfo.FieldType, e.idxVals[i]) + } else { + e.idxVals[i], err = table.CastValue(e.ctx, e.idxVals[i], colInfo) + } if err != nil { return nil, err } - e.idxVals[i] = casted } - encodedIdxVals, err := codec.EncodeKey(e.ctx.GetSessionVars().StmtCtx, nil, e.idxVals...) + + encodedIdxVals, err := codec.EncodeKey(sc, nil, e.idxVals...) if err != nil { return nil, err } return tablecodec.EncodeIndexSeekKey(e.tblInfo.ID, e.idxInfo.ID, encodedIdxVals), nil } +// handlePadCharToFullLength handles the "PAD_CHAR_TO_FULL_LENGTH" sql mode for +// CHAR[N] index columns. + +// NOTE: kv.ErrNotExist is returned to indicate that this value can not match +// any (key, value) pair in tikv storage. This error should be handle by +// the caller. +func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType, val types.Datum) (types.Datum, error) { + targetStr, err := val.ToString() + if err != nil { + return val, err + } + + if !sc.PadCharToFullLength || types.IsBinaryStr(ft) || ft.Tp == mysql.TypeVarchar || ft.Tp == mysql.TypeVarString { + val.SetString(targetStr) + return val, nil + } + + if len(targetStr) != ft.Flen { + // return kv.ErrNotExist to indicate that this value can not match any + // (key, value) pair in tikv storage. + return val, kv.ErrNotExist + } + + // Trailing spaces of data typed "CHAR[N]" is trimed in the storage, we + // need to trim these trailing spaces as well. + noTrailingSpace := strings.TrimRight(targetStr, " ") + val.SetString(noTrailingSpace) + return val, nil +} + func (e *PointGetExecutor) get(key kv.Key) (val []byte, err error) { txn, err := e.ctx.Txn(true) if err != nil { diff --git a/executor/point_get_test.go b/executor/point_get_test.go index e3ba00ccf9f7f..348b99fdcedc9 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -56,3 +56,42 @@ func (s *testSuite1) TestPointGet(c *C) { `5 5 6 5 6 7 6 7 7`, )) } + +func (s *testSuite1) TestPointGetCharPK(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 char(2) primary key, b char(2));`) + + tk.MustExec(`insert into t values("aa", "bb");`) + tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) + + tk.MustExec(`truncate table t;`) + tk.MustExec(`insert into t values("a ", "b ");`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + // Test CHAR BINARY. + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t(a char(2) binary primary key, b char(2));`) + + tk.MustExec(`insert into t values("aa", "bb");`) + tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) + + tk.MustExec(`truncate table t;`) + tk.MustExec(`insert into t values("a ", "b ");`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) +} diff --git a/util/testkit/testkit.go b/util/testkit/testkit.go index 2eaf3302cbebb..ec339b22c4163 100644 --- a/util/testkit/testkit.go +++ b/util/testkit/testkit.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "sort" + "strings" "sync/atomic" "github.com/pingcap/check" @@ -175,6 +176,14 @@ func (tk *TestKit) MustExec(sql string, args ...interface{}) { } } +// MustPointGet checks whether the plan for the sql is Point_Get. +func (tk *TestKit) MustPointGet(sql string, args ...interface{}) *Result { + rs := tk.MustQuery("explain "+sql, args...) + tk.c.Assert(len(rs.rows), check.Equals, 1) + tk.c.Assert(strings.Contains(rs.rows[0][0], "Point_Get"), check.IsTrue) + return tk.MustQuery(sql, args...) +} + // MustQuery query the statements and returns result rows. // If expected result is set it asserts the query result equals expected result. func (tk *TestKit) MustQuery(sql string, args ...interface{}) *Result { From c467be09ad8d5493df5b7c20a043067032b786eb Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Thu, 11 Apr 2019 21:21:02 +0800 Subject: [PATCH 2/8] consider varchar --- executor/point_get.go | 47 ++++++++++++++------ executor/point_get_test.go | 91 +++++++++++++++++++++++++++++++++++--- 2 files changed, 120 insertions(+), 18 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index ebded466178e6..083b62a2d486f 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -17,6 +17,7 @@ import ( "context" "strings" + "github.com/pingcap/errors" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/expression" @@ -150,22 +151,42 @@ func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType return val, err } - if !sc.PadCharToFullLength || types.IsBinaryStr(ft) || ft.Tp == mysql.TypeVarchar || ft.Tp == mysql.TypeVarString { + isBinary := mysql.HasBinaryFlag(ft.Flag) // || ft.Collate == charset.CollationBin + isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar + isChar := ft.Tp == mysql.TypeString + switch { + case isVarchar && isBinary: + noTrailingSpace := strings.TrimRight(targetStr, " ") + if numSpacesToFill := ft.Flen - len(noTrailingSpace); numSpacesToFill > 0 { + noTrailingSpace += strings.Repeat(" ", numSpacesToFill) + } + val.SetString(noTrailingSpace) + return val, nil + case isVarchar && !isBinary: val.SetString(targetStr) return val, nil + case isChar && isBinary: + noTrailingSpace := strings.TrimRight(targetStr, " ") + val.SetString(noTrailingSpace) + return val, nil + case isChar && !isBinary && !sc.PadCharToFullLength: + val.SetString(targetStr) + return val, nil + case isChar && !isBinary && sc.PadCharToFullLength: + if len(targetStr) != ft.Flen { + // return kv.ErrNotExist to indicate that this value can not match any + // (key, value) pair in tikv storage. + return val, kv.ErrNotExist + } + // Trailing spaces of data typed "CHAR[N]" is trimed in the storage, we + // need to trim these trailing spaces as well. + noTrailingSpace := strings.TrimRight(targetStr, " ") + val.SetString(noTrailingSpace) + return val, nil + default: + // should never happen. + return val, errors.Errorf("handlePadCharToFullLength: unhandled FieldType: %v", ft.String()) } - - if len(targetStr) != ft.Flen { - // return kv.ErrNotExist to indicate that this value can not match any - // (key, value) pair in tikv storage. - return val, kv.ErrNotExist - } - - // Trailing spaces of data typed "CHAR[N]" is trimed in the storage, we - // need to trim these trailing spaces as well. - noTrailingSpace := strings.TrimRight(targetStr, " ") - val.SetString(noTrailingSpace) - return val, nil } func (e *PointGetExecutor) get(key kv.Key) (val []byte, err error) { diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 348b99fdcedc9..bbcbeffeaa09b 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -62,36 +62,117 @@ func (s *testSuite1) TestPointGetCharPK(c *C) { tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) tk.MustExec(`create table t(a char(2) primary key, b char(2));`) - tk.MustExec(`insert into t values("aa", "bb");`) + + // Test truncate without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) + tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) + + // Test truncate with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) tk.MustExec(`truncate table t;`) tk.MustExec(`insert into t values("a ", "b ");`) + + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) - // Test CHAR BINARY. + // // Test CHAR BINARY. tk.MustExec(`drop table if exists t;`) tk.MustExec(`create table t(a char(2) binary primary key, b char(2));`) + tk.MustExec(`insert into t values(" ", " ");`) + tk.MustExec(`insert into t values("a ", "b ");`) + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) +} + +func (s *testSuite1) TestPointGetVarcharPK(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 varchar(2) primary key, b varchar(2));`) tk.MustExec(`insert into t values("aa", "bb");`) + + // Test truncate without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) + tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) + + // Test truncate with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) tk.MustPointGet(`select * from t where a = "aab";`).Check(testkit.Rows()) tk.MustExec(`truncate table t;`) tk.MustExec(`insert into t values("a ", "b ");`) - tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b`)) - tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) - tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + // // Test VARCHAR BINARY. + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t(a varchar(2) binary primary key, b varchar(2));`) + tk.MustExec(`insert into t values(" ", " ");`) + tk.MustExec(`insert into t values("a ", "b ");`) + + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) + tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) } From bf9e6b0d4e6a48679c5c7feb30d1aeeb315ddb3b Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Tue, 16 Apr 2019 13:43:13 +0800 Subject: [PATCH 3/8] consider binary type --- executor/point_get.go | 18 ++++-- executor/point_get_test.go | 119 +++++++++++++++++++++++++++++++++++++ util/testkit/testkit.go | 14 +++++ 3 files changed, 145 insertions(+), 6 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 083b62a2d486f..c4e0042e6f7df 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -18,6 +18,7 @@ import ( "strings" "github.com/pingcap/errors" + "github.com/pingcap/parser/charset" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/expression" @@ -151,28 +152,33 @@ func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType return val, err } - isBinary := mysql.HasBinaryFlag(ft.Flag) // || ft.Collate == charset.CollationBin + hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar isChar := ft.Tp == mysql.TypeString + isBinary := ft.Tp == mysql.TypeString && ft.Collate == charset.CollationBin + isVarBinary := ft.Tp == mysql.TypeString && ft.Collate == charset.CollationBin switch { - case isVarchar && isBinary: + case isBinary || isVarBinary: + val.SetString(targetStr) + return val, nil + case isVarchar && hasBinaryFlag: noTrailingSpace := strings.TrimRight(targetStr, " ") if numSpacesToFill := ft.Flen - len(noTrailingSpace); numSpacesToFill > 0 { noTrailingSpace += strings.Repeat(" ", numSpacesToFill) } val.SetString(noTrailingSpace) return val, nil - case isVarchar && !isBinary: + case isVarchar && !hasBinaryFlag: val.SetString(targetStr) return val, nil - case isChar && isBinary: + case isChar && hasBinaryFlag: noTrailingSpace := strings.TrimRight(targetStr, " ") val.SetString(noTrailingSpace) return val, nil - case isChar && !isBinary && !sc.PadCharToFullLength: + case isChar && !hasBinaryFlag && !sc.PadCharToFullLength: val.SetString(targetStr) return val, nil - case isChar && !isBinary && sc.PadCharToFullLength: + case isChar && !hasBinaryFlag && sc.PadCharToFullLength: if len(targetStr) != ft.Flen { // return kv.ErrNotExist to indicate that this value can not match any // (key, value) pair in tikv storage. diff --git a/executor/point_get_test.go b/executor/point_get_test.go index bbcbeffeaa09b..88b4635ea1e86 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -114,6 +114,63 @@ func (s *testSuite1) TestPointGetCharPK(c *C) { tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) } +func (s *testSuite1) TestIndexLookupCharPK(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 char(2) primary key, b char(2));`) + tk.MustExec(`insert into t values("aa", "bb");`) + + // Test truncate without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustIndexLookup(`select * from t tmp where a = "aa";`).Check(testkit.Rows(`aa bb`)) + tk.MustIndexLookup(`select * from t tmp where a = "aab";`).Check(testkit.Rows()) + + // Test truncate with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "aa";`).Check(testkit.Rows(`aa bb`)) + tk.MustIndexLookup(`select * from t tmp where a = "aab";`).Check(testkit.Rows()) + + tk.MustExec(`truncate table t;`) + tk.MustExec(`insert into t values("a ", "b ");`) + + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + + // // Test CHAR BINARY. + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t(a char(2) binary primary key, b char(2));`) + tk.MustExec(`insert into t values(" ", " ");`) + tk.MustExec(`insert into t values("a ", "b ");`) + + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) +} + func (s *testSuite1) TestPointGetVarcharPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) @@ -176,3 +233,65 @@ func (s *testSuite1) TestPointGetVarcharPK(c *C) { tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) } + +func (s *testSuite1) TestPointGetBinaryPK(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 binary(2) primary key, b binary(2));`) + tk.MustExec(`insert into t values("a", "b");`) + + tk.MustExec(`set @@sql_mode="";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a\0";`).Check(testkit.Rows("a\x00 b\x00")) + + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a\0";`).Check(testkit.Rows("a\x00 b\x00")) + + tk.MustExec(`insert into t values("a ", "b ");`) + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) + + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustPointGet(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) +} + +func (s *testSuite1) TestIndexLookupBinaryPK(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 binary(2) primary key, b binary(2));`) + tk.MustExec(`insert into t values("a", "b");`) + + tk.MustExec(`set @@sql_mode="";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a\0";`).Check(testkit.Rows("a\x00 b\x00")) + + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a\0";`).Check(testkit.Rows("a\x00 b\x00")) + + tk.MustExec(`insert into t values("a ", "b ");`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + + // `PAD_CHAR_TO_FULL_LENGTH` should not affect the result. + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b `)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) +} diff --git a/util/testkit/testkit.go b/util/testkit/testkit.go index ec339b22c4163..f846420c09788 100644 --- a/util/testkit/testkit.go +++ b/util/testkit/testkit.go @@ -176,6 +176,20 @@ func (tk *TestKit) MustExec(sql string, args ...interface{}) { } } +// MustIndexLookup checks whether the plan for the sql is Point_Get. +func (tk *TestKit) MustIndexLookup(sql string, args ...interface{}) *Result { + rs := tk.MustQuery("explain "+sql, args...) + hasIndexLookup := false + for i := range rs.rows { + if strings.Contains(rs.rows[i][0], "IndexLookUp") { + hasIndexLookup = true + break + } + } + tk.c.Assert(hasIndexLookup, check.IsTrue) + return tk.MustQuery(sql, args...) +} + // MustPointGet checks whether the plan for the sql is Point_Get. func (tk *TestKit) MustPointGet(sql string, args ...interface{}) *Result { rs := tk.MustQuery("explain "+sql, args...) From e87dfbdbe2983763bae682b640afa28da3f3ba3e Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Thu, 9 May 2019 09:10:24 +0800 Subject: [PATCH 4/8] comment out unpassed UTs --- executor/point_get.go | 8 +++---- executor/point_get_test.go | 46 +++++++++++++++++++------------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index b7630338ee6bf..4519290ad8ac1 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -161,7 +161,7 @@ func (e *PointGetExecutor) encodeIndexKey() (_ []byte, err error) { // CHAR[N] index columns. // NOTE: kv.ErrNotExist is returned to indicate that this value can not match -// any (key, value) pair in tikv storage. This error should be handle by +// any (key, value) pair in tikv storage. This error should be handled by // the caller. func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType, val types.Datum) (types.Datum, error) { targetStr, err := val.ToString() @@ -170,10 +170,10 @@ func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType } hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) - isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar isChar := ft.Tp == mysql.TypeString - isBinary := ft.Tp == mysql.TypeString && ft.Collate == charset.CollationBin - isVarBinary := ft.Tp == mysql.TypeString && ft.Collate == charset.CollationBin + isBinary := isChar && ft.Collate == charset.CollationBin + isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar + isVarBinary := isVarchar && ft.Collate == charset.CollationBin switch { case isBinary || isVarBinary: val.SetString(targetStr) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 88b4635ea1e86..88c3ec66e062d 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -140,35 +140,35 @@ func (s *testSuite1) TestIndexLookupCharPK(c *C) { tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) - // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. - tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) - tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + // // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) - // // Test CHAR BINARY. + // Test CHAR BINARY. tk.MustExec(`drop table if exists t;`) tk.MustExec(`create table t(a char(2) binary primary key, b char(2));`) tk.MustExec(`insert into t values(" ", " ");`) tk.MustExec(`insert into t values("a ", "b ");`) - // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. - tk.MustExec(`set @@sql_mode="";`) - tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - - // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. - tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) - tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // tk.MustExec(`set @@sql_mode="";`) + // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + + // // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + // tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) } func (s *testSuite1) TestPointGetVarcharPK(c *C) { From 8bb324d6d5b9c03dcb7b411904255fa8329a59e5 Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Thu, 9 May 2019 09:44:09 +0800 Subject: [PATCH 5/8] tiny refine tests --- executor/executor_test.go | 1 + executor/point_get_test.go | 60 ++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index c21b8104fae28..937111eff316e 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -89,6 +89,7 @@ var _ = Suite(&testSuite3{}) var _ = Suite(&testBypassSuite{}) var _ = Suite(&testUpdateSuite{}) var _ = Suite(&testOOMSuite{}) +var _ = Suite(&testPointGetSuite{}) type testSuite struct { cluster *mocktikv.Cluster diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 88c3ec66e062d..3292ea193bda4 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -14,11 +14,57 @@ package executor_test import ( + "fmt" + . "github.com/pingcap/check" + "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/session" + "github.com/pingcap/tidb/store/mockstore" + "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/util/testkit" ) -func (s *testSuite1) TestPointGet(c *C) { +type testPointGetSuite struct { + store kv.Storage + dom *domain.Domain + cli *checkRequestClient +} + +func (s *testPointGetSuite) SetUpSuite(c *C) { + cli := &checkRequestClient{} + hijackClient := func(c tikv.Client) tikv.Client { + cli.Client = c + return cli + } + s.cli = cli + + var err error + s.store, err = mockstore.NewMockTikvStore( + mockstore.WithHijackClient(hijackClient), + ) + c.Assert(err, IsNil) + s.dom, err = session.BootstrapSession(s.store) + c.Assert(err, IsNil) + s.dom.SetStatsUpdating(true) +} + +func (s *testPointGetSuite) TearDownSuite(c *C) { + s.dom.Close() + s.store.Close() +} + +func (s *testPointGetSuite) TearDownTest(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + r := tk.MustQuery("show tables") + for _, tb := range r.Rows() { + tableName := tb[0] + tk.MustExec(fmt.Sprintf("drop table %v", tableName)) + } +} + +func (s *testPointGetSuite) TestPointGet(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("create table point (id int primary key, c int, d varchar(10), unique c_d (c, d))") @@ -57,7 +103,7 @@ func (s *testSuite1) TestPointGet(c *C) { )) } -func (s *testSuite1) TestPointGetCharPK(c *C) { +func (s *testPointGetSuite) TestPointGetCharPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) @@ -114,7 +160,7 @@ func (s *testSuite1) TestPointGetCharPK(c *C) { tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) } -func (s *testSuite1) TestIndexLookupCharPK(c *C) { +func (s *testPointGetSuite) TestIndexLookupCharPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) @@ -140,6 +186,7 @@ func (s *testSuite1) TestIndexLookupCharPK(c *C) { tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + // TODO: fix https://github.com/pingcap/tidb/issues/10397 and uncomment the following tests. // // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. // tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) @@ -152,6 +199,7 @@ func (s *testSuite1) TestIndexLookupCharPK(c *C) { tk.MustExec(`insert into t values(" ", " ");`) tk.MustExec(`insert into t values("a ", "b ");`) + // TODO: fix https://github.com/pingcap/tidb/issues/10398 and uncomment the following tests. // // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. // tk.MustExec(`set @@sql_mode="";`) // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) @@ -171,7 +219,7 @@ func (s *testSuite1) TestIndexLookupCharPK(c *C) { // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) } -func (s *testSuite1) TestPointGetVarcharPK(c *C) { +func (s *testPointGetSuite) TestPointGetVarcharPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) @@ -234,7 +282,7 @@ func (s *testSuite1) TestPointGetVarcharPK(c *C) { tk.MustPointGet(`select * from t where a = " ";`).Check(testkit.Rows(` `)) } -func (s *testSuite1) TestPointGetBinaryPK(c *C) { +func (s *testPointGetSuite) TestPointGetBinaryPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) @@ -265,7 +313,7 @@ func (s *testSuite1) TestPointGetBinaryPK(c *C) { tk.MustPointGet(`select * from t where a = "a ";`).Check(testkit.Rows()) } -func (s *testSuite1) TestIndexLookupBinaryPK(c *C) { +func (s *testPointGetSuite) TestIndexLookupBinaryPK(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`use test;`) tk.MustExec(`drop table if exists t;`) From 50d3834df1018653005fea67b88cc7a4e0d0f8aa Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Mon, 13 May 2019 16:48:59 +0800 Subject: [PATCH 6/8] fix index lookup --- executor/point_get.go | 62 ++------------------------------ executor/point_get_test.go | 46 ++++++++++++------------ util/ranger/points.go | 72 +++++++++++++++++++++++++++++++++++++- 3 files changed, 95 insertions(+), 85 deletions(-) diff --git a/executor/point_get.go b/executor/point_get.go index 4519290ad8ac1..6729eeb1483db 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -15,24 +15,21 @@ package executor import ( "context" - "strings" - "github.com/pingcap/errors" "github.com/pingcap/failpoint" - "github.com/pingcap/parser/charset" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/kv" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/codec" + "github.com/pingcap/tidb/util/ranger" ) func (b *executorBuilder) buildPointGet(p *plannercore.PointGetPlan) Executor { @@ -141,7 +138,7 @@ func (e *PointGetExecutor) encodeIndexKey() (_ []byte, err error) { for i := range e.idxVals { colInfo := e.tblInfo.Columns[e.idxInfo.Columns[i].Offset] if colInfo.Tp == mysql.TypeString || colInfo.Tp == mysql.TypeVarString || colInfo.Tp == mysql.TypeVarchar { - e.idxVals[i], err = handlePadCharToFullLength(sc, &colInfo.FieldType, e.idxVals[i]) + e.idxVals[i], err = ranger.HandlePadCharToFullLength(sc, &colInfo.FieldType, e.idxVals[i]) } else { e.idxVals[i], err = table.CastValue(e.ctx, e.idxVals[i], colInfo) } @@ -157,61 +154,6 @@ func (e *PointGetExecutor) encodeIndexKey() (_ []byte, err error) { return tablecodec.EncodeIndexSeekKey(e.tblInfo.ID, e.idxInfo.ID, encodedIdxVals), nil } -// handlePadCharToFullLength handles the "PAD_CHAR_TO_FULL_LENGTH" sql mode for -// CHAR[N] index columns. - -// NOTE: kv.ErrNotExist is returned to indicate that this value can not match -// any (key, value) pair in tikv storage. This error should be handled by -// the caller. -func handlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType, val types.Datum) (types.Datum, error) { - targetStr, err := val.ToString() - if err != nil { - return val, err - } - - hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) - isChar := ft.Tp == mysql.TypeString - isBinary := isChar && ft.Collate == charset.CollationBin - isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar - isVarBinary := isVarchar && ft.Collate == charset.CollationBin - switch { - case isBinary || isVarBinary: - val.SetString(targetStr) - return val, nil - case isVarchar && hasBinaryFlag: - noTrailingSpace := strings.TrimRight(targetStr, " ") - if numSpacesToFill := ft.Flen - len(noTrailingSpace); numSpacesToFill > 0 { - noTrailingSpace += strings.Repeat(" ", numSpacesToFill) - } - val.SetString(noTrailingSpace) - return val, nil - case isVarchar && !hasBinaryFlag: - val.SetString(targetStr) - return val, nil - case isChar && hasBinaryFlag: - noTrailingSpace := strings.TrimRight(targetStr, " ") - val.SetString(noTrailingSpace) - return val, nil - case isChar && !hasBinaryFlag && !sc.PadCharToFullLength: - val.SetString(targetStr) - return val, nil - case isChar && !hasBinaryFlag && sc.PadCharToFullLength: - if len(targetStr) != ft.Flen { - // return kv.ErrNotExist to indicate that this value can not match any - // (key, value) pair in tikv storage. - return val, kv.ErrNotExist - } - // Trailing spaces of data typed "CHAR[N]" is trimed in the storage, we - // need to trim these trailing spaces as well. - noTrailingSpace := strings.TrimRight(targetStr, " ") - val.SetString(noTrailingSpace) - return val, nil - default: - // should never happen. - return val, errors.Errorf("handlePadCharToFullLength: unhandled FieldType: %v", ft.String()) - } -} - func (e *PointGetExecutor) get(key kv.Key) (val []byte, err error) { txn, err := e.ctx.Txn(true) if err != nil { diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 3292ea193bda4..bab2bb1e33fde 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -186,12 +186,11 @@ func (s *testPointGetSuite) TestIndexLookupCharPK(c *C) { tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) - // TODO: fix https://github.com/pingcap/tidb/issues/10397 and uncomment the following tests. - // // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. - // tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) - // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows()) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows()) // Test CHAR BINARY. tk.MustExec(`drop table if exists t;`) @@ -199,24 +198,23 @@ func (s *testPointGetSuite) TestIndexLookupCharPK(c *C) { tk.MustExec(`insert into t values(" ", " ");`) tk.MustExec(`insert into t values("a ", "b ");`) - // TODO: fix https://github.com/pingcap/tidb/issues/10398 and uncomment the following tests. - // // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. - // tk.MustExec(`set @@sql_mode="";`) - // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - - // // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. - // tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) - // tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) - // tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + // Test trailing spaces without sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + + // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. + tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) + tk.MustIndexLookup(`select * from t tmp where a = "a";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = "a ";`).Check(testkit.Rows(`a b`)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) + tk.MustIndexLookup(`select * from t tmp where a = " ";`).Check(testkit.Rows(` `)) } func (s *testPointGetSuite) TestPointGetVarcharPK(c *C) { diff --git a/util/ranger/points.go b/util/ranger/points.go index 9e73643068394..8ad0abc954f26 100644 --- a/util/ranger/points.go +++ b/util/ranger/points.go @@ -17,12 +17,15 @@ import ( "fmt" "math" "sort" + "strings" "github.com/pingcap/errors" "github.com/pingcap/parser/ast" + "github.com/pingcap/parser/charset" "github.com/pingcap/parser/mysql" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/expression" + "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" @@ -208,11 +211,19 @@ func (r *builder) buildFormBinOp(expr *expression.ScalarFunction) []point { op string value types.Datum err error + ft *types.FieldType ) - if _, ok := expr.GetArgs()[0].(*expression.Column); ok { + if col, ok := expr.GetArgs()[0].(*expression.Column); ok { value, err = expr.GetArgs()[1].Eval(chunk.Row{}) op = expr.FuncName.L + ft = col.RetType } else { + col, ok := expr.GetArgs()[1].(*expression.Column) + if !ok { + return nil + } + ft = col.RetType + value, err = expr.GetArgs()[0].Eval(chunk.Row{}) switch expr.FuncName.L { case ast.GE: @@ -234,6 +245,11 @@ func (r *builder) buildFormBinOp(expr *expression.ScalarFunction) []point { return nil } + value, err = HandlePadCharToFullLength(r.sc, ft, value) + if err != nil { + return nil + } + switch op { case ast.EQ: startPoint := point{value: value, start: true} @@ -265,6 +281,60 @@ func (r *builder) buildFormBinOp(expr *expression.ScalarFunction) []point { return nil } +// HandlePadCharToFullLength handles the "PAD_CHAR_TO_FULL_LENGTH" sql mode for +// CHAR[N] index columns. +// NOTE: kv.ErrNotExist is returned to indicate that this value can not match +// any (key, value) pair in tikv storage. This error should be handled by +// the caller. +func HandlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType, val types.Datum) (types.Datum, error) { + targetStr, err := val.ToString() + if err != nil { + return val, err + } + + hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) + isChar := ft.Tp == mysql.TypeString + isBinary := isChar && ft.Collate == charset.CollationBin + isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar + isVarBinary := isVarchar && ft.Collate == charset.CollationBin + switch { + case isBinary || isVarBinary: + val.SetString(targetStr) + return val, nil + case isVarchar && hasBinaryFlag: + noTrailingSpace := strings.TrimRight(targetStr, " ") + if numSpacesToFill := ft.Flen - len(noTrailingSpace); numSpacesToFill > 0 { + noTrailingSpace += strings.Repeat(" ", numSpacesToFill) + } + val.SetString(noTrailingSpace) + return val, nil + case isVarchar && !hasBinaryFlag: + val.SetString(targetStr) + return val, nil + case isChar && hasBinaryFlag: + noTrailingSpace := strings.TrimRight(targetStr, " ") + val.SetString(noTrailingSpace) + return val, nil + case isChar && !hasBinaryFlag && !sc.PadCharToFullLength: + val.SetString(targetStr) + return val, nil + case isChar && !hasBinaryFlag && sc.PadCharToFullLength: + if len(targetStr) != ft.Flen { + // return kv.ErrNotExist to indicate that this value can not match any + // (key, value) pair in tikv storage. + return val, kv.ErrNotExist + } + // Trailing spaces of data typed "CHAR[N]" is trimed in the storage, we + // need to trim these trailing spaces as well. + noTrailingSpace := strings.TrimRight(targetStr, " ") + val.SetString(noTrailingSpace) + return val, nil + default: + // should never happen. + return val, errors.Errorf("HandlePadCharToFullLength: unhandled FieldType: %v", ft.String()) + } +} + func (r *builder) buildFromIsTrue(expr *expression.ScalarFunction, isNot int) []point { if isNot == 1 { // NOT TRUE range is {[null null] [0, 0]} From abe03a2bedd635fbe9988c3e41a9dbe69f0635fd Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Tue, 14 May 2019 09:19:31 +0800 Subject: [PATCH 7/8] fix ut --- util/ranger/points.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/util/ranger/points.go b/util/ranger/points.go index 8ad0abc954f26..7e458329c03cb 100644 --- a/util/ranger/points.go +++ b/util/ranger/points.go @@ -330,8 +330,7 @@ func HandlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType val.SetString(noTrailingSpace) return val, nil default: - // should never happen. - return val, errors.Errorf("HandlePadCharToFullLength: unhandled FieldType: %v", ft.String()) + return val, nil } } From 88b5fbb89f7306d0a18f5584ff4fdb4f837c7829 Mon Sep 17 00:00:00 2001 From: Zhang Jian Date: Tue, 14 May 2019 15:56:50 +0800 Subject: [PATCH 8/8] address comment --- util/ranger/points.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/util/ranger/points.go b/util/ranger/points.go index 7e458329c03cb..75ae0a793ed3b 100644 --- a/util/ranger/points.go +++ b/util/ranger/points.go @@ -287,16 +287,21 @@ func (r *builder) buildFormBinOp(expr *expression.ScalarFunction) []point { // any (key, value) pair in tikv storage. This error should be handled by // the caller. func HandlePadCharToFullLength(sc *stmtctx.StatementContext, ft *types.FieldType, val types.Datum) (types.Datum, error) { + isChar := (ft.Tp == mysql.TypeString) + isBinary := (isChar && ft.Collate == charset.CollationBin) + isVarchar := (ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar) + isVarBinary := (isVarchar && ft.Collate == charset.CollationBin) + + if !isChar && !isVarchar && !isBinary && !isVarBinary { + return val, nil + } + + hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) targetStr, err := val.ToString() if err != nil { return val, err } - hasBinaryFlag := mysql.HasBinaryFlag(ft.Flag) - isChar := ft.Tp == mysql.TypeString - isBinary := isChar && ft.Collate == charset.CollationBin - isVarchar := ft.Tp == mysql.TypeVarString || ft.Tp == mysql.TypeVarchar - isVarBinary := isVarchar && ft.Collate == charset.CollationBin switch { case isBinary || isVarBinary: val.SetString(targetStr)