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

execution: fix different error code from MySQL when inserting incorrect bigint value #19854

Closed

Conversation

TszKitLo40
Copy link
Contributor

@TszKitLo40 TszKitLo40 commented Sep 8, 2020

What problem does this PR solve?

Issue Number: close #17832

Problem Summary:
fix different error code from MySQL when inserting incorrect bigint value

What's Changed:
Return types.ErrTruncated when the data is truncated in the cast value function.

Check List

Tests

  • Manual test (Same steps in the issue)

Release note

  • fix different error code from MySQL when inserting incorrect bigint value

@ti-srebot ti-srebot added the contribution This PR is from a community contributor. label Sep 8, 2020
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

Please add a unit test for this change.

@TszKitLo40 TszKitLo40 requested a review from a team as a code owner September 8, 2020 11:25
@TszKitLo40 TszKitLo40 requested review from wshwsh12 and removed request for a team September 8, 2020 11:25
@github-actions github-actions bot added the sig/execution SIG execution label Sep 8, 2020
@TszKitLo40 TszKitLo40 force-pushed the incorrect-error-code-bigint-value branch from 2e8351e to 191a8b2 Compare September 8, 2020 11:35
@TszKitLo40
Copy link
Contributor Author

TszKitLo40 commented Sep 9, 2020

My commit failed to pass the tests. I have no idea how to know whether the error should be thrown between ErrWarnDataTruncated and ErrTruncatedWrongValueForField. @wjhuang2016 @SunRunAway Can you give me some advice?

@SunRunAway
Copy link
Contributor

The failed test case says,

[2020-09-09T09:27:58.004Z] ----------------------------------------------------------------------

[2020-09-09T09:27:58.004Z] FAIL: executor_test.go:2915: testSuite.TestEmptyEnum

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] executor_test.go:2922:

[2020-09-09T09:27:58.004Z]     c.Assert(terror.ErrorEqual(err, table.ErrTruncatedWrongValueForField), IsTrue, Commentf("err %v", err))

[2020-09-09T09:27:58.004Z] ... obtained bool = false

[2020-09-09T09:27:58.004Z] ... err [table:1265]Data truncated for column 'e' at row 1

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] ----------------------------------------------------------------------

Could you help fix it? @TszKitLo40

@TszKitLo40
Copy link
Contributor Author

The failed test case says,

[2020-09-09T09:27:58.004Z] ----------------------------------------------------------------------

[2020-09-09T09:27:58.004Z] FAIL: executor_test.go:2915: testSuite.TestEmptyEnum

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] executor_test.go:2922:

[2020-09-09T09:27:58.004Z]     c.Assert(terror.ErrorEqual(err, table.ErrTruncatedWrongValueForField), IsTrue, Commentf("err %v", err))

[2020-09-09T09:27:58.004Z] ... obtained bool = false

[2020-09-09T09:27:58.004Z] ... err [table:1265]Data truncated for column 'e' at row 1

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] 

[2020-09-09T09:27:58.004Z] ----------------------------------------------------------------------

Could you help fix it? @TszKitLo40

I am not sure whether my implementation is correct. What do you think of my implementation?

Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

Please resolve conflict.

@TszKitLo40
Copy link
Contributor Author

Please resolve conflict.

I have resolved it.

@TszKitLo40
Copy link
Contributor Author

I found that the bug is that '2 ^ 31 - 1' would be truncated to 2. ErrTruncatedWrongVal error would be thrown and I change it to ErrorTruncated. I am not sure whether my idea is correct.

@@ -177,7 +177,7 @@ func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo, r
if err1 != nil {
logutil.BgLogger().Warn("Datum ToString failed", zap.Stringer("Datum", val), zap.Error(err1))
}
err = sc.HandleTruncate(types.ErrTruncatedWrongVal.GenWithStackByArgs(col.FieldType.CompactStr(), str))
err = sc.HandleTruncate(types.ErrTruncated.GenWithStackByArgs(col.FieldType.CompactStr(), str))
Copy link
Member

Choose a reason for hiding this comment

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

Modifying the code here will cause upper-level misjudgment errors and cause other problems. We should find other ways to resolved this issue.

@ichn-hu ichn-hu mentioned this pull request Nov 3, 2020
@zz-jason
Copy link
Member

zz-jason commented Feb 9, 2021

I'm going to close this PR since it hasn't been updated for a long time. feel free to reopen if you want to continue with it. thank you for your contribution.

@zz-jason zz-jason closed this Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenge-program component/expression contribution This PR is from a community contributor. sig/execution SIG execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Different error code from MySQL when inserting incorrect bigint value
8 participants