From ad36c4d8bf2211ab7ab4dfc73ff5c23b6c8a7045 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Mon, 6 May 2019 20:49:56 +0800 Subject: [PATCH 1/5] get row should handle virtual columns Signed-off-by: Shuaipeng Yu --- executor/batch_checker.go | 17 ++++++++++++++++- executor/insert.go | 2 +- executor/replace.go | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/executor/batch_checker.go b/executor/batch_checker.go index 3496a306664a6..11260aa8217e2 100644 --- a/executor/batch_checker.go +++ b/executor/batch_checker.go @@ -16,12 +16,14 @@ package executor import ( "github.com/pingcap/errors" "github.com/pingcap/parser/model" + "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/chunk" ) type keyValue struct { @@ -284,7 +286,8 @@ func (b *batchChecker) deleteDupKeys(ctx sessionctx.Context, t table.Table, rows // getOldRow gets the table record row from storage for batch check. // t could be a normal table or a partition, but it must not be a PartitionedTable. -func (b *batchChecker) getOldRow(ctx sessionctx.Context, t table.Table, handle int64) ([]types.Datum, error) { +func (b *batchChecker) getOldRow(ctx sessionctx.Context, t table.Table, handle int64, + genExprs []expression.Expression) ([]types.Datum, error) { oldValue, ok := b.dupOldRowValues[string(t.RecordKey(handle))] if !ok { return nil, errors.NotFoundf("can not be duplicated row, due to old row not found. handle %d", handle) @@ -295,6 +298,7 @@ func (b *batchChecker) getOldRow(ctx sessionctx.Context, t table.Table, handle i return nil, err } // Fill write-only and write-reorg columns with originDefaultValue if not found in oldValue. + gIdx := 0 for _, col := range cols { if col.State != model.StatePublic && oldRow[col.Offset].IsNull() { _, found := oldRowMap[col.ID] @@ -305,6 +309,17 @@ func (b *batchChecker) getOldRow(ctx sessionctx.Context, t table.Table, handle i } } } + if col.IsGenerated() { + val, err := genExprs[gIdx].Eval(chunk.MutRowFromDatums(oldRow).ToRow()) + if err != nil { + return nil, err + } + oldRow[col.Offset], err = table.CastValue(ctx, val, col.ToInfo()) + if err != nil { + return nil, err + } + gIdx++ + } } return oldRow, nil } diff --git a/executor/insert.go b/executor/insert.go index fc43828c25550..e65681157d2ec 100644 --- a/executor/insert.go +++ b/executor/insert.go @@ -169,7 +169,7 @@ func (e *InsertExec) Open(ctx context.Context) error { // updateDupRow updates a duplicate row to a new row. func (e *InsertExec) updateDupRow(row toBeCheckedRow, handle int64, onDuplicate []*expression.Assignment) error { - oldRow, err := e.getOldRow(e.ctx, e.Table, handle) + oldRow, err := e.getOldRow(e.ctx, e.Table, handle, e.GenExprs) if err != nil { logutil.Logger(context.Background()).Error("get old row failed when insert on dup", zap.Int64("handle", handle), zap.String("toBeInsertedRow", types.DatumsToStrNoErr(row.row))) return err diff --git a/executor/replace.go b/executor/replace.go index d900744d25dd1..26d3875eddb4e 100644 --- a/executor/replace.go +++ b/executor/replace.go @@ -55,7 +55,7 @@ func (e *ReplaceExec) Open(ctx context.Context) error { // but if the to-be-removed row equals to the to-be-added row, no remove or add things to do. func (e *ReplaceExec) removeRow(handle int64, r toBeCheckedRow) (bool, error) { newRow := r.row - oldRow, err := e.batchChecker.getOldRow(e.ctx, r.t, handle) + oldRow, err := e.batchChecker.getOldRow(e.ctx, r.t, handle, e.GenExprs) if err != nil { logutil.Logger(context.Background()).Error("get old row failed when replace", zap.Int64("handle", handle), zap.String("toBeInsertedRow", types.DatumsToStrNoErr(r.row))) return false, err From 980d7b0720bc6f009eabba90a5ed93ee2c75f7fd Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Mon, 6 May 2019 21:48:04 +0800 Subject: [PATCH 2/5] add a replace test Signed-off-by: Shuaipeng Yu --- executor/write_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/executor/write_test.go b/executor/write_test.go index 1c1ba50866b2a..875444e5f89d6 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -898,6 +898,14 @@ func (s *testSuite4) TestReplace(c *C) { tk.MustExec(`replace into t1 select * from (select 1, 2) as tmp;`) c.Assert(int64(tk.Se.AffectedRows()), Equals, int64(2)) tk.CheckLastMessage("Records: 1 Duplicates: 1 Warnings: 0") + + // Test Replace with generated column + tk.MustExec(`drop table if exists t1;`) + tk.MustExec("create table t1(id int, id_gen int as(`id` + 42), b int, unique key id_gen(`id_gen`));") + tk.MustExec(`insert into t1 (id, b) values(1,1),(2,2),(3,3),(4,4),(5,5);`) + tk.MustExec(`replace into t1 (id, b) values(1,1);`) + tk.MustExec(`replace into t1 (id, b) values(1,1),(2,2);`) + tk.MustExec(`replace into t1 (id, b) values(6,16),(7,17),(8,18);`) } func (s *testSuite4) TestPartitionedTableReplace(c *C) { From 6aed1a92a0b794f499347a3731e80086159ce2ef Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Tue, 7 May 2019 14:10:20 +0800 Subject: [PATCH 3/5] add test case for insert on dup Signed-off-by: Shuaipeng Yu --- executor/write_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/executor/write_test.go b/executor/write_test.go index 875444e5f89d6..26124f5a6d313 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -898,14 +898,24 @@ func (s *testSuite4) TestReplace(c *C) { tk.MustExec(`replace into t1 select * from (select 1, 2) as tmp;`) c.Assert(int64(tk.Se.AffectedRows()), Equals, int64(2)) tk.CheckLastMessage("Records: 1 Duplicates: 1 Warnings: 0") +} +func (s *testSuite2) TestVirtualColumn(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") // Test Replace with generated column tk.MustExec(`drop table if exists t1;`) - tk.MustExec("create table t1(id int, id_gen int as(`id` + 42), b int, unique key id_gen(`id_gen`));") + tk.MustExec(`create table t1(id int, id_gen int as(id + 42), b int, unique key id_gen(id_gen));`) tk.MustExec(`insert into t1 (id, b) values(1,1),(2,2),(3,3),(4,4),(5,5);`) tk.MustExec(`replace into t1 (id, b) values(1,1);`) tk.MustExec(`replace into t1 (id, b) values(1,1),(2,2);`) tk.MustExec(`replace into t1 (id, b) values(6,16),(7,17),(8,18);`) + tk.MustQuery("select * from t1;").Check(testkit.Rows( + "1 43 1", "2 44 2", "3 45 3", "4 46 4", "5 47 5", "6 48 16", "7 49 17", "8 50 18")) + tk.MustExec(`insert into t1 (id, b) values (6,18) on duplicate key update id = -id;`) + tk.MustExec(`insert into t1 (id, b) values (7,28) on duplicate key update b = -values(b);`) + tk.MustQuery("select * from t1;").Check(testkit.Rows( + "1 43 1", "2 44 2", "3 45 3", "4 46 4", "5 47 5", "-6 36 16", "7 49 -28", "8 50 18")) } func (s *testSuite4) TestPartitionedTableReplace(c *C) { From 79e42bfe248e512a7705932663dd88e2fdcd5159 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Tue, 7 May 2019 14:11:50 +0800 Subject: [PATCH 4/5] update case name Signed-off-by: Shuaipeng Yu --- executor/write_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/write_test.go b/executor/write_test.go index 26124f5a6d313..c19cbbc5a6da7 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -900,7 +900,7 @@ func (s *testSuite4) TestReplace(c *C) { tk.CheckLastMessage("Records: 1 Duplicates: 1 Warnings: 0") } -func (s *testSuite2) TestVirtualColumn(c *C) { +func (s *testSuite2) TestGeneratedColumnForInsert(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") // Test Replace with generated column From 94205b5dacf41257f90d39ba7886a7e274eb3445 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Tue, 7 May 2019 15:03:58 +0800 Subject: [PATCH 5/5] eval for virtual only Signed-off-by: Shuaipeng Yu --- executor/batch_checker.go | 17 ++++++++++------- executor/write_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/executor/batch_checker.go b/executor/batch_checker.go index 11260aa8217e2..12d2b7c63a834 100644 --- a/executor/batch_checker.go +++ b/executor/batch_checker.go @@ -310,13 +310,16 @@ func (b *batchChecker) getOldRow(ctx sessionctx.Context, t table.Table, handle i } } if col.IsGenerated() { - val, err := genExprs[gIdx].Eval(chunk.MutRowFromDatums(oldRow).ToRow()) - if err != nil { - return nil, err - } - oldRow[col.Offset], err = table.CastValue(ctx, val, col.ToInfo()) - if err != nil { - return nil, err + // only the virtual column needs fill back. + if !col.GeneratedStored { + val, err := genExprs[gIdx].Eval(chunk.MutRowFromDatums(oldRow).ToRow()) + if err != nil { + return nil, err + } + oldRow[col.Offset], err = table.CastValue(ctx, val, col.ToInfo()) + if err != nil { + return nil, err + } } gIdx++ } diff --git a/executor/write_test.go b/executor/write_test.go index c19cbbc5a6da7..798fec9133615 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -903,7 +903,8 @@ func (s *testSuite4) TestReplace(c *C) { func (s *testSuite2) TestGeneratedColumnForInsert(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") - // Test Replace with generated column + + // test cases for default behavior tk.MustExec(`drop table if exists t1;`) tk.MustExec(`create table t1(id int, id_gen int as(id + 42), b int, unique key id_gen(id_gen));`) tk.MustExec(`insert into t1 (id, b) values(1,1),(2,2),(3,3),(4,4),(5,5);`) @@ -916,6 +917,28 @@ func (s *testSuite2) TestGeneratedColumnForInsert(c *C) { tk.MustExec(`insert into t1 (id, b) values (7,28) on duplicate key update b = -values(b);`) tk.MustQuery("select * from t1;").Check(testkit.Rows( "1 43 1", "2 44 2", "3 45 3", "4 46 4", "5 47 5", "-6 36 16", "7 49 -28", "8 50 18")) + + // test cases for virtual and stored columns in the same table + tk.MustExec(`drop table if exists t`) + tk.MustExec(`create table t + (i int as(k+1) stored, j int as(k+2) virtual, k int, unique key idx_i(i), unique key idx_j(j))`) + tk.MustExec(`insert into t (k) values (1), (2)`) + tk.MustExec(`replace into t (k) values (1), (2)`) + tk.MustQuery(`select * from t`).Check(testkit.Rows("2 3 1", "3 4 2")) + + tk.MustExec(`drop table if exists t`) + tk.MustExec(`create table t + (i int as(k+1) stored, j int as(k+2) virtual, k int, unique key idx_j(j))`) + tk.MustExec(`insert into t (k) values (1), (2)`) + tk.MustExec(`replace into t (k) values (1), (2)`) + tk.MustQuery(`select * from t`).Check(testkit.Rows("2 3 1", "3 4 2")) + + tk.MustExec(`drop table if exists t`) + tk.MustExec(`create table t + (i int as(k+1) stored, j int as(k+2) virtual, k int, unique key idx_i(i))`) + tk.MustExec(`insert into t (k) values (1), (2)`) + tk.MustExec(`replace into t (k) values (1), (2)`) + tk.MustQuery(`select * from t`).Check(testkit.Rows("2 3 1", "3 4 2")) } func (s *testSuite4) TestPartitionedTableReplace(c *C) {