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

types: fix string to integer cast #11295

Merged
merged 16 commits into from
Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from 12 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
2 changes: 2 additions & 0 deletions executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1346,8 +1346,10 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {
sc.NotFillCache = !opts.SQLCache
}
sc.PadCharToFullLength = ctx.GetSessionVars().SQLMode.HasPadCharToFullLengthMode()
sc.CastStrToIntStrict = true
case *ast.ExplainStmt:
sc.InExplainStmt = true
sc.CastStrToIntStrict = true
case *ast.ShowStmt:
sc.IgnoreTruncate = true
sc.IgnoreZeroInDate = true
Expand Down
2 changes: 2 additions & 0 deletions executor/point_get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,6 @@ func (s *testPointGetSuite) TestIssue10677(c *C) {
tk.MustQuery("select * from t where pk = 1").Check(testkit.Rows("1"))
tk.MustQuery("desc select * from t where pk = '1'").Check(testkit.Rows("Point_Get_1 1.00 root table:t, handle:1"))
tk.MustQuery("select * from t where pk = '1'").Check(testkit.Rows("1"))
tk.MustQuery("desc select * from t where pk = '1.0'").Check(testkit.Rows("Point_Get_1 1.00 root table:t, handle:1"))
tk.MustQuery("select * from t where pk = '1.0'").Check(testkit.Rows("1"))
}
5 changes: 4 additions & 1 deletion planner/core/point_get_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ func tryPointGetPlan(ctx sessionctx.Context, selStmt *ast.SelectStmt) *PointGetP
p.IsTableDual = true
return p
}
return nil
// some scenarios cast to int with error, but we may use this value in point get
if !terror.ErrorEqual(types.ErrTruncatedWrongVal, err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we return a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning has been added in truncated error handler

return nil
}
}
cmp, err := intDatum.CompareDatum(ctx.GetSessionVars().StmtCtx, &handlePair.value)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions sessionctx/stmtctx/stmtctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ type StatementContext struct {
BatchCheck bool
InNullRejectCheck bool
AllowInvalidDate bool
// CastStrToIntStrict is used to control the way we cast float format string to int.
// If ConvertStrToIntStrict is false, we convert it to a valid float string first,
// then cast the float string to int string. Otherwise, we cast string to integer
// prefix in a strict way, only extract 0-9 and (+ or - in first bit).
CastStrToIntStrict bool

// mu struct holds variables that change during execution.
mu struct {
Expand Down
34 changes: 29 additions & 5 deletions types/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,35 @@ func NumberToDuration(number int64, fsp int) (Duration, error) {

// getValidIntPrefix gets prefix of the string which can be successfully parsed as int.
func getValidIntPrefix(sc *stmtctx.StatementContext, str string) (string, error) {
floatPrefix, err := getValidFloatPrefix(sc, str)
if err != nil {
return floatPrefix, errors.Trace(err)
if !sc.CastStrToIntStrict {
floatPrefix, err := getValidFloatPrefix(sc, str)
if err != nil {
return floatPrefix, errors.Trace(err)
}
return floatStrToIntStr(sc, floatPrefix, str)
}

validLen := 0
for i := 0; i < len(str); i++ {
c := str[i]
if c == '+' || c == '-' {
zz-jason marked this conversation as resolved.
Show resolved Hide resolved
if i != 0 {
break
}
} else if c >= '0' && c <= '9' {
validLen = i + 1
} else {
break
}
}
valid := str[:validLen]
if valid == "" {
valid = "0"
}
if validLen == 0 || validLen != len(str) {
return valid, errors.Trace(handleTruncateError(sc, ErrTruncatedWrongVal.GenWithStackByArgs("INTEGER", str)))
}
return floatStrToIntStr(sc, floatPrefix, str)
return valid, nil
}

// roundIntStr is to round int string base on the number following dot.
Expand Down Expand Up @@ -625,7 +649,7 @@ func getValidFloatPrefix(sc *stmtctx.StatementContext, s string) (valid string,
valid = "0"
}
if validLen == 0 || validLen != len(s) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if len(str) == 0, validLen can still be zero, but it's not truncated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked some behavior in MySQL, and get the following result

mysql> show create table t;                                                                                                                                                                                                                   +-------+--------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                             |
+-------+--------------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `id` int(11) DEFAULT NULL,
  `val` varchar(255) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+--------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> insert into t values ('', 'a');
ERROR 1366 (HY000): Incorrect integer value: '' for column 'id' at row 1
mysql> insert into t values ('0', 'a'), ('1', 'a');
Query OK, 2 rows affected (0.01 sec)
Records: 2  Duplicates: 0  Warnings: 0

mysql> select * from t where id = '';
+------+------+
| id   | val  |
+------+------+
|    0 | a    |
+------+------+
1 row in set (0.00 sec)

mysql> update t set val = 'b' where id = '';
Query OK, 1 row affected (0.00 sec)
Rows matched: 1  Changed: 1  Warnings: 0

mysql> select * from t;
+------+------+
| id   | val  |
+------+------+
|    0 | b    |
|    1 | a    |
+------+------+
2 rows in set (0.00 sec)

mysql> select * from t where id = '1.0';
+------+------+
| id   | val  |
+------+------+
|    1 | a    |
+------+------+
1 row in set (0.00 sec)

mysql> update t set val = 'c' where id = '1.0';
Query OK, 1 row affected (0.01 sec)
Rows matched: 1  Changed: 1  Warnings: 0

Copy link
Contributor Author

@amyangfei amyangfei Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so in select and update context, '' will be cast to 0 without no warnings. Added these two conditions in the first logic of getValidFloatPrefix

err = errors.Trace(handleTruncateError(sc))
err = errors.Trace(handleTruncateError(sc, ErrTruncated))
}
return valid, err
}
Expand Down
74 changes: 74 additions & 0 deletions types/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,80 @@ func (s *testTypeConvertSuite) TestConvert(c *C) {
signedAccept(c, mysql.TypeNewDecimal, dec, "-0.00123")
}

func (s *testTypeConvertSuite) TestGetValidInt(c *C) {
tests := []struct {
origin string
valid string
warning bool
}{
{"100", "100", false},
{"-100", "-100", false},
{"1abc", "1", true},
{"-1-1", "-1", true},
{"+1+1", "+1", true},
{"123..34", "123", true},
{"123.23E-10", "123", true},
{"1.1e1.3", "1", true},
{"11e1.3", "11", true},
{"1.", "1", true},
{".1", "0", true},
{"", "0", true},
{"123e+", "123", true},
{"123de", "123", true},
}
sc := new(stmtctx.StatementContext)
sc.TruncateAsWarning = true
sc.CastStrToIntStrict = true
warningCount := 0
for _, tt := range tests {
prefix, err := getValidIntPrefix(sc, tt.origin)
c.Assert(err, IsNil)
c.Assert(prefix, Equals, tt.valid)
_, err = strconv.ParseInt(prefix, 10, 64)
c.Assert(err, IsNil)
warnings := sc.GetWarnings()
if tt.warning {
c.Assert(warnings, HasLen, warningCount+1)
c.Assert(terror.ErrorEqual(warnings[len(warnings)-1].Err, ErrTruncatedWrongVal), IsTrue)
warningCount += 1
} else {
c.Assert(warnings, HasLen, warningCount)
}
}

tests2 := []struct {
origin string
valid string
warning bool
}{
{"100", "100", false},
{"-100", "-100", false},
{"1abc", "1", true},
{"-1-1", "-1", true},
{"+1+1", "+1", true},
{"123..34", "123.", true},
{"123.23E-10", "0", false},
{"1.1e1.3", "1.1e1", true},
{"11e1.3", "11e1", true},
{"1.", "1", false},
{".1", "0", false},
{"", "0", true},
{"123e+", "123", true},
{"123de", "123", true},
}
sc.TruncateAsWarning = false
sc.CastStrToIntStrict = false
for _, tt := range tests2 {
prefix, err := getValidIntPrefix(sc, tt.origin)
if tt.warning {
c.Assert(terror.ErrorEqual(err, ErrTruncated), IsTrue)
} else {
c.Assert(err, IsNil)
}
c.Assert(prefix, Equals, tt.valid)
}
}

func (s *testTypeConvertSuite) TestGetValidFloat(c *C) {
tests := []struct {
origin string
Expand Down
6 changes: 3 additions & 3 deletions types/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -1780,14 +1780,14 @@ func (ds *datumsSorter) Swap(i, j int) {
ds.datums[i], ds.datums[j] = ds.datums[j], ds.datums[i]
}

func handleTruncateError(sc *stmtctx.StatementContext) error {
func handleTruncateError(sc *stmtctx.StatementContext, err error) error {
if sc.IgnoreTruncate {
return nil
}
if !sc.TruncateAsWarning {
return ErrTruncated
return err
}
sc.AppendWarning(ErrTruncated)
sc.AppendWarning(err)
return nil
}

Expand Down