使用sqlparser替换正则进行queryevent的解析 #382
Conversation
|
Great, PTAL @GregoryIan |
| } | ||
| ns = append(ns, n) | ||
| } | ||
| case *ast.AlterTableStmt: |
There was a problem hiding this comment.
still AlterTableStmt problem, AlterTableStmt contains field Specs and AlterTableRenameTable spec stores table name in its NewTable field, you know? you should fetch new table name from it I think
There was a problem hiding this comment.
i think what we need is just clear old table cache. for new table, it will be fetched when table cache miss. i think it's unneccessary to get new table name here.
There was a problem hiding this comment.
It is, but it's also relay on definition of OnTableChanged, would user want to receive new table like create table, rename table? we should make it clear @siddontang
|
Very good code refine, there are still a few problems, but I believe we will be able to solve it soon @lintanghui |
| } | ||
| for _, st := range stmts { | ||
| nodes := parseStmt(st) | ||
| if len(nodes) == 0 { |
There was a problem hiding this comment.
if len(nodes) != 1, then fatal? similar with others.
There was a problem hiding this comment.
one stmt may contains multi tables, eg: rename a to b,c to d; then nodes will contain two nodes a and c.
There was a problem hiding this comment.
yep, I think in these cases, we must assert the number of the nodes is what we want.
There was a problem hiding this comment.
what we need to do is to range nodes and then clear old table cache, rather than care about the number if nodes. if len(nodes)<0 range it, if len(nodes)==0 do nothing.
There was a problem hiding this comment.
In test cases, we must ensure parseStmt works correctly.
There was a problem hiding this comment.
yep, some test had been writed to test parseStmt
There was a problem hiding this comment.
some test had been writed to test
parseStmt
so, test the length of return nodes too?
|
PTAL @siddontang |
| github.com/pingcap/errors v0.11.0 h1:DCJQB8jrHbQ1VVlMFIrbj2ApScNNotVmkSNplu2yUt4= | ||
| github.com/pingcap/errors v0.11.0/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= | ||
| github.com/pingcap/parser v0.0.0-20190506092653-e336082eb825 h1:U9Kdnknj4n2v76Mg7wazevZ5N9U1OIaMwSNRVLEcLX0= | ||
| github.com/pingcap/parser v0.0.0-20190506092653-e336082eb825/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA= | ||
| github.com/pingcap/tipb v0.0.0-20190428032612-535e1abaa330 h1:rRMLMjIMFulCX9sGKZ1hoov/iROMsKyC8Snc02nSukw= |
There was a problem hiding this comment.
em, why do we use tipb here?
| github.com/pingcap/errors v0.11.0 | ||
| github.com/pingcap/parser v0.0.0-20190506092653-e336082eb825 | ||
| github.com/pingcap/tipb v0.0.0-20190428032612-535e1abaa330 // indirect |
#381
某些case下的queryevent正则无法正常解析到表结构变更无法刷新table缓存导致binlog格式解析失败
例如:使用ghost时的rename语句为:
rename /* gh-ost */ tablepercona.order_basictopercona._order_basic_del,percona._order_basic_ghotopercona.order_basic当ddl存在注释或同时操作多个table的时候正则匹配到对应到表变更