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

executor: make the inserting errors more easier to understand (#24044) #26189

Closed
wants to merge 9 commits into from
7 changes: 4 additions & 3 deletions executor/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,13 @@ func (e *InsertExec) doDupRowUpdate(ctx context.Context, handle kv.Handle, oldRo
// Update old row when the key is duplicated.
e.evalBuffer4Dup.SetDatums(e.row4Update...)
for _, col := range cols {
val, err1 := col.Expr.Eval(e.evalBuffer4Dup.ToRow())
if err1 != nil {
row := e.evalBuffer4Dup.ToRow()
val, err1 := col.Expr.Eval(row)
if err1 = e.handleErr(e.Table.WritableCols()[col.Col.Index], &val, row.Idx(), err1); err1 != nil {
return err1
}
e.row4Update[col.Col.Index], err1 = table.CastValue(e.ctx, val, col.Col.ToInfo(), false, false)
if err1 != nil {
if err1 = e.handleErr(e.Table.WritableCols()[col.Col.Index], &val, row.Idx(), err1); err1 != nil {
return err1
}
e.evalBuffer4Dup.SetDatum(col.Col.Index, e.row4Update[col.Col.Index])
Expand Down
10 changes: 5 additions & 5 deletions executor/stale_txn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (s *testStaleTxnSerialSuite) TestSelectAsOf(c *C) {
sql string
expectPhysicalTS int64
preSec int64
// IsStaleness is auto cleanup in select stmt.
// errorStr is auto cleanup in select stmt.
errorStr string
}{
{
Expand Down Expand Up @@ -1020,7 +1020,7 @@ func (s *testStaleTxnSuite) TestStmtCtxStaleFlag(c *C) {
},
// assert select statement
{
sql: fmt.Sprintf("select * from t"),
sql: "select * from t",
Copy link
Member

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.

hasStaleFlag: false,
},
// assert select statement in stale transaction
Expand All @@ -1029,7 +1029,7 @@ func (s *testStaleTxnSuite) TestStmtCtxStaleFlag(c *C) {
hasStaleFlag: false,
},
{
sql: fmt.Sprintf("select * from t"),
sql: "select * from t",
hasStaleFlag: true,
},
{
Expand All @@ -1042,12 +1042,12 @@ func (s *testStaleTxnSuite) TestStmtCtxStaleFlag(c *C) {
hasStaleFlag: false,
},
{
sql: fmt.Sprintf("select * from t"),
sql: "select * from t",
hasStaleFlag: true,
},
// assert select statement after consumed set transaction
{
sql: fmt.Sprintf("select * from t"),
sql: "select * from t",
hasStaleFlag: false,
},
// assert prepare statement with select as of statement
Expand Down
2 changes: 1 addition & 1 deletion store/copr/coprocessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

testutils.BootstrapWithMultiRegions(cluster, []byte("g"), []byte("n"), []byte("t"))
pdCli := &tikv.CodecPDClient{Client: pdClient}
cache := NewRegionCache(tikv.NewRegionCache(pdCli))
Expand Down
3 changes: 1 addition & 2 deletions util/memory/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

@mmyj mmyj Jul 13, 2021

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. ^_^

Copy link
Contributor Author

@zoomxi zoomxi Jul 13, 2021

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

// we wouldn't maintain its children in order to avoiding mutex contention.
children map[int][]*Tracker
}
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

ditto

}

// NewTracker creates a memory tracker.
Expand Down