Skip to content

Commit 26ea8b1

Browse files
tomershaniiitomershaniii
committed
create table parsing - review feeback
1 parent 467ccbd commit 26ea8b1

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
@@ -7463,10 +7463,7 @@ pub enum SqlOption {
74637463
/// Any option that consists of a key value pair where the value is an expression. e.g.
74647464
///
74657465
/// WITH(DISTRIBUTION = ROUND_ROBIN)
7466-
KeyValue {
7467-
key: Ident,
7468-
value: Expr,
7469-
},
7466+
KeyValue { key: Ident, value: Expr },
74707467
/// One or more table partitions and represents which partition the boundary values belong to,
74717468
/// e.g.
74727469
///
@@ -7478,15 +7475,17 @@ pub enum SqlOption {
74787475
range_direction: Option<PartitionRangeDirection>,
74797476
for_values: Vec<Expr>,
74807477
},
7481-
7478+
/// Comment parameter (supports `=` and no `=` syntax)
74827479
Comment(CommentDef),
7483-
7480+
/// MySQL TableSpace option
7481+
/// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
74847482
TableSpace(TablespaceOption),
7485-
7486-
// Advanced parameter formations
7487-
// UNION = (tbl_name[,tbl_name]...)
7488-
// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver)
7489-
// ENGINE = SummingMergeTree([columns])
7483+
/// An option representing a key value pair, where the value is a parenthesized list and with an optional name
7484+
/// e.g.
7485+
///
7486+
/// UNION = (tbl_name[,tbl_name]...) <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
7487+
/// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver) <https://clickhouse.com/docs/engines/table-engines/mergetree-family/replication>
7488+
/// ENGINE = SummingMergeTree([columns]) <https://clickhouse.com/docs/engines/table-engines/mergetree-family/summingmergetree>
74907489
NamedParenthesizedList(NamedParenthesizedList),
74917490
}
74927491

@@ -7524,7 +7523,7 @@ impl fmt::Display for SqlOption {
75247523
match tablespace_option.storage {
75257524
Some(StorageType::Disk) => write!(f, " STORAGE DISK"),
75267525
Some(StorageType::Memory) => write!(f, " STORAGE MEMORY"),
7527-
_ => Ok(()),
7526+
None => Ok(()),
75287527
}
75297528
}
75307529
SqlOption::Comment(comment) => match comment {
@@ -7537,11 +7536,11 @@ impl fmt::Display for SqlOption {
75377536
},
75387537
SqlOption::NamedParenthesizedList(value) => {
75397538
write!(f, "{} = ", value.key)?;
7540-
if let Some(key) = &value.value {
7539+
if let Some(key) = &value.name {
75417540
write!(f, "{}", key)?;
75427541
}
7543-
if !value.parameters.is_empty() {
7544-
write!(f, "({})", display_comma_separated(&value.parameters))?
7542+
if !value.values.is_empty() {
7543+
write!(f, "({})", display_comma_separated(&value.values))?
75457544
}
75467545
Ok(())
75477546
}
@@ -7560,6 +7559,8 @@ pub enum StorageType {
75607559
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
75617560
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
75627561
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
7562+
/// MySql TableSpace option
7563+
/// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
75637564
pub struct TablespaceOption {
75647565
pub name: String,
75657566
pub storage: Option<StorageType>,
@@ -8833,10 +8834,17 @@ impl Display for CreateViewParams {
88338834
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
88348835
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
88358836
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
8837+
/// Key/Value, where the value is a (optionally named) list of identifiers
8838+
///
8839+
/// ```sql
8840+
/// UNION = (tbl_name[,tbl_name]...)
8841+
/// ENGINE = ReplicatedMergeTree('/table_name','{replica}', ver)
8842+
/// ENGINE = SummingMergeTree([columns])
8843+
/// ```
88368844
pub struct NamedParenthesizedList {
88378845
pub key: Ident,
8838-
pub value: Option<Ident>,
8839-
pub parameters: Vec<Ident>,
8846+
pub name: Option<Ident>,
8847+
pub values: Vec<Ident>,
88408848
}
88418849

88428850
/// 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
@@ -569,7 +569,7 @@ impl Spanned for CreateTable {
569569
without_rowid: _, // bool
570570
like,
571571
clone,
572-
comment_after_column_def: _, // todo, no span
572+
comment: _, // todo, no span
573573
on_commit: _,
574574
on_cluster: _, // todo, clickhouse specific
575575
primary_key: _, // todo, clickhouse specific
@@ -989,8 +989,8 @@ impl Spanned for SqlOption {
989989
SqlOption::Comment(_) => Span::empty(),
990990
SqlOption::NamedParenthesizedList(NamedParenthesizedList {
991991
key: name,
992-
value,
993-
parameters: values,
992+
name: value,
993+
values,
994994
}) => union_spans(core::iter::once(name.span).chain(values.iter().map(|i| i.span)))
995995
.union_opt(&value.as_ref().map(|i| i.span)),
996996
}

‎src/parser/mod.rs‎

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

71407140
fn parse_plain_option(&mut self) -> Result<Option<SqlOption>, ParserError> {
71417141
// Single parameter option
7142+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
71427143
if self.parse_keywords(&[Keyword::START, Keyword::TRANSACTION]) {
71437144
return Ok(Some(SqlOption::Ident(Ident::new("START TRANSACTION"))));
71447145
}
71457146

71467147
// Custom option
7148+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
71477149
if self.parse_keywords(&[Keyword::COMMENT]) {
71487150
let has_eq = self.consume_token(&Token::Eq);
71497151
let value = self.next_token();
@@ -7162,6 +7164,8 @@ impl<'a> Parser<'a> {
71627164
return comment;
71637165
}
71647166

7167+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
7168+
// <https://clickhouse.com/docs/sql-reference/statements/create/table>
71657169
if self.parse_keywords(&[Keyword::ENGINE]) {
71667170
let _ = self.consume_token(&Token::Eq);
71677171
let value = self.next_token();
@@ -7177,8 +7181,8 @@ impl<'a> Parser<'a> {
71777181
Ok(Some(SqlOption::NamedParenthesizedList(
71787182
NamedParenthesizedList {
71797183
key: Ident::new("ENGINE"),
7180-
value: Some(Ident::new(w.value)),
7181-
parameters,
7184+
name: Some(Ident::new(w.value)),
7185+
values: parameters,
71827186
},
71837187
)))
71847188
}
@@ -7190,12 +7194,12 @@ impl<'a> Parser<'a> {
71907194
return engine;
71917195
}
71927196

7197+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
71937198
if self.parse_keywords(&[Keyword::TABLESPACE]) {
71947199
let _ = self.consume_token(&Token::Eq);
71957200
let value = self.next_token();
71967201

71977202
let tablespace = match value.token {
7198-
// TABLESPACE tablespace_name [STORAGE DISK] | [TABLESPACE tablespace_name] STORAGE MEMORY
71997203
Token::Word(Word { value: name, .. }) | Token::SingleQuotedString(name) => {
72007204
let storage = match self.parse_keyword(Keyword::STORAGE) {
72017205
true => {
@@ -7227,12 +7231,12 @@ impl<'a> Parser<'a> {
72277231
return tablespace;
72287232
}
72297233

7234+
// <https://dev.mysql.com/doc/refman/8.4/en/create-table.html>
72307235
if self.parse_keyword(Keyword::UNION) {
72317236
let _ = self.consume_token(&Token::Eq);
72327237
let value = self.next_token();
72337238

72347239
match value.token {
7235-
// UNION [=] (tbl_name[,tbl_name]...)
72367240
Token::LParen => {
72377241
let tables: Vec<Ident> =
72387242
self.parse_comma_separated0(Parser::parse_identifier, Token::RParen)?;
@@ -7241,8 +7245,8 @@ impl<'a> Parser<'a> {
72417245
return Ok(Some(SqlOption::NamedParenthesizedList(
72427246
NamedParenthesizedList {
72437247
key: Ident::new("UNION"),
7244-
value: None,
7245-
parameters: tables,
7248+
name: None,
7249+
values: tables,
72467250
},
72477251
)));
72487252
}
@@ -7254,85 +7258,58 @@ impl<'a> Parser<'a> {
72547258

72557259
// Key/Value parameter option
72567260
let key = if self.parse_keywords(&[Keyword::DEFAULT, Keyword::CHARSET]) {
7257-
// [DEFAULT] CHARACTER SET [=] charset_name
72587261
Ident::new("DEFAULT CHARSET")
72597262
} else if self.parse_keyword(Keyword::CHARSET) {
7260-
// [DEFAULT] CHARACTER SET [=] charset_name
72617263
Ident::new("CHARSET")
72627264
} else if self.parse_keywords(&[Keyword::DEFAULT, Keyword::CHARACTER, Keyword::SET]) {
7263-
// [DEFAULT] CHARACTER SET [=] charset_name
72647265
Ident::new("DEFAULT CHARACTER SET")
72657266
} else if self.parse_keywords(&[Keyword::CHARACTER, Keyword::SET]) {
7266-
// [DEFAULT] CHARACTER SET [=] charset_name
72677267
Ident::new("CHARACTER SET")
72687268
} else if self.parse_keywords(&[Keyword::DEFAULT, Keyword::COLLATE]) {
7269-
// [DEFAULT] COLLATE [=] collation_name
72707269
Ident::new("DEFAULT COLLATE")
72717270
} else if self.parse_keyword(Keyword::COLLATE) {
7272-
// [DEFAULT] COLLATE [=] collation_name
72737271
Ident::new("COLLATE")
72747272
} else if self.parse_keywords(&[Keyword::DATA, Keyword::DIRECTORY]) {
7275-
// {DATA | INDEX} DIRECTORY [=] 'absolute path to directory'
72767273
Ident::new("DATA DIRECTORY")
72777274
} else if self.parse_keywords(&[Keyword::INDEX, Keyword::DIRECTORY]) {
7278-
// {DATA | INDEX} DIRECTORY [=] 'absolute path to directory'
72797275
Ident::new("INDEX DIRECTORY")
72807276
} else if self.parse_keyword(Keyword::KEY_BLOCK_SIZE) {
7281-
// KEY_BLOCK_SIZE [=] value
72827277
Ident::new("KEY_BLOCK_SIZE")
72837278
} else if self.parse_keyword(Keyword::ROW_FORMAT) {
7284-
// ROW_FORMAT [=] {DEFAULT | DYNAMIC | FIXED | COMPRESSED | REDUNDANT | COMPACT}
72857279
Ident::new("ROW_FORMAT")
72867280
} else if self.parse_keyword(Keyword::PACK_KEYS) {
7287-
// PACK_KEYS [=] {0 | 1 | DEFAULT}
72887281
Ident::new("PACK_KEYS")
72897282
} else if self.parse_keyword(Keyword::STATS_AUTO_RECALC) {
7290-
// STATS_AUTO_RECALC [=] {DEFAULT | 0 | 1}
72917283
Ident::new("STATS_AUTO_RECALC")
72927284
} else if self.parse_keyword(Keyword::STATS_PERSISTENT) {
7293-
//STATS_PERSISTENT [=] {DEFAULT | 0 | 1}
72947285
Ident::new("STATS_PERSISTENT")
72957286
} else if self.parse_keyword(Keyword::STATS_SAMPLE_PAGES) {
7296-
// STATS_SAMPLE_PAGES [=] value
72977287
Ident::new("STATS_SAMPLE_PAGES")
72987288
} else if self.parse_keyword(Keyword::DELAY_KEY_WRITE) {
7299-
// DELAY_KEY_WRITE [=] {0 | 1}
73007289
Ident::new("DELAY_KEY_WRITE")
73017290
} else if self.parse_keyword(Keyword::COMPRESSION) {
7302-
// COMPRESSION [=] {'ZLIB' | 'LZ4' | 'NONE'}
73037291
Ident::new("COMPRESSION")
73047292
} else if self.parse_keyword(Keyword::ENCRYPTION) {
7305-
// ENCRYPTION [=] {'Y' | 'N'}
73067293
Ident::new("ENCRYPTION")
73077294
} else if self.parse_keyword(Keyword::MAX_ROWS) {
7308-
// MAX_ROWS [=] value
73097295
Ident::new("MAX_ROWS")
73107296
} else if self.parse_keyword(Keyword::MIN_ROWS) {
7311-
// MIN_ROWS [=] value
73127297
Ident::new("MIN_ROWS")
73137298
} else if self.parse_keyword(Keyword::AUTOEXTEND_SIZE) {
7314-
// AUTOEXTEND_SIZE [=] value
73157299
Ident::new("AUTOEXTEND_SIZE")
73167300
} else if self.parse_keyword(Keyword::AVG_ROW_LENGTH) {
7317-
// AVG_ROW_LENGTH [=] value
73187301
Ident::new("AVG_ROW_LENGTH")
73197302
} else if self.parse_keyword(Keyword::CHECKSUM) {
7320-
// CHECKSUM [=] {0 | 1}
73217303
Ident::new("CHECKSUM")
73227304
} else if self.parse_keyword(Keyword::CONNECTION) {
7323-
// CONNECTION [=] 'connect_string'
73247305
Ident::new("CONNECTION")
73257306
} else if self.parse_keyword(Keyword::ENGINE_ATTRIBUTE) {
7326-
// ENGINE_ATTRIBUTE [=] 'string'
73277307
Ident::new("ENGINE_ATTRIBUTE")
73287308
} else if self.parse_keyword(Keyword::PASSWORD) {
7329-
// PASSWORD [=] 'string'
73307309
Ident::new("PASSWORD")
73317310
} else if self.parse_keyword(Keyword::SECONDARY_ENGINE_ATTRIBUTE) {
7332-
// SECONDARY_ENGINE_ATTRIBUTE [=] 'string'
73337311
Ident::new("SECONDARY_ENGINE_ATTRIBUTE")
73347312
} else if self.parse_keyword(Keyword::INSERT_METHOD) {
7335-
// INSERT_METHOD [=] { NO | FIRST | LAST }
73367313
Ident::new("INSERT_METHOD")
73377314
} else if self.parse_keyword(Keyword::AUTO_INCREMENT) {
73387315
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
@@ -1667,7 +1667,7 @@ fn parse_create_table_with_valid_options() {
16671667
without_rowid: false,
16681668
like: None,
16691669
clone: None,
1670-
comment_after_column_def: None,
1670+
comment: None,
16711671
on_commit: None,
16721672
on_cluster: None,
16731673
primary_key: None,
@@ -1832,7 +1832,7 @@ fn parse_create_table_with_identity_column() {
18321832
without_rowid: false,
18331833
like: None,
18341834
clone: None,
1835-
comment_after_column_def: None,
1835+
comment: None,
18361836
on_commit: None,
18371837
on_cluster: None,
18381838
primary_key: None,

0 commit comments

Comments
 (0)