From 8f7fa8c2d8dcade2376fed03f4770302ebd971db Mon Sep 17 00:00:00 2001 From: siddontang Date: Tue, 22 Sep 2015 14:39:30 +0800 Subject: [PATCH 1/5] table: add ColumnOffset to get column offset with name --- table/table.go | 3 +++ table/tables/tables.go | 32 ++++++++++++++++++++++++++++++++ table/tables/tables_test.go | 22 ++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/table/table.go b/table/table.go index 7270c82f904d0..ad8ccd8e3a542 100644 --- a/table/table.go +++ b/table/table.go @@ -107,6 +107,9 @@ type Table interface { // LockRow locks a row. // If update is true, set row lock key to current txn. LockRow(ctx context.Context, h int64, update bool) error + + // ColumnOffset gets the column offset in whole columns with column name. + ColumnOffset(name string) (int, error) } // TableFromMeta builds a table.Table from *model.TableInfo. diff --git a/table/tables/tables.go b/table/tables/tables.go index c714e5a60faa6..584c78ca98d50 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -558,6 +558,38 @@ func (t *Table) AllocAutoID() (int64, error) { return t.alloc.Alloc(t.ID) } +// ColumnOffset implements table.Table ColumnOffset interface. +func (t *Table) ColumnOffset(name string) (int, error) { + seps := strings.Split(name, ".") + var ( + table string + col string + ) + + // Now we only support column or table.column qualified name. + // TODO: support db.table.column later. + if len(seps) == 1 { + col = name + } else if len(seps) == 2 { + table = seps[0] + col = seps[1] + } else { + return -1, errors.Errorf("invalid column format %s", name) + } + + if len(table) > 0 && table != t.Name.O { + return -1, errors.Errorf("column table %s is not equal %s", table, t.Name.O) + } + + for _, c := range t.Columns { + if strings.EqualFold(col, c.Name.L) { + return c.Offset, nil + } + } + + return -1, errors.Errorf("unknown column %s", name) +} + func init() { table.TableFromMeta = TableFromMeta } diff --git a/table/tables/tables_test.go b/table/tables/tables_test.go index 9d97daee03c27..cc837d23391fe 100644 --- a/table/tables/tables_test.go +++ b/table/tables/tables_test.go @@ -102,6 +102,28 @@ func (ts *testSuite) TestBasic(c *C) { c.Assert(cnt, Equals, 0) _, err = ts.se.Execute("drop table test.t") c.Assert(err, IsNil) + + offsetTbl := []struct { + Name string + Offset int + }{ + {"a", 0}, + {"b", 1}, + {"c", -1}, + {"t.a", 0}, + {"t.c", -1}, + {"t1.c", -1}, + {"test.test.t.c", -1}, + } + + for _, t := range offsetTbl { + index, err := tb.ColumnOffset(t.Name) + if t.Offset == -1 { + c.Assert(err, NotNil) + } else { + c.Assert(index, Equals, t.Offset) + } + } } func countEntriesWithPrefix(ctx context.Context, prefix string) (int, error) { From 604d78da452f4c9b9ad33231f1e691a71581f7b6 Mon Sep 17 00:00:00 2001 From: siddontang Date: Tue, 22 Sep 2015 14:40:01 +0800 Subject: [PATCH 2/5] stmts: delete where support qualified name --- stmt/stmts/delete.go | 11 +++++++---- stmt/stmts/delete_test.go | 4 ++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/stmt/stmts/delete.go b/stmt/stmts/delete.go index 1ad7f85dd2604..9bc59ad4f0489 100644 --- a/stmt/stmts/delete.go +++ b/stmt/stmts/delete.go @@ -130,11 +130,14 @@ func (s *DeleteStmt) hitWhere(ctx context.Context, t table.Table, data []interfa return true, nil } m := map[interface{}]interface{}{} - - // Set parameter for evaluating expression. - for _, col := range t.Cols() { - m[col.Name.L] = data[col.Offset] + m[expression.ExprEvalIdentFunc] = func(name string) (interface{}, error) { + offset, err := t.ColumnOffset(name) + if err != nil { + return nil, errors.Trace(err) + } + return data[offset], nil } + ok, err := expression.EvalBoolExpr(ctx, s.Where, m) if err != nil { return false, errors.Trace(err) diff --git a/stmt/stmts/delete_test.go b/stmt/stmts/delete_test.go index 6a9686287de32..e2855eb7e9f61 100644 --- a/stmt/stmts/delete_test.go +++ b/stmt/stmts/delete_test.go @@ -87,6 +87,10 @@ func (s *testStmtSuite) TestDelete(c *C) { r = mustExec(c, s.testDB, `DELETE from test where 0;`) checkResult(c, r, 0, 0) + mustExec(c, s.testDB, "insert into test values (2, 'abc')") + r = mustExec(c, s.testDB, `delete from test where test.id = 2 limit 1`) + checkResult(c, r, 1, 0) + // Select data tx := mustBegin(c, s.testDB) rows, err := tx.Query(s.selectSql) From 71d2cd623298532cc8faac02312ae5b6c473b86a Mon Sep 17 00:00:00 2001 From: siddontang Date: Tue, 22 Sep 2015 14:40:18 +0800 Subject: [PATCH 3/5] kv: cleanup log --- kv/union_iter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kv/union_iter.go b/kv/union_iter.go index 3310de0cbc62f..d592ce19ce97c 100644 --- a/kv/union_iter.go +++ b/kv/union_iter.go @@ -102,7 +102,7 @@ func (iter *UnionIter) updateCur() { } else { // record from dirty comes first if len(iter.dirtyIt.Value()) == 0 { - log.Warnf("delete a record not exists? k = %s", string(iter.dirtyIt.Key())) + log.Warnf("delete a record not exists? k = %q", iter.dirtyIt.Key()) // jump over this deletion iter.dirtyNext() continue From 48deaeb38aad6b513570ca3187fd751482590411 Mon Sep 17 00:00:00 2001 From: siddontang Date: Tue, 22 Sep 2015 17:17:31 +0800 Subject: [PATCH 4/5] stmts: use GetIdentValue to support qualified name --- stmt/stmts/delete.go | 19 +++++++++++-------- stmt/stmts/delete_test.go | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/stmt/stmts/delete.go b/stmt/stmts/delete.go index 9bc59ad4f0489..584b3a8b84985 100644 --- a/stmt/stmts/delete.go +++ b/stmt/stmts/delete.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/tidb/column" "github.com/pingcap/tidb/context" "github.com/pingcap/tidb/expression" + "github.com/pingcap/tidb/field" "github.com/pingcap/tidb/plan" "github.com/pingcap/tidb/plan/plans" "github.com/pingcap/tidb/rset" @@ -125,17 +126,13 @@ func (s *DeleteStmt) planMultiTable(ctx context.Context) (plan.Plan, error) { return r, nil } -func (s *DeleteStmt) hitWhere(ctx context.Context, t table.Table, data []interface{}) (bool, error) { +func (s *DeleteStmt) hitWhere(ctx context.Context, fields []*field.ResultField, data []interface{}) (bool, error) { if s.Where == nil { return true, nil } m := map[interface{}]interface{}{} m[expression.ExprEvalIdentFunc] = func(name string) (interface{}, error) { - offset, err := t.ColumnOffset(name) - if err != nil { - return nil, errors.Trace(err) - } - return data[offset], nil + return plans.GetIdentValue(name, fields, data, field.DefaultFieldFlag) } ok, err := expression.EvalBoolExpr(ctx, s.Where, m) @@ -189,7 +186,7 @@ func (s *DeleteStmt) tryDeleteUsingIndex(ctx context.Context, t table.Table) (bo return false, err } log.Infof("try delete with index id:%d, ctx:%s", handle, ctx) - ok, err := s.hitWhere(ctx, t, data) + ok, err := s.hitWhere(ctx, p.GetFields(), data) if err != nil { return false, errors.Trace(err) } @@ -207,6 +204,7 @@ func (s *DeleteStmt) tryDeleteUsingIndex(ctx context.Context, t table.Table) (bo // Exec implements the stmt.Statement Exec interface. func (s *DeleteStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { + // TODO: unify single from and multi from together. if s.MultiTable { return s.execMultiTable(ctx) } @@ -225,12 +223,14 @@ func (s *DeleteStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { return nil, nil } + fields := field.ColsToResultFields(t.Cols(), t.TableName().L) + var cnt uint64 err = t.IterRecords(ctx, t.FirstKey(), t.Cols(), func(h int64, data []interface{}, cols []*column.Col) (bool, error) { if s.Limit != nil && cnt >= s.Limit.Count { return false, nil } - ok, err = s.hitWhere(ctx, t, data) + ok, err = s.hitWhere(ctx, fields, data) if err != nil { return false, errors.Trace(err) } @@ -297,6 +297,7 @@ func (s *DeleteStmt) execMultiTable(ctx context.Context) (_ rset.Recordset, err if row == nil { break } + for _, entry := range row.RowKeys { tid := entry.Tbl.TableID() if _, ok := tblIDMap[tid]; !ok { @@ -305,6 +306,7 @@ func (s *DeleteStmt) execMultiTable(ctx context.Context) (_ rset.Recordset, err rowKeyMap[entry.Key] = entry.Tbl } } + for k, t := range rowKeyMap { handle, err := util.DecodeHandleFromRowKey(k) if err != nil { @@ -319,5 +321,6 @@ func (s *DeleteStmt) execMultiTable(ctx context.Context) (_ rset.Recordset, err return nil, errors.Trace(err) } } + return nil, nil } diff --git a/stmt/stmts/delete_test.go b/stmt/stmts/delete_test.go index e2855eb7e9f61..eea4522028b94 100644 --- a/stmt/stmts/delete_test.go +++ b/stmt/stmts/delete_test.go @@ -161,3 +161,43 @@ func (s *testStmtSuite) TestMultiTableDelete(c *C) { c.Assert(cnt, Equals, 3) rows.Close() } + +func (s *testStmtSuite) TestQualifedDelete(c *C) { + mustExec(c, s.testDB, "drop table if exists t1") + mustExec(c, s.testDB, "drop table if exists t2") + mustExec(c, s.testDB, "create table t1 (c1 int, c2 int, index (c1))") + mustExec(c, s.testDB, "create table t2 (c1 int, c2 int)") + mustExec(c, s.testDB, "insert into t1 values (1, 1), (2, 2)") + + // delete with index + r := mustExec(c, s.testDB, "delete from t1 where t1.c1 = 1") + checkResult(c, r, 1, 0) + + // delete with no index + r = mustExec(c, s.testDB, "delete from t1 where t1.c2 = 2") + checkResult(c, r, 1, 0) + + rows, err := s.testDB.Query("select * from t1") + c.Assert(err, IsNil) + cnt := 0 + for rows.Next() { + cnt++ + } + rows.Close() + c.Assert(cnt, Equals, 0) + + _, err = s.testDB.Exec("delete from t1 as a where a.c1 = 1") + c.Assert(err, NotNil) + + mustExec(c, s.testDB, "insert into t1 values (1, 1), (2, 2)") + mustExec(c, s.testDB, "insert into t2 values (2, 1), (3,1)") + + r = mustExec(c, s.testDB, "delete t1, t2 from t1 join t2 where t1.c1 = t2.c2") + checkResult(c, r, 3, 0) + + mustExec(c, s.testDB, "insert into t2 values (2, 1), (3,1)") + r = mustExec(c, s.testDB, "delete a, b from t1 as a join t2 as b where a.c2 = b.c1") + checkResult(c, r, 2, 0) + + mustExec(c, s.testDB, "drop table t1, t2") +} From 09d2502845061fc2ae36cb87979f84f094dfa310 Mon Sep 17 00:00:00 2001 From: siddontang Date: Tue, 22 Sep 2015 17:18:58 +0800 Subject: [PATCH 5/5] table: remove unnecessary interface --- table/table.go | 3 --- table/tables/tables.go | 32 -------------------------------- table/tables/tables_test.go | 22 ---------------------- 3 files changed, 57 deletions(-) diff --git a/table/table.go b/table/table.go index ad8ccd8e3a542..7270c82f904d0 100644 --- a/table/table.go +++ b/table/table.go @@ -107,9 +107,6 @@ type Table interface { // LockRow locks a row. // If update is true, set row lock key to current txn. LockRow(ctx context.Context, h int64, update bool) error - - // ColumnOffset gets the column offset in whole columns with column name. - ColumnOffset(name string) (int, error) } // TableFromMeta builds a table.Table from *model.TableInfo. diff --git a/table/tables/tables.go b/table/tables/tables.go index 584c78ca98d50..c714e5a60faa6 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -558,38 +558,6 @@ func (t *Table) AllocAutoID() (int64, error) { return t.alloc.Alloc(t.ID) } -// ColumnOffset implements table.Table ColumnOffset interface. -func (t *Table) ColumnOffset(name string) (int, error) { - seps := strings.Split(name, ".") - var ( - table string - col string - ) - - // Now we only support column or table.column qualified name. - // TODO: support db.table.column later. - if len(seps) == 1 { - col = name - } else if len(seps) == 2 { - table = seps[0] - col = seps[1] - } else { - return -1, errors.Errorf("invalid column format %s", name) - } - - if len(table) > 0 && table != t.Name.O { - return -1, errors.Errorf("column table %s is not equal %s", table, t.Name.O) - } - - for _, c := range t.Columns { - if strings.EqualFold(col, c.Name.L) { - return c.Offset, nil - } - } - - return -1, errors.Errorf("unknown column %s", name) -} - func init() { table.TableFromMeta = TableFromMeta } diff --git a/table/tables/tables_test.go b/table/tables/tables_test.go index cc837d23391fe..9d97daee03c27 100644 --- a/table/tables/tables_test.go +++ b/table/tables/tables_test.go @@ -102,28 +102,6 @@ func (ts *testSuite) TestBasic(c *C) { c.Assert(cnt, Equals, 0) _, err = ts.se.Execute("drop table test.t") c.Assert(err, IsNil) - - offsetTbl := []struct { - Name string - Offset int - }{ - {"a", 0}, - {"b", 1}, - {"c", -1}, - {"t.a", 0}, - {"t.c", -1}, - {"t1.c", -1}, - {"test.test.t.c", -1}, - } - - for _, t := range offsetTbl { - index, err := tb.ColumnOffset(t.Name) - if t.Offset == -1 { - c.Assert(err, NotNil) - } else { - c.Assert(index, Equals, t.Offset) - } - } } func countEntriesWithPrefix(ctx context.Context, prefix string) (int, error) {