Doris SQL: add Doris Dialect - #2380
Conversation
fc16163 to
a55690c
Compare
|
2500 lines of a PR are a bit too many, could you fracture this down into smaller reviewable units? |
@LucaCappelletti94 Of course, I would do that later and add more info in issue link #2379 |
ecca2d6 to
bffa3fd
Compare
@LucaCappelletti94 Done. Could you please take a look? |
8f0906f to
7ff24be
Compare
|
@alamb @benesch @LucaCappelletti94 Hi team, any improvement should I have? |
LucaCappelletti94
left a comment
LucaCappelletti94
left a comment
There was a problem hiding this comment.
This is the sort of PR that would really benefit from GitHub stabilizing stacked PR also for forks' branches. Let's hope that happens soon. In the meantime, here are a few notes on this first PR in the chain.
| #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] | ||
| pub struct DorisDialect {} | ||
|
|
||
| impl Dialect for DorisDialect { |
There was a problem hiding this comment.
I believe you are still currently missing:
supports_limit_commaparse_infix(theDIV)supports_group_by_with_modifier
I am not familiar enough with Doris to say anything about the other methods, but I suggest you audit them against the engine.
| } | ||
|
|
||
| fn is_identifier_start(&self, ch: char) -> bool { | ||
| ch.is_ascii_alphabetic() || ch == '_' || !ch.is_ascii() |
There was a problem hiding this comment.
Since these methods at this time are extensively identical and meant to be identical to the MySqlDialect, I suggest they are kept aligned by using the same approach used with redshift and postgres and dispatch the method call to MySqlDialect. When they are strictly distinct, a comment with a documentation link may be desirable.
| } | ||
|
|
||
| #[test] | ||
| fn doris_identifier_and_string_literal_gates() { |
There was a problem hiding this comment.
I am unsure such a test is necessary
|
|
||
| #[test] | ||
| fn parse_doris_strings_and_identifiers() { | ||
| doris().verified_stmt( |
There was a problem hiding this comment.
I believe here you may want to use verified_only_select
|
|
||
| #[test] | ||
| fn doris_and_generic_parse_common_sql_identically() { | ||
| doris_and_generic().verified_stmt("SELECT 1 AS properties FROM t"); |
There was a problem hiding this comment.
I imagine this will be more relevant in a later PR in this series where you add the keyword PROPERTIES?
2 participants
Summary
Adds Doris SQL dialect support to close #2379
This PR introduces
DorisDialectand parser/AST support for common Doris SQL syntax, especially DorisCREATE TABLEclauses, partition/distribution definitions, table properties, Doris column options, and Doris load statements.Changes
Add
DorisDialectAdd Doris
CREATE TABLEsupportENGINE = OLAPDUPLICATE KEY,UNIQUE KEY,AGGREGATE KEYCOMMENTPARTITION BY RANGE/LISTAUTO PARTITIONMAXVALUE/MAX_VALUEDISTRIBUTED BY HASH/RANDOMBUCKETS <n>andBUCKETS AUTOPROPERTIES (...)Add Doris AST structures
Add Doris-specific statement parsing
LOAD DATA INFILECREATE ROUTINE LOADExtend tests
Testing
cargo fmt --check cargo test --test sqlparser_doris