Skip to content

Commit 32833ce

Browse files
tomershaniiitomershaniii
committed
create table parsing - review feeback
1 parent eff0f62 commit 32833ce

File tree

11 files changed

+78
-98
lines changed

11 files changed

+78
-98
lines changed

‎src/ast/dml.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ pub struct CreateTable {
154154
pub like: Option<ObjectName>,
155155
pub clone: Option<ObjectName>,
156156
// For Hive dialect, the table comment is after the column definitions without `=`,
157-
// so we need to add an extra variant to allow to identify this case when displaying.
157+
// so the `comment` field is optional and different than the comment field in the general options list.
158158
// [Hive](https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-CreateTable)
159-
pub comment_after_column_def: Option<CommentDef>,
159+
pub comment: Option<CommentDef>,
160160
pub on_commit: Option<OnCommit>,
161161
/// ClickHouse "ON CLUSTER" clause:
162162
/// <https://clickhouse.com/docs/en/sql-reference/distributed-ddl/>
@@ -277,7 +277,7 @@ impl Display for CreateTable {
277277

278278
// Hive table comment should be after column definitions, please refer to:
279279
// [Hive](https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL#LanguageManualDDL-CreateTable)
280-
if let Some(comment) = &self.comment_after_column_def {
280+
if let Some(comment) = &self.comment {
281281
write!(f, " COMMENT '{comment}'")?;
282282
}
283283

‎src/ast/helpers/stmt_create_table.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub struct CreateTableBuilder {
8484
pub without_rowid: bool,
8585
pub like: Option<ObjectName>,
8686
pub clone: Option<ObjectName>,
87-
pub comment_after_column_def: Option<CommentDef>,
87+
pub comment: Option<CommentDef>,
8888
pub on_commit: Option<OnCommit>,
8989
pub on_cluster: Option<Ident>,
9090
pub primary_key: Option<Box<Expr>>,
@@ -133,7 +133,7 @@ impl CreateTableBuilder {
133133
without_rowid: false,
134134
like: None,
135135
clone: None,
136-
comment_after_column_def: None,
136+
comment: None,
137137
on_commit: None,
138138
on_cluster: None,
139139
primary_key: None,
@@ -250,7 +250,7 @@ impl CreateTableBuilder {
250250
}
251251

252252
pub fn comment_after_column_def(mut self, comment: Option<CommentDef>) -> Self {
253-
self.comment_after_column_def = comment;
253+
self.comment = comment;
254254
self
255255
}
256256

@@ -404,7 +404,7 @@ impl CreateTableBuilder {
404404
without_rowid: self.without_rowid,
405405
like: self.like,
406406
clone: self.clone,
407-
comment_after_column_def: self.comment_after_column_def,
407+
comment: self.comment,
408408
on_commit: self.on_commit,
409409
on_cluster: self.on_cluster,
410410
primary_key: self.primary_key,
@@ -460,7 +460,7 @@ impl TryFrom<Statement> for CreateTableBuilder {
460460
without_rowid,
461461
like,
462462
clone,
463-
comment_after_column_def: comment,
463+
comment,
464464
on_commit,
465465
on_cluster,
466466
primary_key,
@@ -503,7 +503,7 @@ impl TryFrom<Statement> for CreateTableBuilder {
503503
without_rowid,
504504
like,
505505
clone,
506-
comment_after_column_def: comment,
506+
comment,
507507
on_commit,
508508
on_cluster,
509509
primary_key,

‎src/ast/mod.rs‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7566,10 +7566,7 @@ pub enum SqlOption {
75667566
/// Any option that consists of a key value pair where the value is an expression. e.g.
75677567
///
75687568
/// WITH(DISTRIBUTION = ROUND_ROBIN)
7569-
KeyValue {
7570-
key: Ident,
7571-
value: Expr,
7572-
},
7569+
KeyValue { key: Ident, value: Expr },
75737570
/// One or more table partitions and represents which partition the boundary values belong to,
75747571
/// e.g.
75757572
///
@@ -7581,15 +7578,17 @@ pub enum SqlOption {
75817578
range_direction: Option<PartitionRangeDirection>,
75827579
for_values: Vec<Expr>,
75837580
},
7584-
7581+
/// Comment parameter (supports `=` and no `=` syntax)
75857582
Comment(CommentDef),
7586-
7583+
/// MySQL TableSpace option
7584+
/// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
75877585
TableSpace(TablespaceOption),
7588-
7589-
// Advanced parameter formations
7590-
// UNION = (tbl_name[,tbl_name]...)
7591-
// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver)
7592-
// ENGINE = SummingMergeTree([columns])
7586+
/// An option representing a key value pair, where the value is a parenthesized list and with an optional name
7587+
/// e.g.
7588+
///
7589+
/// UNION = (tbl_name\[,tbl_name\]...) <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
7590+
/// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver) <https://clickhouse.com/docs/engines/table-engines/mergetree-family/replication>
7591+
/// ENGINE = SummingMergeTree(\[columns\]) <https://clickhouse.com/docs/engines/table-engines/mergetree-family/summingmergetree>
75937592
NamedParenthesizedList(NamedParenthesizedList),
75947593
}
75957594

@@ -7627,7 +7626,7 @@ impl fmt::Display for SqlOption {
76277626
match tablespace_option.storage {
76287627
Some(StorageType::Disk) => write!(f, " STORAGE DISK"),
76297628
Some(StorageType::Memory) => write!(f, " STORAGE MEMORY"),
7630-
_ => Ok(()),
7629+
None => Ok(()),
76317630
}
76327631
}
76337632
SqlOption::Comment(comment) => match comment {
@@ -7640,11 +7639,11 @@ impl fmt::Display for SqlOption {
76407639
},
76417640
SqlOption::NamedParenthesizedList(value) => {
76427641
write!(f, "{} = ", value.key)?;
7643-
if let Some(key) = &value.value {
7642+
if let Some(key) = &value.name {
76447643
write!(f, "{}", key)?;
76457644
}
7646-
if !value.parameters.is_empty() {
7647-
write!(f, "({})", display_comma_separated(&value.parameters))?
7645+
if !value.values.is_empty() {
7646+
write!(f, "({})", display_comma_separated(&value.values))?
76487647
}
76497648
Ok(())
76507649
}
@@ -7663,6 +7662,8 @@ pub enum StorageType {
76637662
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
76647663
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
76657664
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
7665+
/// MySql TableSpace option
7666+
/// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
76667667
pub struct TablespaceOption {
76677668
pub name: String,
76687669
pub storage: Option<StorageType>,
@@ -8936,10 +8937,17 @@ impl Display for CreateViewParams {
89368937
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
89378938
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
89388939
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
8940+
/// Key/Value, where the value is a (optionally named) list of identifiers
8941+
///
8942+
/// ```sql
8943+
/// UNION = (tbl_name[,tbl_name]...)
8944+
/// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver)
8945+
/// ENGINE = SummingMergeTree([columns])
8946+
/// ```
89398947
pub struct NamedParenthesizedList {
89408948
pub key: Ident,
8941-
pub value: Option<Ident>,
8942-
pub parameters: Vec<Ident>,
8949+
pub name: Option<Ident>,
8950+
pub values: Vec<Ident>,
89438951
}
89448952

89458953
/// Snowflake `WITH ROW ACCESS POLICY policy_name ON (identifier, ...)`

‎src/ast/spans.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ impl Spanned for CreateTable {
573573
without_rowid: _, // bool
574574
like,
575575
clone,
576-
comment_after_column_def: _, // todo, no span
576+
comment: _, // todo, no span
577577
on_commit: _,
578578
on_cluster: _, // todo, clickhouse specific
579579
primary_key: _, // todo, clickhouse specific
@@ -1001,8 +1001,8 @@ impl Spanned for SqlOption {
10011001
SqlOption::Comment(_) => Span::empty(),
10021002
SqlOption::NamedParenthesizedList(NamedParenthesizedList {
10031003
key: name,
1004-
value,
1005-
parameters: values,
1004+
name: value,
1005+
values,
10061006
}) => union_spans(core::iter::once(name.span).chain(values.iter().map(|i| i.span)))
10071007
.union_opt(&value.as_ref().map(|i| i.span)),
10081008
}

‎src/parser/mod.rs‎

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7222,11 +7222,13 @@ impl<'a> Parser<'a> {
72227222

72237223
fn parse_plain_option(&mut self) -> Result<Option<SqlOption>, ParserError> {
72247224
// Single parameter option
7225+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
72257226
if self.parse_keywords(&[Keyword::START, Keyword::TRANSACTION]) {
72267227
return Ok(Some(SqlOption::Ident(Ident::new("START TRANSACTION"))));
72277228
}
72287229

72297230
// Custom option
7231+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
72307232
if self.parse_keywords(&[Keyword::COMMENT]) {
72317233
let has_eq = self.consume_token(&Token::Eq);
72327234
let value = self.next_token();
@@ -7245,6 +7247,8 @@ impl<'a> Parser<'a> {
72457247
return comment;
72467248
}
72477249

7250+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
7251+
// <https://clickhouse.com/docs/sql-reference/statements/create/table>
72487252
if self.parse_keywords(&[Keyword::ENGINE]) {
72497253
let _ = self.consume_token(&Token::Eq);
72507254
let value = self.next_token();
@@ -7260,8 +7264,8 @@ impl<'a> Parser<'a> {
72607264
Ok(Some(SqlOption::NamedParenthesizedList(
72617265
NamedParenthesizedList {
72627266
key: Ident::new("ENGINE"),
7263-
value: Some(Ident::new(w.value)),
7264-
parameters,
7267+
name: Some(Ident::new(w.value)),
7268+
values: parameters,
72657269
},
72667270
)))
72677271
}
@@ -7273,12 +7277,12 @@ impl<'a> Parser<'a> {
72737277
return engine;
72747278
}
72757279

7280+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
72767281
if self.parse_keywords(&[Keyword::TABLESPACE]) {
72777282
let _ = self.consume_token(&Token::Eq);
72787283
let value = self.next_token();
72797284

72807285
let tablespace = match value.token {
7281-
// TABLESPACE tablespace_name [STORAGE DISK] | [TABLESPACE tablespace_name] STORAGE MEMORY
72827286
Token::Word(Word { value: name, .. }) | Token::SingleQuotedString(name) => {
72837287
let storage = match self.parse_keyword(Keyword::STORAGE) {
72847288
true => {
@@ -7310,12 +7314,12 @@ impl<'a> Parser<'a> {
73107314
return tablespace;
73117315
}
73127316

7317+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
73137318
if self.parse_keyword(Keyword::UNION) {
73147319
let _ = self.consume_token(&Token::Eq);
73157320
let value = self.next_token();
73167321

73177322
match value.token {
7318-
// UNION [=] (tbl_name[,tbl_name]...)
73197323
Token::LParen => {
73207324
let tables: Vec<Ident> =
73217325
self.parse_comma_separated0(Parser::parse_identifier, Token::RParen)?;
@@ -7324,8 +7328,8 @@ impl<'a> Parser<'a> {
73247328
return Ok(Some(SqlOption::NamedParenthesizedList(
73257329
NamedParenthesizedList {
73267330
key: Ident::new("UNION"),
7327-
value: None,
7328-
parameters: tables,
7331+
name: None,
7332+
values: tables,
73297333
},
73307334
)));
73317335
}
@@ -7337,85 +7341,58 @@ impl<'a> Parser<'a> {
73377341

73387342
// Key/Value parameter option
73397343
let key = if self.parse_keywords(&[Keyword::DEFAULT, Keyword::CHARSET]) {
7340-
// [DEFAULT] CHARACTER SET [=] charset_name
73417344
Ident::new("DEFAULT CHARSET")
73427345
} else if self.parse_keyword(Keyword::CHARSET) {
7343-
// [DEFAULT] CHARACTER SET [=] charset_name
73447346
Ident::new("CHARSET")
73457347
} else if self.parse_keywords(&[Keyword::DEFAULT, Keyword::CHARACTER, Keyword::SET]) {
7346-
// [DEFAULT] CHARACTER SET [=] charset_name
73477348
Ident::new("DEFAULT CHARACTER SET")
73487349
} else if self.parse_keywords(&[Keyword::CHARACTER, Keyword::SET]) {
7349-
// [DEFAULT] CHARACTER SET [=] charset_name
73507350
Ident::new("CHARACTER SET")
73517351
} else if self.parse_keywords(&[Keyword::DEFAULT, Keyword::COLLATE]) {
7352-
// [DEFAULT] COLLATE [=] collation_name
73537352
Ident::new("DEFAULT COLLATE")
73547353
} else if self.parse_keyword(Keyword::COLLATE) {
7355-
// [DEFAULT] COLLATE [=] collation_name
73567354
Ident::new("COLLATE")
73577355
} else if self.parse_keywords(&[Keyword::DATA, Keyword::DIRECTORY]) {
7358-
// {DATA | INDEX} DIRECTORY [=] 'absolute path to directory'
73597356
Ident::new("DATA DIRECTORY")
73607357
} else if self.parse_keywords(&[Keyword::INDEX, Keyword::DIRECTORY]) {
7361-
// {DATA | INDEX} DIRECTORY [=] 'absolute path to directory'
73627358
Ident::new("INDEX DIRECTORY")
73637359
} else if self.parse_keyword(Keyword::KEY_BLOCK_SIZE) {
7364-
// KEY_BLOCK_SIZE [=] value
73657360
Ident::new("KEY_BLOCK_SIZE")
73667361
} else if self.parse_keyword(Keyword::ROW_FORMAT) {
7367-
// ROW_FORMAT [=] {DEFAULT | DYNAMIC | FIXED | COMPRESSED | REDUNDANT | COMPACT}
73687362
Ident::new("ROW_FORMAT")
73697363
} else if self.parse_keyword(Keyword::PACK_KEYS) {
7370-
// PACK_KEYS [=] {0 | 1 | DEFAULT}
73717364
Ident::new("PACK_KEYS")
73727365
} else if self.parse_keyword(Keyword::STATS_AUTO_RECALC) {
7373-
// STATS_AUTO_RECALC [=] {DEFAULT | 0 | 1}
73747366
Ident::new("STATS_AUTO_RECALC")
73757367
} else if self.parse_keyword(Keyword::STATS_PERSISTENT) {
7376-
//STATS_PERSISTENT [=] {DEFAULT | 0 | 1}
73777368
Ident::new("STATS_PERSISTENT")
73787369
} else if self.parse_keyword(Keyword::STATS_SAMPLE_PAGES) {
7379-
// STATS_SAMPLE_PAGES [=] value
73807370
Ident::new("STATS_SAMPLE_PAGES")
73817371
} else if self.parse_keyword(Keyword::DELAY_KEY_WRITE) {
7382-
// DELAY_KEY_WRITE [=] {0 | 1}
73837372
Ident::new("DELAY_KEY_WRITE")
73847373
} else if self.parse_keyword(Keyword::COMPRESSION) {
7385-
// COMPRESSION [=] {'ZLIB' | 'LZ4' | 'NONE'}
73867374
Ident::new("COMPRESSION")
73877375
} else if self.parse_keyword(Keyword::ENCRYPTION) {
7388-
// ENCRYPTION [=] {'Y' | 'N'}
73897376
Ident::new("ENCRYPTION")
73907377
} else if self.parse_keyword(Keyword::MAX_ROWS) {
7391-
// MAX_ROWS [=] value
73927378
Ident::new("MAX_ROWS")
73937379
} else if self.parse_keyword(Keyword::MIN_ROWS) {
7394-
// MIN_ROWS [=] value
73957380
Ident::new("MIN_ROWS")
73967381
} else if self.parse_keyword(Keyword::AUTOEXTEND_SIZE) {
7397-
// AUTOEXTEND_SIZE [=] value
73987382
Ident::new("AUTOEXTEND_SIZE")
73997383
} else if self.parse_keyword(Keyword::AVG_ROW_LENGTH) {
7400-
// AVG_ROW_LENGTH [=] value
74017384
Ident::new("AVG_ROW_LENGTH")
74027385
} else if self.parse_keyword(Keyword::CHECKSUM) {
7403-
// CHECKSUM [=] {0 | 1}
74047386
Ident::new("CHECKSUM")
74057387
} else if self.parse_keyword(Keyword::CONNECTION) {
7406-
// CONNECTION [=] 'connect_string'
74077388
Ident::new("CONNECTION")
74087389
} else if self.parse_keyword(Keyword::ENGINE_ATTRIBUTE) {
7409-
// ENGINE_ATTRIBUTE [=] 'string'
74107390
Ident::new("ENGINE_ATTRIBUTE")
74117391
} else if self.parse_keyword(Keyword::PASSWORD) {
7412-
// PASSWORD [=] 'string'
74137392
Ident::new("PASSWORD")
74147393
} else if self.parse_keyword(Keyword::SECONDARY_ENGINE_ATTRIBUTE) {
7415-
// SECONDARY_ENGINE_ATTRIBUTE [=] 'string'
74167394
Ident::new("SECONDARY_ENGINE_ATTRIBUTE")
74177395
} else if self.parse_keyword(Keyword::INSERT_METHOD) {
7418-
// INSERT_METHOD [=] { NO | FIRST | LAST }
74197396
Ident::new("INSERT_METHOD")
74207397
} else if self.parse_keyword(Keyword::AUTO_INCREMENT) {
74217398
Ident::new("AUTO_INCREMENT")

‎tests/sqlparser_clickhouse.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -751,8 +751,8 @@ fn parse_create_table_with_primary_key() {
751751
assert!(plain_options.contains(&SqlOption::NamedParenthesizedList(
752752
NamedParenthesizedList {
753753
key: Ident::new("ENGINE"),
754-
value: Some(Ident::new("SharedMergeTree")),
755-
parameters: vec![
754+
name: Some(Ident::new("SharedMergeTree")),
755+
values: vec![
756756
Ident::with_quote('\'', "/clickhouse/tables/{uuid}/{shard}"),
757757
Ident::with_quote('\'', "{replica}"),
758758
]

‎tests/sqlparser_duckdb.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ fn test_duckdb_union_datatype() {
741741
without_rowid: Default::default(),
742742
like: Default::default(),
743743
clone: Default::default(),
744-
comment_after_column_def: Default::default(),
744+
comment: Default::default(),
745745
on_commit: Default::default(),
746746
on_cluster: Default::default(),
747747
primary_key: Default::default(),

‎tests/sqlparser_hive.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,7 @@ fn create_table_with_comment() {
130130
" INTO 4 BUCKETS"
131131
);
132132
match hive().verified_stmt(sql) {
133-
Statement::CreateTable(CreateTable {
134-
comment_after_column_def: comment,
135-
..
136-
}) => {
133+
Statement::CreateTable(CreateTable { comment, .. }) => {
137134
assert_eq!(
138135
comment,
139136
Some(CommentDef::WithoutEq("table comment".to_string()))

‎tests/sqlparser_mssql.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ fn parse_create_table_with_valid_options() {
17511751
without_rowid: false,
17521752
like: None,
17531753
clone: None,
1754-
comment_after_column_def: None,
1754+
comment: None,
17551755
on_commit: None,
17561756
on_cluster: None,
17571757
primary_key: None,
@@ -1916,7 +1916,7 @@ fn parse_create_table_with_identity_column() {
19161916
without_rowid: false,
19171917
like: None,
19181918
clone: None,
1919-
comment_after_column_def: None,
1919+
comment: None,
19201920
on_commit: None,
19211921
on_cluster: None,
19221922
primary_key: None,

0 commit comments

Comments
 (0)