sql/*: add support for create index statement #182
Conversation
erizocosmico
force-pushed
the
feature/create-index
branch
from
8b782c5 to
4854afb
Compare
sql/index.go
Outdated
| @@ -40,8 +40,8 @@ type Index interface { | |||
| Database() string | |||
| // Table returns the table name this index belongs to. | |||
| Table() string | |||
| // Expression returns the indexed expression. | |||
| Expression() Expression | |||
| // Expressions returns the indexed expressions. | |||
There was a problem hiding this comment.
Can you add here all the explanation about if there is more than one expression they are Columns and so on?
sql/index.go
Outdated
| @@ -104,8 +104,8 @@ type Mergeable interface { | |||
| type IndexDriver interface { | |||
| // ID returns the unique name of the driver. | |||
| ID() string | |||
| // Create a new index for the given expression and id. | |||
| Create(path, db, id string, expr Expression) (Index, error) | |||
| // Create a new index. | |||
There was a problem hiding this comment.
The same as the previous comment here.
sql/index.go
Outdated
| @@ -120,7 +120,7 @@ type indexKey struct { | |||
|
|
|||
| // IndexRegistry keeps track of all indexes in the engine. | |||
| type IndexRegistry struct { | |||
| root string | |||
| Root string | |||
There was a problem hiding this comment.
Some documentation for this, now that is public?
sql/index.go
Outdated
| r.retainIndex(db, idx.ID()) | ||
| return idx | ||
| if idx.Database() == db { | ||
| for _, e := range idx.Expressions() { |
There was a problem hiding this comment.
All the expressions must match, not only one. So maybe we need to change IndexByExpression(db string, expr Expression) Index to IndexByExpression(db string, expr ...Expression) Index and use exprListsEqual
Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
erizocosmico
force-pushed
the
feature/create-index
branch
from
4854afb to
880fccd
Compare
|
Done @ajnavarro |
|
Don't merge this until the |
Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
|
Now it's fully finished. Can be reviewed again. |
| } | ||
|
|
||
| if singleQuote || doubleQuote { | ||
| switch true { |
There was a problem hiding this comment.
this is smart, if we have a cascade of ifs 👍
when we jump into ignoreNext is it ok to break it and just buf.WriteRune(r) (just asking because sounds like we ignore but write it)
There was a problem hiding this comment.
In this case, what ignoreNext does is ignore the meaning of the next character, but we need everything written, that's why there is no break in there. We need every character written because we delegate the real parsing to sqlparser.
Depends on #179
Closes #171
Missing:
There are a few questions around that in the document, they will be implemented once they are answered.