-
Notifications
You must be signed in to change notification settings - Fork 188
use restore feature to refine codes releated ddl #54
Conversation
/run-all-tests |
/run-all-tests |
@GregoryIan resolve the conflict. |
I would do it after #50 merged |
@csuzhangxc @amyangfei PTAL |
syncer/ddl_test.go
Outdated
c.Assert(err, IsNil) | ||
|
||
c.Assert(len(sqls), Equals, 1) | ||
getSQL := sqls[0] | ||
c.Assert(getSQL, Equals, tc.expected) | ||
|
||
ast2, err := parser.ParseOneStmt(getSQL, "", "") | ||
c.Assert(err, IsNil) | ||
// FIXME: remove me after implement generated function in restore |
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.
latest parser has fixed this bug, we can remove these comments now
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 useless, because of some very tricky code, I decide remove these code
test.go
Outdated
@@ -0,0 +1,119 @@ | |||
package main |
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.
is this file used for testing only, can remove now?
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.
yep
@@ -128,7 +110,7 @@ func (s *Syncer) skipQuery(tables []*filter.Table, stmt ast.StmtNode, sql string | |||
|
|||
if len(tables) > 0 { | |||
tbs := s.bwList.ApplyOn(tables) | |||
if len(tbs) == 0 { | |||
if len(tbs) != len(tables) { |
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.
suppose we rename a table from name old
to name new
, table old
is in do-tables
and new
is new ignore-tables
.
should we filter this rename DDL?
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.
yep, it's a open problem, we can ignore it or throw an error, it depends its definition
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.
LGTM
@csuzhangxc @july2993 PTAL |
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.
LGTM
What problem does this PR solve?
use restore feature to refine codes releated ddl
What is changed and how it works?
use restore feature to implement rename and split ddl
Check List
Tests