-
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
executor: make the inserting errors more easier to understand (#24044) #26189
Conversation
…pingcap#24044) and a few changes hinted by lint
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/cc @wjhuang2016 @lzmhhh123 @mmyj @wshwsh12 please give my PR a review, thanks |
@@ -1020,7 +1020,7 @@ func (s *testStaleTxnSuite) TestStmtCtxStaleFlag(c *C) { | |||
}, | |||
// assert select statement | |||
{ | |||
sql: fmt.Sprintf("select * from t"), | |||
sql: "select * from t", |
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.
It's duplicate with #26096.
store/copr/coprocessor_test.go
Outdated
@@ -155,7 +155,7 @@ func (s *testCoprocessorSuite) TestBuildTasks(c *C) { | |||
func (s *testCoprocessorSuite) TestSplitRegionRanges(c *C) { | |||
// nil --- 'g' --- 'n' --- 't' --- nil | |||
// <- 0 -> <- 1 -> <- 2 -> <- 3 -> | |||
_, cluster, pdClient, err := testutils.NewMockTiKV("", nil) | |||
_, cluster, pdClient, _ := testutils.NewMockTiKV("", nil) |
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.
ditto
@@ -53,7 +53,7 @@ import ( | |||
type Tracker struct { | |||
mu struct { | |||
sync.Mutex | |||
// The children memory trackers. If the Tracker is the Global Tracker, like executor.GlobalDiskUsageTracker, | |||
// children memory trackers. If the Tracker is the Global Tracker, like executor.GlobalDiskUsageTracker, |
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.
Why delete The
here?
And this fix is uncorrelated with making the inserting errors more easier to understand
, If it's necessary, I think we could be modified it in another PR.
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 goword abort the commit unless modified it. is this change too small to create another PR?
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.
You mean that the goword
checker makes git commit
failed?
I think one PR to do one thing is better, it's no problem to create a small PR to fix a typo or make lint happy. However, fixing many typos in one PR is better. ^_^
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.
ok, thank you for reply
@@ -95,7 +95,6 @@ func InitTracker(t *Tracker, label int, bytesLimit int64, action ActionOnExceed) | |||
t.bytesSoftLimit = int64(float64(bytesLimit) * softScale) | |||
t.maxConsumed = 0 | |||
t.isGlobal = false | |||
return |
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.
ditto
Please add a unit test. |
…pingcap#24044) and a few changes hinted by lint
…pingcap#24044) and a few changes hinted by lint
What problem does this PR solve?
Issue Number: close #24044
Problem Summary: The error messages when inserting errors are not easy to understand
What is changed and how it works?
What's Changed: changed doDupRowUpdate function in insert.go
How it Works: convert the err message in doDupRowUpdate when get column value
Check List
Tests
Release note