Skip to content

Commit 3665d8c

Browse files
bombsimonbombsimon
committed
Address PR feedback
- Rename T-SQL to MSSQL in docs - Add examples and links to all `SqlOption` - Remove duplicate `parse_options` function - Avoid nested if/else chains - Rename field of `SqlOption::KeyValue` to `key` - Extend testing with function calls, negative tests and using `verified_stmt` - Move parsing of `ASC`|`DESC` to function
1 parent 7612a1e commit 3665d8c

File tree

6 files changed

+242
-171
lines changed

6 files changed

+242
-171
lines changed

‎src/ast/mod.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,7 +1944,7 @@ pub enum CreateTableOptions {
19441944
///
19451945
/// <https://www.postgresql.org/docs/current/sql-createtable.html>
19461946
///
1947-
/// T-sql supports more specific options that's not only key-value pairs.
1947+
/// MSSQL supports more specific options that's not only key-value pairs.
19481948
///
19491949
/// WITH (
19501950
/// DISTRIBUTION = ROUND_ROBIN,
@@ -5656,15 +5656,22 @@ pub enum PartitionRangeDirection {
56565656
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
56575657
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
56585658
pub enum SqlOption {
5659-
/// Clustered represents the clustered version of table storage for T-sql.
5659+
/// Clustered represents the clustered version of table storage for MSSQL.
56605660
///
56615661
/// <https://learn.microsoft.com/en-us/sql/t-sql/statements/create-table-azure-sql-data-warehouse?view=aps-pdw-2016-au7#TableOptions>
56625662
Clustered(TableOptionsClustered),
5663-
/// Single identifier options, e.g. `HEAP`.
5663+
/// Single identifier options, e.g. `HEAP` for MSSQL.
5664+
///
5665+
/// <https://learn.microsoft.com/en-us/sql/t-sql/statements/create-table-azure-sql-data-warehouse?view=aps-pdw-2016-au7#TableOptions>
56645666
Ident(Ident),
5665-
/// Any option that consists of a key value pair where the value is an expression.
5666-
KeyValue { name: Ident, value: Expr },
5667-
/// One or more table partitions and represents which partition the boundary values belong to.
5667+
/// Any option that consists of a key value pair where the value is an expression. e.g.
5668+
///
5669+
/// WITH(DISTRIBUTION = ROUND_ROBIN)
5670+
KeyValue { key: Ident, value: Expr },
5671+
/// One or more table partitions and represents which partition the boundary values belong to,
5672+
/// e.g.
5673+
///
5674+
/// PARTITION (id RANGE LEFT FOR VALUES (10, 20, 30, 40))
56685675
///
56695676
/// <https://learn.microsoft.com/en-us/sql/t-sql/statements/create-table-azure-sql-data-warehouse?view=aps-pdw-2016-au7#TablePartitionOptions>
56705677
Partition {
@@ -5681,7 +5688,7 @@ impl fmt::Display for SqlOption {
56815688
SqlOption::Ident(ident) => {
56825689
write!(f, "{}", ident)
56835690
}
5684-
SqlOption::KeyValue { name, value } => {
5691+
SqlOption::KeyValue { key: name, value } => {
56855692
write!(f, "{} = {}", name, value)
56865693
}
56875694
SqlOption::Partition {

‎src/parser/mod.rs

Lines changed: 48 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4458,7 +4458,7 @@ impl<'a> Parser<'a> {
44584458
let name = self.parse_object_name(allow_unquoted_hyphen)?;
44594459
let columns = self.parse_view_columns()?;
44604460
let mut options = CreateTableOptions::None;
4461-
let with_options = self.parse_table_options(Keyword::WITH)?;
4461+
let with_options = self.parse_options(Keyword::WITH)?;
44624462
if !with_options.is_empty() {
44634463
options = CreateTableOptions::With(with_options);
44644464
}
@@ -5621,8 +5621,7 @@ impl<'a> Parser<'a> {
56215621
let clustered_by = self.parse_optional_clustered_by()?;
56225622
let hive_formats = self.parse_hive_formats()?;
56235623
// PostgreSQL supports `WITH ( options )`, before `AS`
5624-
// T-sql supports `WITH` options for clustering and distribution
5625-
let with_options = self.parse_table_options(Keyword::WITH)?;
5624+
let with_options = self.parse_options(Keyword::WITH)?;
56265625
let table_properties = self.parse_options(Keyword::TBLPROPERTIES)?;
56275626

56285627
let engine = if self.parse_keyword(Keyword::ENGINE) {
@@ -6400,17 +6399,6 @@ impl<'a> Parser<'a> {
64006399
Ok(None)
64016400
}
64026401

6403-
pub fn parse_table_options(&mut self, keyword: Keyword) -> Result<Vec<SqlOption>, ParserError> {
6404-
if self.parse_keyword(keyword) {
6405-
self.expect_token(&Token::LParen)?;
6406-
let options = self.parse_comma_separated(Parser::parse_sql_option)?;
6407-
self.expect_token(&Token::RParen)?;
6408-
Ok(options)
6409-
} else {
6410-
Ok(vec![])
6411-
}
6412-
}
6413-
64146402
pub fn parse_options(&mut self, keyword: Keyword) -> Result<Vec<SqlOption>, ParserError> {
64156403
if self.parse_keyword(keyword) {
64166404
self.expect_token(&Token::LParen)?;
@@ -6505,64 +6493,62 @@ impl<'a> Parser<'a> {
65056493
}
65066494

65076495
pub fn parse_sql_option(&mut self) -> Result<SqlOption, ParserError> {
6508-
let next_token = self.peek_token();
65096496
let is_mssql = dialect_of!(self is MsSqlDialect|GenericDialect);
65106497

6511-
let Token::Word(w) = next_token.token else {
6512-
let (name, value) = self.parse_key_value()?;
6513-
return Ok(SqlOption::KeyValue { name, value });
6514-
};
6515-
6516-
match w.keyword {
6517-
Keyword::HEAP if is_mssql => Ok(SqlOption::Ident(self.parse_identifier(false)?)),
6518-
Keyword::PARTITION if is_mssql => self.parse_table_option_partition(),
6519-
Keyword::CLUSTERED if is_mssql => self.parse_table_option_clustered(),
6498+
match self.peek_token().token {
6499+
Token::Word(w) if w.keyword == Keyword::HEAP && is_mssql => {
6500+
Ok(SqlOption::Ident(self.parse_identifier(false)?))
6501+
}
6502+
Token::Word(w) if w.keyword == Keyword::PARTITION && is_mssql => {
6503+
self.parse_option_partition()
6504+
}
6505+
Token::Word(w) if w.keyword == Keyword::CLUSTERED && is_mssql => {
6506+
self.parse_option_clustered()
6507+
}
65206508
_ => {
65216509
let (name, value) = self.parse_key_value()?;
6522-
Ok(SqlOption::KeyValue { name, value })
6510+
Ok(SqlOption::KeyValue { key: name, value })
65236511
}
65246512
}
65256513
}
65266514

6527-
pub fn parse_table_option_clustered(&mut self) -> Result<SqlOption, ParserError> {
6528-
self.expect_keyword(Keyword::CLUSTERED)?;
6529-
6530-
if self.parse_keywords(&[Keyword::COLUMNSTORE, Keyword::INDEX]) {
6531-
if self.parse_keyword(Keyword::ORDER) {
6532-
Ok(SqlOption::Clustered(
6533-
TableOptionsClustered::ColumnstoreIndexOrder(
6534-
self.parse_parenthesized_column_list(IsOptional::Mandatory, false)?,
6535-
),
6536-
))
6537-
} else {
6538-
Ok(SqlOption::Clustered(
6539-
TableOptionsClustered::ColumnstoreIndex,
6540-
))
6541-
}
6542-
} else {
6543-
self.expect_keyword(Keyword::INDEX)?;
6515+
pub fn parse_option_clustered(&mut self) -> Result<SqlOption, ParserError> {
6516+
if self.parse_keywords(&[
6517+
Keyword::CLUSTERED,
6518+
Keyword::COLUMNSTORE,
6519+
Keyword::INDEX,
6520+
Keyword::ORDER,
6521+
]) {
6522+
Ok(SqlOption::Clustered(
6523+
TableOptionsClustered::ColumnstoreIndexOrder(
6524+
self.parse_parenthesized_column_list(IsOptional::Mandatory, false)?,
6525+
),
6526+
))
6527+
} else if self.parse_keywords(&[Keyword::CLUSTERED, Keyword::COLUMNSTORE, Keyword::INDEX]) {
6528+
Ok(SqlOption::Clustered(
6529+
TableOptionsClustered::ColumnstoreIndex,
6530+
))
6531+
} else if self.parse_keywords(&[Keyword::CLUSTERED, Keyword::INDEX]) {
65446532
self.expect_token(&Token::LParen)?;
65456533

65466534
let columns = self.parse_comma_separated(|p| {
65476535
let name = p.parse_identifier(false)?;
6548-
let asc = if p.parse_keyword(Keyword::ASC) {
6549-
Some(true)
6550-
} else if p.parse_keyword(Keyword::DESC) {
6551-
Some(false)
6552-
} else {
6553-
None
6554-
};
6536+
let asc = p.parse_asc();
65556537

65566538
Ok(ClusteredIndex { name, asc })
65576539
})?;
65586540

65596541
self.expect_token(&Token::RParen)?;
65606542

65616543
Ok(SqlOption::Clustered(TableOptionsClustered::Index(columns)))
6544+
} else {
6545+
Err(ParserError::ParserError(
6546+
"invalid CLUSTERED sequence".to_string(),
6547+
))
65626548
}
65636549
}
65646550

6565-
pub fn parse_table_option_partition(&mut self) -> Result<SqlOption, ParserError> {
6551+
pub fn parse_option_partition(&mut self) -> Result<SqlOption, ParserError> {
65666552
self.expect_keyword(Keyword::PARTITION)?;
65676553
self.expect_token(&Token::LParen)?;
65686554
let column_name = self.parse_identifier(false)?;
@@ -11083,17 +11069,23 @@ impl<'a> Parser<'a> {
1108311069
})
1108411070
}
1108511071

11086-
/// Parse an expression, optionally followed by ASC or DESC (used in ORDER BY)
11087-
pub fn parse_order_by_expr(&mut self) -> Result<OrderByExpr, ParserError> {
11088-
let expr = self.parse_expr()?;
11089-
11090-
let asc = if self.parse_keyword(Keyword::ASC) {
11072+
/// Parsae ASC or DESC, returns Option<true> if ASC or Option<false> if DESC. If token is not
11073+
/// one of ASC or DESC, `None` is returned.
11074+
pub fn parse_asc(&mut self) -> Option<bool> {
11075+
if self.parse_keyword(Keyword::ASC) {
1109111076
Some(true)
1109211077
} else if self.parse_keyword(Keyword::DESC) {
1109311078
Some(false)
1109411079
} else {
1109511080
None
11096-
};
11081+
}
11082+
}
11083+
11084+
/// Parse an expression, optionally followed by ASC or DESC (used in ORDER BY)
11085+
pub fn parse_order_by_expr(&mut self) -> Result<OrderByExpr, ParserError> {
11086+
let expr = self.parse_expr()?;
11087+
11088+
let asc = self.parse_asc();
1109711089

1109811090
let nulls_first = if self.parse_keywords(&[Keyword::NULLS, Keyword::FIRST]) {
1109911091
Some(true)

‎tests/sqlparser_bigquery.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ fn parse_create_view_with_options() {
269269
name: Ident::new("age"),
270270
data_type: None,
271271
options: Some(vec![SqlOption::KeyValue {
272-
name: Ident::new("description"),
272+
key: Ident::new("description"),
273273
value: Expr::Value(Value::DoubleQuotedString("field age".to_string())),
274274
}])
275275
},
@@ -289,7 +289,7 @@ fn parse_create_view_with_options() {
289289
};
290290
assert_eq!(
291291
&SqlOption::KeyValue {
292-
name: Ident::new("description"),
292+
key: Ident::new("description"),
293293
value: Expr::Value(Value::DoubleQuotedString(
294294
"a view that expires in 2 days".to_string()
295295
)),
@@ -416,7 +416,7 @@ fn parse_create_table_with_options() {
416416
ColumnOptionDef {
417417
name: None,
418418
option: ColumnOption::Options(vec![SqlOption::KeyValue {
419-
name: Ident::new("description"),
419+
key: Ident::new("description"),
420420
value: Expr::Value(Value::DoubleQuotedString(
421421
"field x".to_string()
422422
)),
@@ -431,7 +431,7 @@ fn parse_create_table_with_options() {
431431
options: vec![ColumnOptionDef {
432432
name: None,
433433
option: ColumnOption::Options(vec![SqlOption::KeyValue {
434-
name: Ident::new("description"),
434+
key: Ident::new("description"),
435435
value: Expr::Value(Value::DoubleQuotedString(
436436
"field y".to_string()
437437
)),
@@ -450,11 +450,11 @@ fn parse_create_table_with_options() {
450450
])),
451451
Some(vec![
452452
SqlOption::KeyValue {
453-
name: Ident::new("partition_expiration_days"),
453+
key: Ident::new("partition_expiration_days"),
454454
value: Expr::Value(number("1")),
455455
},
456456
SqlOption::KeyValue {
457-
name: Ident::new("description"),
457+
key: Ident::new("description"),
458458
value: Expr::Value(Value::DoubleQuotedString(
459459
"table option description".to_string()
460460
)),
@@ -2011,7 +2011,7 @@ fn test_bigquery_create_function() {
20112011
"42"
20122012
)))),
20132013
options: Some(vec![SqlOption::KeyValue {
2014-
name: Ident::new("x"),
2014+
key: Ident::new("x"),
20152015
value: Expr::Value(Value::SingleQuotedString("y".into())),
20162016
}]),
20172017
behavior: None,

‎tests/sqlparser_common.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3638,11 +3638,11 @@ fn parse_create_table_with_options() {
36383638
assert_eq!(
36393639
vec![
36403640
SqlOption::KeyValue {
3641-
name: "foo".into(),
3641+
key: "foo".into(),
36423642
value: Expr::Value(Value::SingleQuotedString("bar".into())),
36433643
},
36443644
SqlOption::KeyValue {
3645-
name: "a".into(),
3645+
key: "a".into(),
36463646
value: Expr::Value(number("123")),
36473647
},
36483648
],
@@ -3871,7 +3871,7 @@ fn parse_alter_table() {
38713871
assert_eq!(
38723872
table_properties,
38733873
[SqlOption::KeyValue {
3874-
name: Ident {
3874+
key: Ident {
38753875
value: "classification".to_string(),
38763876
quote_style: Some('\'')
38773877
},
@@ -3959,11 +3959,11 @@ fn parse_alter_view_with_options() {
39593959
assert_eq!(
39603960
vec![
39613961
SqlOption::KeyValue {
3962-
name: "foo".into(),
3962+
key: "foo".into(),
39633963
value: Expr::Value(Value::SingleQuotedString("bar".into())),
39643964
},
39653965
SqlOption::KeyValue {
3966-
name: "a".into(),
3966+
key: "a".into(),
39673967
value: Expr::Value(number("123")),
39683968
},
39693969
],
@@ -6531,11 +6531,11 @@ fn parse_create_view_with_options() {
65316531
assert_eq!(
65326532
CreateTableOptions::With(vec![
65336533
SqlOption::KeyValue {
6534-
name: "foo".into(),
6534+
key: "foo".into(),
65356535
value: Expr::Value(Value::SingleQuotedString("bar".into())),
65366536
},
65376537
SqlOption::KeyValue {
6538-
name: "a".into(),
6538+
key: "a".into(),
65396539
value: Expr::Value(number("123")),
65406540
},
65416541
]),
@@ -8629,11 +8629,11 @@ fn parse_cache_table() {
86298629
has_as: false,
86308630
options: vec![
86318631
SqlOption::KeyValue {
8632-
name: Ident::with_quote('\'', "K1"),
8632+
key: Ident::with_quote('\'', "K1"),
86338633
value: Expr::Value(Value::SingleQuotedString("V1".into())),
86348634
},
86358635
SqlOption::KeyValue {
8636-
name: Ident::with_quote('\'', "K2"),
8636+
key: Ident::with_quote('\'', "K2"),
86378637
value: Expr::Value(number("0.88")),
86388638
},
86398639
],
@@ -8654,11 +8654,11 @@ fn parse_cache_table() {
86548654
has_as: false,
86558655
options: vec![
86568656
SqlOption::KeyValue {
8657-
name: Ident::with_quote('\'', "K1"),
8657+
key: Ident::with_quote('\'', "K1"),
86588658
value: Expr::Value(Value::SingleQuotedString("V1".into())),
86598659
},
86608660
SqlOption::KeyValue {
8661-
name: Ident::with_quote('\'', "K2"),
8661+
key: Ident::with_quote('\'', "K2"),
86628662
value: Expr::Value(number("0.88")),
86638663
},
86648664
],
@@ -8679,11 +8679,11 @@ fn parse_cache_table() {
86798679
has_as: true,
86808680
options: vec![
86818681
SqlOption::KeyValue {
8682-
name: Ident::with_quote('\'', "K1"),
8682+
key: Ident::with_quote('\'', "K1"),
86838683
value: Expr::Value(Value::SingleQuotedString("V1".into())),
86848684
},
86858685
SqlOption::KeyValue {
8686-
name: Ident::with_quote('\'', "K2"),
8686+
key: Ident::with_quote('\'', "K2"),
86878687
value: Expr::Value(number("0.88")),
86888688
},
86898689
],
@@ -9490,7 +9490,7 @@ fn parse_unload() {
94909490
quote_style: Some('\'')
94919491
},
94929492
with: vec![SqlOption::KeyValue {
9493-
name: Ident {
9493+
key: Ident {
94949494
value: "format".to_string(),
94959495
quote_style: None
94969496
},

0 commit comments

Comments
 (0)