planner, executor: fix error code #13733
Conversation
ghost
requested review from
wshwsh12 and
XuHuaiyu
and removed request for
a team
|
/run-all-tests |
planner/core/errors.go
Outdated
| codeWindowRowsIntervalUse = mysql.ErrWindowRowsIntervalUse | ||
| codeWindowFunctionIgnoresFrame = mysql.ErrWindowFunctionIgnoresFrame | ||
| codeUnsupportedOnGeneratedColumn = mysql.ErrUnsupportedOnGeneratedColumn | ||
| codeUnsupportedType terror.ErrCode = mysql.ErrUnsupportedType |
There was a problem hiding this comment.
Sorry I didn't get what you mean? Do you mean we want to totally get ride of codeXXX and just use mysql.ErrXX?
planner/core/errors.go
Outdated
| ErrCartesianProductUnsupported = terror.ClassOptimizer.New(codeUnsupported, mysql.MySQLErrName[mysql.ErrCartesianProductUnsupported]) | ||
| ErrStmtNotFound = terror.ClassOptimizer.New(codeStmtNotFound, mysql.MySQLErrName[mysql.ErrPreparedStmtNotFound]) | ||
| ErrWrongParamCount = terror.ClassOptimizer.New(codeWrongParamCount, mysql.MySQLErrName[mysql.ErrWrongParamCount]) | ||
| ErrSchemaChanged = terror.ClassOptimizer.New(codeSchemaChanged, mysql.MySQLErrName[mysql.ErrSchemaChanged]) | ||
| ErrTablenameNotAllowedHere = terror.ClassOptimizer.New(codeTablenameNotAllowedHere, "Table '%s' from one of the %ss cannot be used in %s") |
There was a problem hiding this comment.
We should use the predefined error message.
There was a problem hiding this comment.
This one is a little different, it has a predefined error message in mysql/errname.go for ErrTablenameNotAllowedHere, but it is different from the native error message.
ErrTablenameNotAllowedHere: "Table '%-.192s' from one of the SELECTs cannot be used in %-.32s",
One work around is to then create another error code for this error, say ErrTablenameNotAllowedHere2 if we want every error message to be strictly defined in mysql/errname.go.
There was a problem hiding this comment.
I think we could modify the error message in mysql/errname.go
planner/core/errors.go
Outdated
| @@ -147,6 +146,12 @@ var ( | |||
|
|
|||
| func init() { | |||
| mysqlErrCodeMap := map[terror.ErrCode]uint16{ | |||
| codeUnsupportedType: mysql.ErrUnsupportedType, | |||
There was a problem hiding this comment.
Please add some tests for this map.
There was a problem hiding this comment.
Could you give some hint on how to add tests for this map?
There was a problem hiding this comment.
Codecov Report
@@ Coverage Diff @@
## master #13733 +/- ##
================================================
+ Coverage 80.3765% 80.3859% +0.0094%
================================================
Files 474 474
Lines 118333 117941 -392
================================================
- Hits 95112 94808 -304
+ Misses 15804 15706 -98
- Partials 7417 7427 +10 |
|
@jackysp PTAL again. |
|
LGTM, please fix the build and test. |
|
Will do, pingcap/parser#650 merged. |
|
/run-common-test |
3 similar comments
|
/run-common-test |
|
/run-common-test |
|
/run-common-test |
|
/run-all-tests |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
|
/run-all-tests tidb-test=pr/961 |
|
/run-common-test tidb-test=pr/961 |
1 similar comment
|
/run-common-test tidb-test=pr/961 |
|
/run-all-tests tidb-test=pr/961 |
|
@jackysp PR https://github.com/pingcap/tidb-test/pull/961 fixes common-test, both PR are ready for merge, PTAL. |
zz-jason
added
status/LGT2
zz-jason
added
status/all tests passed
and removed
status/can-merge
ghost
requested review from
francis0407 and
winoros
and removed request for
a team
What problem does this PR solve?
Associating internal error code for planner/core/errors.go, expression/errors.go and executor/errors.go to parser/mysql/errcode.go.
What is changed and how it works?
Adding more error code and set up the mapping.
Check List
Tests
Code changes
Side effects
Related changes
Release note