Add ORDER BY to CreateTable for ClickHouse #478
Conversation
There was a problem hiding this comment.
Thank you @julienfr112 -- this is a nice addition. Looking close.
| @@ -857,6 +857,8 @@ pub enum Statement { | |||
| default_charset: Option<String>, | |||
| collation: Option<String>, | |||
| on_commit: Option<OnCommit>, | |||
| clickhouse_order: Option<Vec<Ident>>, | |||
There was a problem hiding this comment.
Can you please change this field name to order by to match the SQL more closely (and perhaps add a comment that it is to support the clickhouse variant)?
| @@ -857,6 +857,8 @@ pub enum Statement { | |||
| default_charset: Option<String>, | |||
| collation: Option<String>, | |||
| on_commit: Option<OnCommit>, | |||
| clickhouse_order: Option<Vec<Ident>>, | |||
| query_after: Option<Box<Query>>, | |||
There was a problem hiding this comment.
Can you please add a comment here that this supports a query at the end of the statement (aka for clickhouse)
|
FYI I plan to release 0.17.0 tomorrow in case you want to work on this PR before then: #488 |
Pull Request Test Coverage Report for Build 2240029283
💛 - Coveralls |
|
Marking this as a draft to show it is waiting some changes so I can easily filter out which PRs need review. Please mark it as "ready for review" when it is next ready. |
|
Closing as stale -- please reopen if you still plan to work on it |
Few edits to allow to add order by to create tables.
docs is here :
https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/mergetree/