loader, syncer: support create/drop view#1310
loader, syncer: support create/drop view#1310lance6716 wants to merge 26 commits intopingcap:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
waiting pingcap/tidb-tools#405 (but I think it's OK to merge this PR before tidb-tools. In days before v2.0.1 release will update dependencies as well) |
| } | ||
| } | ||
|
|
||
| for view := range views { |
There was a problem hiding this comment.
Is it possible to create the view in a different database with the table?
There was a problem hiding this comment.
I guess user may create that type of view. And dumpling will generate a "table file" for each view at view's database, I guess this logic is fine?
| if err != nil { | ||
| return terror.WithScope(err, terror.ScopeInternal) | ||
| } | ||
| } |
There was a problem hiding this comment.
It sames that loader will ignore all SET sqls here. Is this by design?
There was a problem hiding this comment.
oh, query will be added in L1222. We just didn't rename them
There was a problem hiding this comment.
oh, yes. My fault. Maybe you can add a comment before L1191 🤣
| for view := range views { | ||
| viewFile := fmt.Sprintf("%s/%s.%s-schema-view.sql", l.cfg.Dir, db, view) | ||
| l.logger.Info("start to create view", zap.String("view file", viewFile)) | ||
| err := l.restoreView(ctx, dbConn, viewFile, db, view) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| l.logger.Info("finish to create view", zap.String("view file", viewFile)) | ||
| } |
There was a problem hiding this comment.
What if some views needed tables in other databases are not created yet? I think we should create views when all tables in all databases have been created correctly.
|
updated code and PR description, PTAL @lichunzhu @GMHDBJD @csuzhangxc |
1f24a99 to
a91adcb
Compare
|
PTAL @GMHDBJD |
| } | ||
|
|
||
| if s.cfg.ShardMode == "" { | ||
| // VIEW statements are not needed sharding synchronized, so pretend this is no-shard-mode |
There was a problem hiding this comment.
What's the behavior when view statement in resharding sync? such as ddl1 view ddl2. How about add a test for it.
There was a problem hiding this comment.
in pessimistic mode, DDL lock will block sync of VIEW. in other cases VIEW is executed directly in downstream
|
data race recorded in #1351 |
| run_sql_source1 "create view ${shardddl1}.v1 as select * from ${shardddl1}.${tb1};" | ||
| sleep 1 | ||
| run_sql_tidb "show create view ${shardddl}.v" | ||
| check_contains "View: v" |
There was a problem hiding this comment.
Seems view is created here in pessimistic?
There was a problem hiding this comment.
yes. VIEW is pretend to sync in none ShardMode, try to immediately execute to downstream.
There was a problem hiding this comment.
But the table may not be created?
create table <-- not sync
create view <-- sync
create table
create view
|
@lance6716: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/hold |
What problem does this PR solve?
close #1300
What is changed and how it works?
support dumpling's view file in load unit, support sync view as well.
sync of view didn't need a sharding sync lock, that is to say we could directly execute CREATE VIEW IF NOT EXISTS / DROP VIEW IF EXISTS to downstream when any shard send it
Check List
Tests
Code changes
Side effects
Related changes