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.

Sign up for GitHub

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

Jump to bottom

types: fix string to integer cast #11295

Merged
merged 16 commits into from
Jul 26, 2019
Merged

types: fix string to integer cast #11295

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
81fac8d
types: fix string to integer convert
amyangfei Jul 17, 2019
8bd3eac
fix integer convert test
amyangfei Jul 17, 2019
9bf4e6a
fix point get with integer cast
amyangfei Jul 17, 2019
a2ab02b
fix ci, add more error check in insert common
amyangfei Jul 17, 2019
7feed83
add ConvertStrToIntStrict in StatementContext
amyangfei Jul 17, 2019
651e58c
fix covert test
amyangfei Jul 17, 2019
bbcaea3
fix ci
amyangfei Jul 17, 2019
56ff762
Merge branch 'master' into cast-integer-fix
amyangfei Jul 18, 2019
d4c91fe
remove duplicated error check
amyangfei Jul 18, 2019
347572d
set ConvertStrToIntStrict in explain
amyangfei Jul 18, 2019
2542fbb
delete mode can't be in ConvertStrToIntStrict
amyangfei Jul 18, 2019
5350e14
address comment, refine variable name and add comments
amyangfei Jul 22, 2019
c964c2f
address comment
amyangfei Jul 26, 2019
c486132
Merge branch 'master' into cast-integer-fix
amyangfei Jul 26, 2019
b5f0657
recover InSelectStmt flat after test
amyangfei Jul 26, 2019
b58c2ea
Merge branch 'master' into cast-integer-fix
amyangfei Jul 26, 2019
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/point_get_test.go
View file
Open in desktop
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"))
}
4 changes: 3 additions & 1 deletion planner/core/point_get_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ func tryPointGetPlan(ctx sessionctx.Context, selStmt *ast.SelectStmt) *PointGetP
p.IsTableDual = true
return p
}
return nil
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
30 changes: 25 additions & 5 deletions types/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,31 @@ 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.InDeleteStmt && str == "" {
return "0", nil
}

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 +645,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 •
edited
Loading

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
63 changes: 52 additions & 11 deletions types/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,18 +606,18 @@ func (s *testTypeConvertSuite) TestConvert(c *C) {

// integer from string
signedAccept(c, mysql.TypeLong, " 234 ", "234")
signedAccept(c, mysql.TypeLong, " 2.35e3 ", "2350")
signedAccept(c, mysql.TypeLong, " 2.e3 ", "2000")
signedAccept(c, mysql.TypeLong, " -2.e3 ", "-2000")
signedAccept(c, mysql.TypeLong, " 2e2 ", "200")
signedAccept(c, mysql.TypeLong, " 0.002e3 ", "2")
signedAccept(c, mysql.TypeLong, " .002e3 ", "2")
signedAccept(c, mysql.TypeLong, " 20e-2 ", "0")
signedAccept(c, mysql.TypeLong, " -20e-2 ", "0")
signedAccept(c, mysql.TypeLong, " +2.51 ", "3")
signedAccept(c, mysql.TypeLong, " -9999.5 ", "-10000")
signedAccept(c, mysql.TypeLong, " 2.35e3 ", "2")
zz-jason marked this conversation as resolved.
Show resolved Hide resolved
signedAccept(c, mysql.TypeLong, " 2.e3 ", "2")
signedAccept(c, mysql.TypeLong, " -2.e3 ", "-2")
signedAccept(c, mysql.TypeLong, " 2e2 ", "2")
signedAccept(c, mysql.TypeLong, " 0.002e3 ", "0")
signedAccept(c, mysql.TypeLong, " .002e3 ", "0")
signedAccept(c, mysql.TypeLong, " 20e-2 ", "20")
signedAccept(c, mysql.TypeLong, " -20e-2 ", "-20")
signedAccept(c, mysql.TypeLong, " +2.51 ", "2")
signedAccept(c, mysql.TypeLong, " -9999.5 ", "-9999")
signedAccept(c, mysql.TypeLong, " 999.4", "999")
signedAccept(c, mysql.TypeLong, " -3.58", "-4")
signedAccept(c, mysql.TypeLong, " -3.58", "-3")
signedDeny(c, mysql.TypeLong, " 1a ", "1")
signedDeny(c, mysql.TypeLong, " +1+ ", "1")

Expand Down Expand Up @@ -689,6 +689,47 @@ 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
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)
}
}
}

func (s *testTypeConvertSuite) TestGetValidFloat(c *C) {
tests := []struct {
origin string
Expand Down
6 changes: 3 additions & 3 deletions types/datum.go
View file
Open in desktop
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