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

planner: fix panic when prepare and execute the insert on duplicate #37924

Merged
merged 8 commits into from
Sep 22, 2022
13 changes: 13 additions & 0 deletions executor/insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,19 @@ func TestUpdateDuplicateKey(t *testing.T) {
"[kv:1062]Duplicate entry '1-2-4' for key 'PRIMARY'")
}

func TestIssue37187(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")

tk.MustExec("drop table if exists a, b")
tk.MustExec("create table t1 (a int(11) ,b varchar(100) ,primary key (a));")
tk.MustExec("create table t2 (c int(11) ,d varchar(100) ,primary key (c));")
tk.MustExec("prepare in1 from 'insert into t1 (a,b) select c,null from t2 t on duplicate key update b=t.d';")
err := tk.ExecToErr("execute in1;")
require.NoError(t, err)
}

func TestInsertWrongValueForField(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
1 change: 1 addition & 0 deletions parser/ast/dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -2023,6 +2023,7 @@ type InsertStmt struct {
// TableHints represents the table level Optimizer Hint for join type.
TableHints []*TableOptimizerHint
PartitionNames []model.CIStr
ColLen int
}

// Restore implements Node interface.
Expand Down
11 changes: 7 additions & 4 deletions planner/core/planbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3821,13 +3821,13 @@ func (b *PlanBuilder) buildSelectPlanOfInsert(ctx context.Context, insert *ast.I
if err != nil {
return err
}
actualColLen := -1
actualColLen := insert.ColLen
Copy link
Contributor Author

@Reminiscent Reminiscent Sep 19, 2022

Choose a reason for hiding this comment

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

Because the initialization value for insert.ColLen is 0. And it's impossible for the variable actualColLen's end value be zero(I guess. In code, it will be assigned to actualColLen = selectPlan.Schema().Len()). So we change the actualColLen's initiation value to 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding some comments like see issue #37187 here?

// For MYSQL, it handles the case that the columns in ON DUPLICATE UPDATE is not the project column of the SELECT clause
// but just in the table occurs in the SELECT CLAUSE.
// e.g. insert into a select x from b ON DUPLICATE KEY UPDATE a.x=b.y; the `y` is not a column of select's output.
// MySQL won't throw error and will execute this SQL successfully.
// To make compatible with this strange feature, we add the variable `actualColLen` and the following IF branch.
if len(insert.OnDuplicate) > 0 {
if len(insert.OnDuplicate) > 0 && actualColLen == 0 {
// If the select has aggregation, it cannot see the columns not in the select field.
// e.g. insert into a select x from b ON DUPLICATE KEY UPDATE a.x=b.y; can be executed successfully.
// insert into a select x from b group by x ON DUPLICATE KEY UPDATE a.x=b.y; will report b.y not found.
Expand Down Expand Up @@ -3874,7 +3874,7 @@ func (b *PlanBuilder) buildSelectPlanOfInsert(ctx context.Context, insert *ast.I
}

// Check to guarantee that the length of the row returned by select is equal to that of affectedValuesCols.
if (actualColLen == -1 && selectPlan.Schema().Len() != len(affectedValuesCols)) || (actualColLen != -1 && actualColLen != len(affectedValuesCols)) {
if (actualColLen == 0 && selectPlan.Schema().Len() != len(affectedValuesCols)) || (actualColLen != 0 && actualColLen != len(affectedValuesCols)) {
return ErrWrongValueCountOnRow.GenWithStackByArgs(1)
}

Expand All @@ -3897,9 +3897,12 @@ func (b *PlanBuilder) buildSelectPlanOfInsert(ctx context.Context, insert *ast.I
return err
}

if actualColLen == -1 {
if actualColLen == 0 {
actualColLen = selectPlan.Schema().Len()
}
if insert.ColLen == 0 {
insert.ColLen = actualColLen
}
insertPlan.RowLen = actualColLen
// schema4NewRow is the schema for the newly created data record based on
// the result of the select statement.
Expand Down