-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ftr: create index #254
Ftr: create index #254
Conversation
Codecov Report
@@ Coverage Diff @@
## master #254 +/- ##
==========================================
- Coverage 37.43% 37.28% -0.16%
==========================================
Files 95 96 +1
Lines 13897 13954 +57
==========================================
Hits 5203 5203
- Misses 8044 8101 +57
Partials 650 650
Continue to review full report at Codecov.
|
pkg/runtime/ast/create_index.go
Outdated
} | ||
|
||
func (c *CreateIndexStatement) Restore(flag RestoreFlag, sb *strings.Builder, args *[]int) error { | ||
sb.WriteString("CREATE INDEX") |
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.
这里 INDEX 后面不加个 空格吗?
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.
这里 INDEX 后面不加个 空格吗?
ok~忘记加了
ref: #167 |
} | ||
|
||
func (c *CreateIndexStatement) Restore(flag RestoreFlag, sb *strings.Builder, args *[]int) error { | ||
sb.WriteString("CREATE INDEX ") |
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.
create index
has other grammar. This pr only implement the simple grammar?
https://dev.mysql.com/doc/refman/8.0/en/create-index.html
CREATE [UNIQUE | FULLTEXT | SPATIAL] INDEX index_name
[index_type]
ON tbl_name (key_part,...)
[index_option]
[algorithm_option | lock_option] ...
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.
yes this pr just implements the basic syntax, I'm not sure if other grammars are often used
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.
lock_option
I suggest we are as consistent as possible with MySQL.
If some grammar is difficult and not common, we are not implemented temporarily.
But some grammars are important, e.g: UNIQUE
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.
ok~
pkg/runtime/optimize/optimizer.go
Outdated
shard, err := o.computeShards(ru, stmt.Table, nil, args) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if len(shard) == 0 { | ||
return plan.Transparent(stmt, args), nil | ||
} |
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.
I think this is not the correct way to get shards.
- check whether the
stmt.Table
is a logical table, bypass if not. - enumerate all shards from the logical table topology, see the example code:
// init shards
shards := rule.DatabaseTables{}
// compute all tables
topology := vt.Topology()
topology.Each(func(dbIdx, tbIdx int) bool {
if d, t, ok := topology.Render(dbIdx, tbIdx); ok {
shards[d] = append(shards[d], t)
}
return true
})
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.
Hello, may I ask you a question, how to check whether the stmt.Table is a logical table ?
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.
See the snippet below:
var ru *rule.Rule // prepare your rule
_, ok := ru.VTable("your_table_name")
// target is logical table if ok is true.
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.
ok~thx
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.
pls fix
* feat: create index * fix: add space behind create index restore * fix: create index add index part spec and tests * fix: remove unnecessary create index tests * fix: add create index logic table check
create index statament