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

types: add error code for package types #13300

Merged
merged 11 commits into from
Nov 26, 2019
Merged

types: add error code for package types #13300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
813f7b7
types: add error code for package types
jackysp Nov 12, 2019
fe69abb
tiny fix
jackysp Nov 12, 2019
2356016
Merge branch 'master' into types_errors
jackysp Nov 21, 2019
f668ff9
address comments
jackysp Nov 21, 2019
e503d87
fix more
jackysp Nov 21, 2019
6350324
fix test
jackysp Nov 21, 2019
23dbdec
fix test
jackysp Nov 21, 2019
afcbfed
clean up
jackysp Nov 21, 2019
7e92952
Merge branch 'master' into types_errors
sre-bot Nov 26, 2019
d3e0375
fix compile
jackysp Nov 26, 2019
43fae1e
Merge branch 'master' into types_errors
lysu Nov 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
55 changes: 28 additions & 27 deletions executor/insert_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,21 +245,34 @@ func (e *InsertValues) handleErr(col *table.Column, val *types.Datum, rowIdx int
return nil
}

if types.ErrDataTooLong.Equal(err) {
return resetErrDataTooLong(col.Name.O, rowIdx+1, err)
// Convert the error with full messages.
var (
colTp byte
colName string
)
if col != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

if col == nil does following code will generate error message use unspecified type and '' column name?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes...

colTp = col.Tp
colName = col.Name.String()
}

if types.ErrOverflow.Equal(err) {
return types.ErrWarnDataOutOfRange.GenWithStackByArgs(col.Name.O, rowIdx+1)
}
if types.ErrTruncated.Equal(err) {
if types.ErrDataTooLong.Equal(err) {
err = resetErrDataTooLong(colName, rowIdx+1, err)
} else if types.ErrOverflow.Equal(err) {
err = types.ErrWarnDataOutOfRange.GenWithStackByArgs(colName, rowIdx+1)
} else if types.ErrTruncated.Equal(err) || types.ErrTruncatedWrongVal.Equal(err) {
valStr, err1 := val.ToString()
if err1 != nil {
logutil.BgLogger().Warn("truncate value failed", zap.Error(err1))
}
return table.ErrTruncatedWrongValueForField.GenWithStackByArgs(types.TypeStr(col.Tp), valStr, col.Name.O, rowIdx+1)
err = table.ErrTruncatedWrongValueForField.GenWithStackByArgs(types.TypeStr(colTp), valStr, colName, rowIdx+1)
}
return e.filterErr(err)

if !e.ctx.GetSessionVars().StmtCtx.DupKeyAsWarning {
return err
}
// TODO: should not filter all types of errors here.
e.handleWarning(err)
return nil
}

// evalRow evaluates a to-be-inserted row. The value of the column may base on another column,
Expand Down Expand Up @@ -346,7 +359,7 @@ func (e *InsertValues) setValueForRefColumn(row []types.Datum, hasValue []bool)
} else if table.ErrNoDefaultValue.Equal(err) {
row[i] = table.GetZeroValue(c.ToInfo())
hasValue[c.Offset] = false
} else if e.filterErr(err) != nil {
} else if e.handleErr(c, &d, 0, err) != nil {
return err
}
}
Expand Down Expand Up @@ -426,7 +439,7 @@ func (e *InsertValues) getRow(ctx context.Context, vals []types.Datum) ([]types.
hasValue := make([]bool, len(e.Table.Cols()))
for i, v := range vals {
casted, err := table.CastValue(e.ctx, v, e.insertColumns[i].ToInfo())
if e.filterErr(err) != nil {
if e.handleErr(nil, &v, 0, err) != nil {
return nil, err
}

Expand All @@ -442,7 +455,7 @@ func (e *InsertValues) getRowInPlace(ctx context.Context, vals []types.Datum, ro
hasValue := make([]bool, len(e.Table.Cols()))
for i, v := range vals {
casted, err := table.CastValue(e.ctx, v, e.insertColumns[i].ToInfo())
if e.filterErr(err) != nil {
if e.handleErr(nil, &v, 0, err) != nil {
return nil, err
}
offset := e.insertColumns[i].Offset
Expand All @@ -452,18 +465,6 @@ func (e *InsertValues) getRowInPlace(ctx context.Context, vals []types.Datum, ro
return e.fillRow(ctx, rowBuf, hasValue)
}

func (e *InsertValues) filterErr(err error) error {
if err == nil {
return nil
}
if !e.ctx.GetSessionVars().StmtCtx.DupKeyAsWarning {
return err
}
// TODO: should not filter all types of errors here.
e.handleWarning(err)
return nil
}

// getColDefaultValue gets the column default value.
func (e *InsertValues) getColDefaultValue(idx int, col *table.Column) (d types.Datum, err error) {
if e.colDefaultVals != nil && e.colDefaultVals[idx].valid {
Expand Down Expand Up @@ -502,7 +503,7 @@ func (e *InsertValues) fillColValue(ctx context.Context, datum types.Datum, idx
}
if !hasValue {
d, err := e.getColDefaultValue(idx, column)
if e.filterErr(err) != nil {
if e.handleErr(column, &datum, 0, err) != nil {
return types.Datum{}, err
}
return d, nil
Expand Down Expand Up @@ -538,7 +539,7 @@ func (e *InsertValues) fillRow(ctx context.Context, row []types.Datum, hasValue
for i, gCol := range gCols {
colIdx := gCol.ColumnInfo.Offset
val, err := e.GenExprs[i].Eval(chunk.MutRowFromDatums(row).ToRow())
if e.filterErr(err) != nil {
if e.handleErr(gCol, &val, 0, err) != nil {
return nil, err
}
row[colIdx], err = table.CastValue(e.ctx, val, gCol.ToInfo())
Expand Down Expand Up @@ -670,7 +671,7 @@ func (e *InsertValues) lazyAdjustAutoIncrementDatum(ctx context.Context, rows []
// Alloc batch N consecutive (min, max] autoIDs.
// max value can be derived from adding one for cnt times.
min, _, err := table.AllocBatchAutoIncrementValue(ctx, e.Table, e.ctx, cnt)
if e.filterErr(err) != nil {
if e.handleErr(col, &autoDatum, cnt, err) != nil {
return nil, err
}
// It's compatible with mysql setting the first allocated autoID to lastInsertID.
Expand Down Expand Up @@ -747,7 +748,7 @@ func (e *InsertValues) adjustAutoIncrementDatum(ctx context.Context, d types.Dat
// Change value 0 to auto id, if NoAutoValueOnZero SQL mode is not set.
if d.IsNull() || e.ctx.GetSessionVars().SQLMode&mysql.ModeNoAutoValueOnZero == 0 {
recordID, err = table.AllocAutoIncrementValue(ctx, e.Table, e.ctx)
if e.filterErr(err) != nil {
if e.handleErr(c, &d, 0, err) != nil {
return types.Datum{}, err
}
// It's compatible with mysql setting the first allocated autoID to lastInsertID.
Expand Down
4 changes: 2 additions & 2 deletions executor/show_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ func (s *testSuite5) TestShowWarnings(c *C) {
tk.MustExec("set @@sql_mode=''")
tk.MustExec("insert show_warnings values ('a')")
c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(1))
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1265|Data Truncated"))
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect FLOAT value: 'a'"))
Copy link
Contributor

Choose a reason for hiding this comment

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

In MySQL, I test this case, but it result is as follows:

mysql> create table if not exists show_warnings (a int);
Query OK, 0 rows affected (0.06 sec)

mysql> set @@sql_mode='';
Query OK, 0 rows affected (0.01 sec)

mysql> insert show_warnings values ('a');
Query OK, 1 row affected, 1 warning (0.01 sec)

mysql> show warnings;
+---------+------+------------------------------------------------------+
| Level   | Code | Message                                              |
+---------+------+------------------------------------------------------+
| Warning | 1366 | Incorrect integer value: 'a' for column 'a' at row 1 |
+---------+------+------------------------------------------------------+
1 row in set (0.00 sec)

Do we need to update this error code?

Copy link
Member Author

@jackysp jackysp Nov 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It is difficult for us to be completely consistent with MySQL in terms of error codes and error messages. What I want to do is to keep us self-consistent, the unique error code + reasonable error message. I think it can help users search the MySQL manual to find the problem if we use the code which MySQL also has (maybe not the same in the same scenes). If want to be completely consistent, I don't feel I can do it. :)

c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(0))
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1265|Data Truncated"))
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect FLOAT value: 'a'"))
c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(0))

// Test Warning level 'Error'
Expand Down
18 changes: 9 additions & 9 deletions executor/write_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (s *testSuite4) TestInsert(c *C) {
tk.MustExec("CREATE TABLE t(a DECIMAL(4,2));")
tk.MustExec("INSERT INTO t VALUES (1.000001);")
r = tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows("Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect DECIMAL value: '1.000001'"))
tk.MustExec("INSERT INTO t VALUES (1.000000);")
r = tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows())
Expand Down Expand Up @@ -268,7 +268,7 @@ func (s *testSuite4) TestInsert(c *C) {
tk.MustExec("insert into t value(20070219173709.055870), (20070219173709.055), (20070219173709.055870123)")
tk.MustQuery("select * from t").Check(testkit.Rows("17:37:09.055870", "17:37:09.055000", "17:37:09.055870"))
_, err = tk.Exec("insert into t value(-20070219173709.055870)")
c.Assert(err.Error(), Equals, "[types:1292]Incorrect time value: '-20070219173709.055870'")
c.Assert(err.Error(), Equals, "[types:1525]Incorrect time value: '-20070219173709.055870'")

tk.MustExec("drop table if exists t")
tk.MustExec("set @@sql_mode=''")
Expand Down Expand Up @@ -496,13 +496,13 @@ func (s *testSuite4) TestInsertIgnore(c *C) {
c.Assert(err, IsNil)
tk.CheckLastMessage("Records: 1 Duplicates: 0 Warnings: 1")
r = tk.MustQuery("SHOW WARNINGS")
r.Check(testkit.Rows("Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '1a'"))
testSQL = "insert ignore into t values ('1a')"
_, err = tk.Exec(testSQL)
c.Assert(err, IsNil)
tk.CheckLastMessage("")
r = tk.MustQuery("SHOW WARNINGS")
r.Check(testkit.Rows("Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '1a'"))

// for duplicates with warning
testSQL = `drop table if exists t;
Expand Down Expand Up @@ -1390,7 +1390,7 @@ func (s *testSuite8) TestUpdate(c *C) {
_, err = tk.Exec("update ignore t set a = 1 where a = (select '2a')")
c.Assert(err, IsNil)
r = tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows("Warning 1265 Data Truncated", "Warning 1265 Data Truncated", "Warning 1062 Duplicate entry '1' for key 'PRIMARY'"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '2a'", "Warning 1292 Truncated incorrect FLOAT value: '2a'", "Warning 1062 Duplicate entry '1' for key 'PRIMARY'"))

tk.MustExec("update ignore t set a = 42 where a = 2;")
tk.MustQuery("select * from t").Check(testkit.Rows("1", "42"))
Expand Down Expand Up @@ -1453,7 +1453,7 @@ func (s *testSuite8) TestUpdate(c *C) {
// A warning rather than data truncated error.
tk.MustExec("update decimals set a = a + 1.23;")
tk.CheckLastMessage("Rows matched: 1 Changed: 1 Warnings: 1")
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1265 Data Truncated"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1292 Truncated incorrect DECIMAL value: '202.23'"))
r = tk.MustQuery("select * from decimals")
r.Check(testkit.Rows("202"))

Expand Down Expand Up @@ -1640,7 +1640,7 @@ func (s *testSuite4) TestPartitionedTableUpdate(c *C) {
_, err = tk.Exec("update ignore t set a = 1 where a = (select '2a')")
c.Assert(err, IsNil)
r = tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows("Warning 1265 Data Truncated", "Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '2a'", "Warning 1292 Truncated incorrect FLOAT value: '2a'"))

// test update ignore for unique key
tk.MustExec("drop table if exists t;")
Expand Down Expand Up @@ -1802,7 +1802,7 @@ func (s *testSuite) TestDelete(c *C) {
c.Assert(err, IsNil)
tk.CheckExecResult(1, 0)
r := tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows("Warning 1265 Data Truncated", "Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '2a'", "Warning 1292 Truncated incorrect FLOAT value: '2a'"))

tk.MustExec(`delete from delete_test ;`)
tk.CheckExecResult(1, 0)
Expand Down Expand Up @@ -1848,7 +1848,7 @@ func (s *testSuite4) TestPartitionedTableDelete(c *C) {
c.Assert(err, IsNil)
tk.CheckExecResult(1, 0)
r := tk.MustQuery("SHOW WARNINGS;")
r.Check(testkit.Rows("Warning 1265 Data Truncated", "Warning 1265 Data Truncated"))
r.Check(testkit.Rows("Warning 1292 Truncated incorrect FLOAT value: '2a'", "Warning 1292 Truncated incorrect FLOAT value: '2a'"))

// Test delete without using index, involve multiple partitions.
tk.MustExec("delete from t ignore index(id) where id >= 13 and id <= 17")
Expand Down
2 changes: 1 addition & 1 deletion expression/builtin_string_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ func (s *testEvaluatorSuite) TestFormat(c *C) {
warnings := s.ctx.GetSessionVars().StmtCtx.GetWarnings()
c.Assert(len(warnings), Equals, tt.warnings, Commentf("test %v", tt))
for i := 0; i < tt.warnings; i++ {
c.Assert(terror.ErrorEqual(types.ErrTruncated, warnings[i].Err), IsTrue, Commentf("test %v", tt))
c.Assert(terror.ErrorEqual(types.ErrTruncatedWrongVal, warnings[i].Err), IsTrue, Commentf("test %v", tt))
}
s.ctx.GetSessionVars().StmtCtx.SetWarnings([]stmtctx.SQLWarn{})
}
Expand Down
Loading