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

*: incorrect error message of inserting enum & set (#19380) #19950

Merged
merged 4 commits into from
Sep 21, 2020

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Sep 11, 2020

cherry-pick #19380 to release-4.0


What problem does this PR solve?

Issue Number: close #19229

Problem Summary:

What is changed and how it works?

What's Changed: Change error code and error info when insert value.

Check List

Tests

  • Unit test
  • Integration test

Release note

  • fix incorrect error message of inserting enum & set

@ti-srebot
Copy link
Contributor Author

/run-all-tests

table/column.go Outdated
@@ -181,7 +181,11 @@ func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo, r
if types.ErrOverflow.Equal(err) && returnOverflow {
return casted, err
}
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the conflict @Patrick0308 .

Copy link
Contributor

Choose a reason for hiding this comment

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

Do I need create a new pr to fix this problem?

Copy link
Contributor

@Patrick0308 Patrick0308 Sep 14, 2020

Choose a reason for hiding this comment

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

What should I do? @qw4990

Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

CI failed, you may resolve the conflicts and file a new PR

@Patrick0308
Copy link
Contributor

@XuHuaiyu @qw4990 I fix conflict in #19989.

@XuHuaiyu
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 15, 2020
@XuHuaiyu
Copy link
Contributor

PTAL @qw4990

@@ -181,7 +181,7 @@ func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo, r
if types.ErrOverflow.Equal(err) && returnOverflow {
return casted, err
}
if types.ErrTruncated.Equal(err) {
if err != nil && types.ErrTruncated.Equal(err) && col.Tp != mysql.TypeSet && col.Tp != mysql.TypeEnum {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need err!=nil because in types.ErrTruncated.Equal(err) fix nil problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. challenge-program difficulty/easy and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 17, 2020
@ti-srebot ti-srebot removed the status/LGT2 Indicates that a PR has LGTM 2. label Sep 21, 2020
@ti-srebot ti-srebot added the status/LGT3 The PR has already had 3 LGTM. label Sep 21, 2020
@bb7133
Copy link
Member

bb7133 commented Sep 21, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 21, 2020
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 20076
  • 20074
  • 20062
  • 20020
  • 19972

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 25a6671 into pingcap:release-4.0 Sep 21, 2020
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 sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants