Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: correct range calculation for CHAR column #10124

Merged
merged 17 commits into from
May 14, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 69 additions & 5 deletions executor/point_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@ package executor

import (
"context"
"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"
"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"
Expand Down Expand Up @@ -85,7 +89,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)
Expand Down Expand Up @@ -115,22 +119,82 @@ 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
zz-jason marked this conversation as resolved.
Show resolved Hide resolved
// 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)
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
zz-jason marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
239 changes: 239 additions & 0 deletions executor/point_get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,242 @@ 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");`)

// 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())
qw4990 marked this conversation as resolved.
Show resolved Hide resolved
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(" ", " ");`)
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) 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;`)
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 ");`)

// 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())

// // 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(` `))
}

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())
}
23 changes: 23 additions & 0 deletions util/testkit/testkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"sort"
"strings"
"sync/atomic"

"github.com/pingcap/check"
Expand Down Expand Up @@ -175,6 +176,28 @@ 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...)
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 {
Expand Down