-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
refine data truncate error msg #7401
Conversation
@crazycs520 Please read the https://github.com/pingcap/tidb/blob/master/CONTRIBUTING.md, and the proper commit message is :
Please follow this style. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reset LGTM
@@ -77,6 +77,8 @@ var ( | |||
ErrInvalidRecordKey = terror.ClassTable.New(codeInvalidRecordKey, "invalid record key") | |||
// ErrTruncateWrongValue returns for truncate wrong value for field. | |||
ErrTruncateWrongValue = terror.ClassTable.New(codeTruncateWrongValue, "incorrect value") | |||
// ErrTruncatedWrongValueForField returns for truncate wrong value for field. | |||
ErrTruncatedWrongValueForField = terror.ClassTable.New(codeTruncateWrongValue, mysql.MySQLErrName[mysql.ErrTruncatedWrongValueForField]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add code map in line 213, in tableMySQLErrCodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codeTruncateWrongValue
already in code map.
_, err = tk.Exec("insert into t values ('abc')") | ||
c.Assert(terror.ErrorEqual(err, types.ErrTruncated), IsTrue) | ||
c.Assert(terror.ErrorEqual(err, table.ErrTruncatedWrongValueForField), IsTrue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure the error code is 1366
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error code of table.ErrTruncatedWrongValueForField
is 1366.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
executor/insert_common.go
Outdated
@@ -198,11 +201,12 @@ func (e *InsertValues) getRow(cols []*table.Column, list []expression.Expression | |||
|
|||
for i, expr := range list { | |||
val, err := expr.Eval(chunk.MutRowFromDatums(row).ToRow()) | |||
if err = e.handleErr(cols[i], rowIdx, err); err != nil { | |||
valStr, _ := val.ToString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move the ToString() operation into the handErr() function? We could use a lazy way to call the ToString function. Or you will call ToString for all the data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@shenli here is a benchmark about func foo1 (d Datum){
_ = d.Kind()
}
func foo2 (d *Datum){
_ = d.Kind()
}
func AllDatumSlice() []Datum {
ds := make([]Datum,0,8)
ds = append(ds,NewIntDatum(1))
ds = append(ds,NewUintDatum(1))
ds = append(ds,NewFloat64Datum(28.0))
ds = append(ds,NewDecimalDatum(NewDecFromStringForTest("72.5")))
ds = append(ds,NewMysqlBitDatum([]byte{1,2,3,4,5,6,7,8,9}))
ds = append(ds,NewTimeDatum(Time{Time: FromGoTime(time.Now()), Fsp: 6, Type: mysql.TypeTimestamp}))
ds = append(ds,NewStringDatum("abcdefghijklmnopqrstuvwxyz"))
ds = append(ds,NewBytesDatum([]byte("abcdefghijklmnopqrstuvwxyz")))
return ds
}
func BenchmarkDatum(b *testing.B){
ds := AllDatumSlice()
for i:=0;i<b.N;i++ {
foo1(ds[i%len(ds)])
}
}
func BenchmarkDatumPoint(b *testing.B){
ds := AllDatumSlice()
for i:=0;i<b.N;i++ {
foo2(&ds[i%len(ds)])
}
} result is: ~/code/goread/src/test2 ᐅ go version
go version go1.10.3 darwin/amd64
~/code/goread/src/test2 ᐅ go test -bench=. -v test_test.go
goarch: amd64
pkg: github.com/pingcap/tidb/types
BenchmarkDatum-8 100000000 10.6 ns/op
BenchmarkDatumPoint-8 200000000 7.91 ns/op
PASS
ok github.com/pingcap/tidb/types 3.495s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Good job!
@zz-jason PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
fix #7390
In TiDB now:
In Mysql
As you can see, The return error of
insert into t values("12asdf");
in TiDB is still different from MySQL.What is changed and how it works?
Check List
Tests