From 815ee5bcb36910a1ad28cdde1a53a6aa8c5e1cdc Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Wed, 3 Jul 2024 11:07:47 -0400 Subject: [PATCH 01/10] Add support for WITH FILL to OrderByExpr ClickHouse supports the ORDER BY ... WITH FILL modifier: https://clickhouse.com/docs/en/sql-reference/statements/select/order-by#order-by-expr-with-fill-modifier WITH FILL itself supports a simple "from", "to", and "step" parameters, and a more sophisticated INTERPOLATE option. --- src/ast/mod.rs | 16 +++--- src/ast/query.rs | 59 ++++++++++++++++++++ src/keywords.rs | 3 ++ src/parser/mod.rs | 62 +++++++++++++++++++++ tests/sqlparser_common.rs | 111 ++++++++++++++++++++++++++++++++++++++ tests/sqlparser_mysql.rs | 1 + 6 files changed, 244 insertions(+), 8 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index c7f461418..e9b0e61a4 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -43,14 +43,14 @@ pub use self::operator::{BinaryOperator, UnaryOperator}; pub use self::query::{ AfterMatchSkip, ConnectBy, Cte, CteAsMaterialized, Distinct, EmptyMatchesMode, ExceptSelectItem, ExcludeSelectItem, ExprWithAlias, Fetch, ForClause, ForJson, ForXml, - GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Join, JoinConstraint, - JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, LockClause, LockType, - MatchRecognizePattern, MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, - NonBlock, Offset, OffsetRows, OrderByExpr, PivotValueSource, Query, RenameSelectItem, - RepetitionQuantifier, ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, - SelectInto, SelectItem, SetExpr, SetOperator, SetQuantifier, SymbolDefinition, Table, - TableAlias, TableFactor, TableVersion, TableWithJoins, Top, TopQuantity, ValueTableMode, - Values, WildcardAdditionalOptions, With, + GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolation, Join, + JoinConstraint, JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, + LockClause, LockType, MatchRecognizePattern, MatchRecognizeSymbol, Measure, + NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, OffsetRows, OrderByExpr, + PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, ReplaceSelectElement, + ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, SetOperator, + SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, TableWithJoins, + Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, WithFill, }; pub use self::value::{ escape_double_quote_string, escape_quoted_string, DateTimeField, DollarQuotedString, diff --git a/src/ast/query.rs b/src/ast/query.rs index d00a0dfcc..75957a789 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -1627,6 +1627,9 @@ pub struct OrderByExpr { pub asc: Option, /// Optional `NULLS FIRST` or `NULLS LAST` pub nulls_first: Option, + /// Optional: `WITH FILL` + /// Supported by [ClickHouse syntax]: + pub with_fill: Option, } impl fmt::Display for OrderByExpr { @@ -1642,6 +1645,62 @@ impl fmt::Display for OrderByExpr { Some(false) => write!(f, " NULLS LAST")?, None => (), } + if let Some(ref with_fill) = self.with_fill { + write!(f, " {}", with_fill)? + } + Ok(()) + } +} + +/// ClickHouse `WITH FILL` modifier for `ORDER BY` clause. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct WithFill { + pub from: Option, + pub to: Option, + pub step: Option, + pub interpolate: Vec, +} + +impl fmt::Display for WithFill { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "WITH FILL")?; + if let Some(ref from) = self.from { + write!(f, " FROM {}", from)?; + } + if let Some(ref to) = self.to { + write!(f, " TO {}", to)?; + } + if let Some(ref step) = self.step { + write!(f, " STEP {}", step)?; + } + if !self.interpolate.is_empty() { + write!( + f, + " INTERPOLATE ({})", + display_comma_separated(&self.interpolate) + )?; + } + Ok(()) + } +} + +/// ClickHouse `INTERPOLATE` clause for use in `WITH FILL` modifier. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct Interpolation { + pub column: Expr, + pub formula: Option, +} + +impl fmt::Display for Interpolation { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.column)?; + if let Some(ref formula) = self.formula { + write!(f, " AS {}", formula)?; + } Ok(()) } } diff --git a/src/keywords.rs b/src/keywords.rs index 5db55e9da..b4ae2a2cf 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -297,6 +297,7 @@ define_keywords!( FILE, FILES, FILE_FORMAT, + FILL, FILTER, FIRST, FIRST_VALUE, @@ -382,6 +383,7 @@ define_keywords!( INT64, INT8, INTEGER, + INTERPOLATE, INTERSECT, INTERSECTION, INTERVAL, @@ -678,6 +680,7 @@ define_keywords!( STDDEV_SAMP, STDIN, STDOUT, + STEP, STORAGE_INTEGRATION, STORED, STRICT, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 4e9c3836b..ffc70779c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -10408,13 +10408,75 @@ impl<'a> Parser<'a> { None }; + let with_fill = if self.parse_keywords(&[Keyword::WITH, Keyword::FILL]) { + Some(self.parse_with_fill()?) + } else { + None + }; + Ok(OrderByExpr { expr, asc, nulls_first, + with_fill, + }) + } + + // Parse a WITH FILL clause (ClickHouse dialect) + // that follow the WITH FILL keywords in a ORDER BY clause + pub fn parse_with_fill(&mut self) -> Result { + let from = if self.parse_keyword(Keyword::FROM) { + Some(self.parse_expr()?) + } else { + None + }; + + let to = if self.parse_keyword(Keyword::TO) { + Some(self.parse_expr()?) + } else { + None + }; + + let step = if self.parse_keyword(Keyword::STEP) { + Some(self.parse_expr()?) + } else { + None + }; + + let interpolate = + if self.parse_keyword(Keyword::INTERPOLATE) && self.consume_token(&Token::LParen) { + let interpolations = self.parse_interpolations()?; + self.expect_token(&Token::RParen)?; + interpolations + } else { + vec![] + }; + + Ok(WithFill { + from, + to, + step, + interpolate, }) } + // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) + // that follow the INTERPOLATE keyword in a WITH FILL clause + pub fn parse_interpolations(&mut self) -> Result, ParserError> { + self.parse_comma_separated(|p| p.parse_interpolation()) + } + + // Parse a INTERPOLATE expression (ClickHouse dialect) + pub fn parse_interpolation(&mut self) -> Result { + let column = self.parse_expr()?; + let formula = if self.parse_keyword(Keyword::AS) { + Some(self.parse_expr()?) + } else { + None + }; + Ok(Interpolation { column, formula }) + } + /// Parse a TOP clause, MSSQL equivalent of LIMIT, /// that follows after `SELECT [DISTINCT]`. pub fn parse_top(&mut self) -> Result { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index ac2133946..b4eca6491 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -2048,16 +2048,19 @@ fn parse_select_order_by() { expr: Expr::Identifier(Ident::new("lname")), asc: Some(true), nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("id")), asc: None, nulls_first: None, + with_fill: None, }, ], select.order_by @@ -2080,11 +2083,13 @@ fn parse_select_order_by_limit() { expr: Expr::Identifier(Ident::new("lname")), asc: Some(true), nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, + with_fill: None, }, ], select.order_by @@ -2103,11 +2108,13 @@ fn parse_select_order_by_nulls_order() { expr: Expr::Identifier(Ident::new("lname")), asc: Some(true), nulls_first: Some(true), + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: Some(false), + with_fill: None, }, ], select.order_by @@ -2115,6 +2122,100 @@ fn parse_select_order_by_nulls_order() { assert_eq!(Some(Expr::Value(number("2"))), select.limit); } +#[test] +fn parse_select_order_by_with_fill() { + let sql = "SELECT id, fname, lname FROM customer WHERE id < 5 \ + ORDER BY lname ASC WITH FILL, \ + fname DESC NULLS LAST WITH FILL FROM 10 TO 20 STEP 2 INTERPOLATE (col1 AS col1 + 1) LIMIT 2"; + let select = verified_query(sql); + assert_eq!( + vec![ + OrderByExpr { + expr: Expr::Identifier(Ident::new("lname")), + asc: Some(true), + nulls_first: None, + with_fill: Some(WithFill { + from: None, + to: None, + step: None, + interpolate: vec![], + }), + }, + OrderByExpr { + expr: Expr::Identifier(Ident::new("fname")), + asc: Some(false), + nulls_first: Some(false), + with_fill: Some(WithFill { + from: Some(Expr::Value(number("10"))), + to: Some(Expr::Value(number("20"))), + step: Some(Expr::Value(number("2"))), + interpolate: vec![Interpolation { + column: Expr::Identifier(Ident::new("col1")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }] + }), + }, + ], + select.order_by + ); + assert_eq!(Some(Expr::Value(number("2"))), select.limit); +} + +#[test] +fn parse_with_fill() { + let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL FROM 10 TO 20 STEP 2 INTERPOLATE (col1 AS col1 + 1, col2 AS col3)"; + let select = verified_query(sql); + assert_eq!( + Some(WithFill { + from: Some(Expr::Value(number("10"))), + to: Some(Expr::Value(number("20"))), + step: Some(Expr::Value(number("2"))), + interpolate: vec![ + Interpolation { + column: Expr::Identifier(Ident::new("col1")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }, + Interpolation { + column: Expr::Identifier(Ident::new("col2")), + formula: Some(Expr::Identifier(Ident::new("col3"))), + }, + ] + }), + select.order_by[0].with_fill + ); +} + +#[test] +fn parse_interpolation() { + let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE (col1 AS col1 + 1, col2 AS col3)"; + let select = verified_query(sql); + assert_eq!( + vec![ + Interpolation { + column: Expr::Identifier(Ident::new("col1")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }, + Interpolation { + column: Expr::Identifier(Ident::new("col2")), + formula: Some(Expr::Identifier(Ident::new("col3"))), + }, + ], + select.order_by[0].with_fill.as_ref().unwrap().interpolate + ); +} + #[test] fn parse_select_group_by() { let sql = "SELECT id, fname, lname FROM customer GROUP BY lname, fname"; @@ -2202,6 +2303,7 @@ fn parse_select_qualify() { expr: Expr::Identifier(Ident::new("o")), asc: None, nulls_first: None, + with_fill: None, }], window_frame: None, })), @@ -2562,6 +2664,7 @@ fn parse_listagg() { }), asc: None, nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident { @@ -2570,6 +2673,7 @@ fn parse_listagg() { }), asc: None, nulls_first: None, + with_fill: None, }, ] }), @@ -4363,6 +4467,7 @@ fn parse_window_functions() { expr: Expr::Identifier(Ident::new("dt")), asc: Some(false), nulls_first: None, + with_fill: None, }], window_frame: None, })), @@ -4570,6 +4675,7 @@ fn test_parse_named_window() { }), asc: None, nulls_first: None, + with_fill: None, }], window_frame: None, }), @@ -7254,11 +7360,13 @@ fn parse_create_index() { expr: Expr::Identifier(Ident::new("name")), asc: None, nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, + with_fill: None, }, ]; match verified_stmt(sql) { @@ -7288,11 +7396,13 @@ fn test_create_index_with_using_function() { expr: Expr::Identifier(Ident::new("name")), asc: None, nulls_first: None, + with_fill: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, + with_fill: None, }, ]; match verified_stmt(sql) { @@ -9547,6 +9657,7 @@ fn test_match_recognize() { expr: Expr::Identifier(Ident::new("price_date")), asc: None, nulls_first: None, + with_fill: None, }], measures: vec![ Measure { diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 4c18d4a75..3475f5013 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -1883,6 +1883,7 @@ fn parse_delete_with_order_by() { }), asc: Some(false), nulls_first: None, + with_fill: None, }], order_by ); From 7022e4227fe00ae4914688ad91e355effd8a159b Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Fri, 5 Jul 2024 10:09:49 -0400 Subject: [PATCH 02/10] Refactor INTERPOLATE and add tests --- src/ast/mod.rs | 17 ++--- src/ast/query.rs | 39 +++++++++--- src/parser/mod.rs | 45 +++++++++----- tests/sqlparser_clickhouse.rs | 113 ++++++++++++++++++++++++++++++++++ tests/sqlparser_common.rs | 111 +++++---------------------------- tests/sqlparser_mysql.rs | 1 + 6 files changed, 198 insertions(+), 128 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index e9b0e61a4..d664f40c0 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -43,14 +43,15 @@ pub use self::operator::{BinaryOperator, UnaryOperator}; pub use self::query::{ AfterMatchSkip, ConnectBy, Cte, CteAsMaterialized, Distinct, EmptyMatchesMode, ExceptSelectItem, ExcludeSelectItem, ExprWithAlias, Fetch, ForClause, ForJson, ForXml, - GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolation, Join, - JoinConstraint, JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, - LockClause, LockType, MatchRecognizePattern, MatchRecognizeSymbol, Measure, - NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, OffsetRows, OrderByExpr, - PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, ReplaceSelectElement, - ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, SetOperator, - SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, TableWithJoins, - Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, WithFill, + GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolation, + InterpolationArg, Join, JoinConstraint, JoinOperator, JsonTableColumn, + JsonTableColumnErrorHandling, LateralView, LockClause, LockType, MatchRecognizePattern, + MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, + OffsetRows, OrderByExpr, PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, + ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, + SetOperator, SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, + TableWithJoins, Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, + WithFill, }; pub use self::value::{ escape_double_quote_string, escape_quoted_string, DateTimeField, DollarQuotedString, diff --git a/src/ast/query.rs b/src/ast/query.rs index 75957a789..1a7e7cd11 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -1630,6 +1630,11 @@ pub struct OrderByExpr { /// Optional: `WITH FILL` /// Supported by [ClickHouse syntax]: pub with_fill: Option, + /// Optional: `INTERPOLATE` + /// Supported by [ClickHouse syntax] + /// + /// [ClickHouse syntax]: + pub interpolate: Option, } impl fmt::Display for OrderByExpr { @@ -1648,11 +1653,23 @@ impl fmt::Display for OrderByExpr { if let Some(ref with_fill) = self.with_fill { write!(f, " {}", with_fill)? } + if let Some(ref interpolate) = self.interpolate { + match interpolate { + InterpolationArg::NoBody => write!(f, " INTERPOLATE")?, + InterpolationArg::EmptyBody => write!(f, " INTERPOLATE ()")?, + InterpolationArg::Columns(columns) => { + write!(f, " INTERPOLATE ({})", display_comma_separated(columns))?; + } + } + } Ok(()) } } /// ClickHouse `WITH FILL` modifier for `ORDER BY` clause. +/// Supported by [ClickHouse syntax] +/// +/// [ClickHouse syntax]: #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] @@ -1660,7 +1677,6 @@ pub struct WithFill { pub from: Option, pub to: Option, pub step: Option, - pub interpolate: Vec, } impl fmt::Display for WithFill { @@ -1675,18 +1691,14 @@ impl fmt::Display for WithFill { if let Some(ref step) = self.step { write!(f, " STEP {}", step)?; } - if !self.interpolate.is_empty() { - write!( - f, - " INTERPOLATE ({})", - display_comma_separated(&self.interpolate) - )?; - } Ok(()) } } -/// ClickHouse `INTERPOLATE` clause for use in `WITH FILL` modifier. +/// ClickHouse `INTERPOLATE` clause for use in `ORDER BY` clause when using `WITH FILL` modifier. +/// Supported by [ClickHouse syntax] +/// +/// [ClickHouse syntax]: #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] @@ -1695,6 +1707,15 @@ pub struct Interpolation { pub formula: Option, } +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum InterpolationArg { + NoBody, + EmptyBody, + Columns(Vec), +} + impl fmt::Display for Interpolation { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.column)?; diff --git a/src/parser/mod.rs b/src/parser/mod.rs index ffc70779c..2906c5a8c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -10408,17 +10408,42 @@ impl<'a> Parser<'a> { None }; - let with_fill = if self.parse_keywords(&[Keyword::WITH, Keyword::FILL]) { + let with_fill = if dialect_of!(self is ClickHouseDialect | GenericDialect) + && self.parse_keywords(&[Keyword::WITH, Keyword::FILL]) + { Some(self.parse_with_fill()?) } else { None }; + let interpolate = if dialect_of!(self is ClickHouseDialect | GenericDialect) + && self.parse_keyword(Keyword::INTERPOLATE) + { + if self.consume_token(&Token::LParen) { + if self.peek_token().token == Token::RParen { + // INTERPOLATE () + self.next_token(); + Some(InterpolationArg::EmptyBody) + } else { + // INTERPOLATE ( ... ) + let interpolations = self.parse_interpolations()?; + self.expect_token(&Token::RParen)?; + Some(InterpolationArg::Columns(interpolations)) + } + } else { + // INTERPOLATE + Some(InterpolationArg::NoBody) + } + } else { + None + }; + Ok(OrderByExpr { expr, asc, nulls_first, with_fill, + interpolate, }) } @@ -10443,25 +10468,11 @@ impl<'a> Parser<'a> { None }; - let interpolate = - if self.parse_keyword(Keyword::INTERPOLATE) && self.consume_token(&Token::LParen) { - let interpolations = self.parse_interpolations()?; - self.expect_token(&Token::RParen)?; - interpolations - } else { - vec![] - }; - - Ok(WithFill { - from, - to, - step, - interpolate, - }) + Ok(WithFill { from, to, step }) } // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) - // that follow the INTERPOLATE keyword in a WITH FILL clause + // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier pub fn parse_interpolations(&mut self) -> Result, ParserError> { self.parse_comma_separated(|p| p.parse_interpolation()) } diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 0c188a24b..f5b0b5720 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -681,6 +681,119 @@ fn parse_group_by_with_modifier() { } } +#[test] +fn parse_select_order_by_with_fill_interpolate() { + let sql = "SELECT id, fname, lname FROM customer WHERE id < 5 \ + ORDER BY \ + fname ASC NULLS FIRST WITH FILL FROM 10 TO 20 STEP 2, \ + lname DESC NULLS LAST WITH FILL FROM 30 TO 40 STEP 3 \ + INTERPOLATE (col1 AS col1 + 1) \ + LIMIT 2"; + let select = clickhouse().verified_query(sql); + assert_eq!( + vec![ + OrderByExpr { + expr: Expr::Identifier(Ident::new("fname")), + asc: Some(true), + nulls_first: Some(true), + with_fill: Some(WithFill { + from: Some(Expr::Value(number("10"))), + to: Some(Expr::Value(number("20"))), + step: Some(Expr::Value(number("2"))), + }), + interpolate: None, + }, + OrderByExpr { + expr: Expr::Identifier(Ident::new("lname")), + asc: Some(false), + nulls_first: Some(false), + with_fill: Some(WithFill { + from: Some(Expr::Value(number("30"))), + to: Some(Expr::Value(number("40"))), + step: Some(Expr::Value(number("3"))), + }), + interpolate: Some(InterpolationArg::Columns(vec![Interpolation { + column: Expr::Identifier(Ident::new("col1")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }])) + }, + ], + select.order_by + ); + assert_eq!(Some(Expr::Value(number("2"))), select.limit); +} + +#[test] +fn parse_with_fill() { + let sql = "SELECT fname FROM customer \ + ORDER BY fname WITH FILL FROM 10 TO 20 STEP 2"; + let select = clickhouse().verified_query(sql); + assert_eq!( + Some(WithFill { + from: Some(Expr::Value(number("10"))), + to: Some(Expr::Value(number("20"))), + step: Some(Expr::Value(number("2"))), + }), + select.order_by[0].with_fill + ); +} + +#[test] +fn parse_interpolation_body_with_columns() { + let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL \ + INTERPOLATE (col1 AS col1 + 1, col2 AS col3, col4 AS col4 + 4)"; + let select = clickhouse().verified_query(sql); + assert_eq!( + Some(InterpolationArg::Columns(vec![ + Interpolation { + column: Expr::Identifier(Ident::new("col1")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }, + Interpolation { + column: Expr::Identifier(Ident::new("col2")), + formula: Some(Expr::Identifier(Ident::new("col3"))), + }, + Interpolation { + column: Expr::Identifier(Ident::new("col4")), + formula: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col4"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("4"))), + }), + }, + ])), + select.order_by[0].interpolate + ); +} + +#[test] +fn parse_interpolation_without_body() { + let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE"; + let select = clickhouse().verified_query(sql); + assert_eq!( + Some(InterpolationArg::NoBody), + select.order_by[0].interpolate + ); +} + +#[test] +fn parse_interpolation_with_empty_body() { + let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE ()"; + let select = clickhouse().verified_query(sql); + assert_eq!( + Some(InterpolationArg::EmptyBody), + select.order_by[0].interpolate + ); +} + fn clickhouse() -> TestedDialects { TestedDialects { dialects: vec![Box::new(ClickHouseDialect {})], diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index b4eca6491..64485f32e 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -2049,18 +2049,21 @@ fn parse_select_order_by() { asc: Some(true), nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("id")), asc: None, nulls_first: None, with_fill: None, + interpolate: None, }, ], select.order_by @@ -2084,12 +2087,14 @@ fn parse_select_order_by_limit() { asc: Some(true), nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }, ], select.order_by @@ -2109,12 +2114,14 @@ fn parse_select_order_by_nulls_order() { asc: Some(true), nulls_first: Some(true), with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: Some(false), with_fill: None, + interpolate: None, }, ], select.order_by @@ -2122,100 +2129,6 @@ fn parse_select_order_by_nulls_order() { assert_eq!(Some(Expr::Value(number("2"))), select.limit); } -#[test] -fn parse_select_order_by_with_fill() { - let sql = "SELECT id, fname, lname FROM customer WHERE id < 5 \ - ORDER BY lname ASC WITH FILL, \ - fname DESC NULLS LAST WITH FILL FROM 10 TO 20 STEP 2 INTERPOLATE (col1 AS col1 + 1) LIMIT 2"; - let select = verified_query(sql); - assert_eq!( - vec![ - OrderByExpr { - expr: Expr::Identifier(Ident::new("lname")), - asc: Some(true), - nulls_first: None, - with_fill: Some(WithFill { - from: None, - to: None, - step: None, - interpolate: vec![], - }), - }, - OrderByExpr { - expr: Expr::Identifier(Ident::new("fname")), - asc: Some(false), - nulls_first: Some(false), - with_fill: Some(WithFill { - from: Some(Expr::Value(number("10"))), - to: Some(Expr::Value(number("20"))), - step: Some(Expr::Value(number("2"))), - interpolate: vec![Interpolation { - column: Expr::Identifier(Ident::new("col1")), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }] - }), - }, - ], - select.order_by - ); - assert_eq!(Some(Expr::Value(number("2"))), select.limit); -} - -#[test] -fn parse_with_fill() { - let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL FROM 10 TO 20 STEP 2 INTERPOLATE (col1 AS col1 + 1, col2 AS col3)"; - let select = verified_query(sql); - assert_eq!( - Some(WithFill { - from: Some(Expr::Value(number("10"))), - to: Some(Expr::Value(number("20"))), - step: Some(Expr::Value(number("2"))), - interpolate: vec![ - Interpolation { - column: Expr::Identifier(Ident::new("col1")), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }, - Interpolation { - column: Expr::Identifier(Ident::new("col2")), - formula: Some(Expr::Identifier(Ident::new("col3"))), - }, - ] - }), - select.order_by[0].with_fill - ); -} - -#[test] -fn parse_interpolation() { - let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE (col1 AS col1 + 1, col2 AS col3)"; - let select = verified_query(sql); - assert_eq!( - vec![ - Interpolation { - column: Expr::Identifier(Ident::new("col1")), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }, - Interpolation { - column: Expr::Identifier(Ident::new("col2")), - formula: Some(Expr::Identifier(Ident::new("col3"))), - }, - ], - select.order_by[0].with_fill.as_ref().unwrap().interpolate - ); -} - #[test] fn parse_select_group_by() { let sql = "SELECT id, fname, lname FROM customer GROUP BY lname, fname"; @@ -2304,6 +2217,7 @@ fn parse_select_qualify() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }], window_frame: None, })), @@ -2665,6 +2579,7 @@ fn parse_listagg() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident { @@ -2674,6 +2589,7 @@ fn parse_listagg() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }, ] }), @@ -4468,6 +4384,7 @@ fn parse_window_functions() { asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }], window_frame: None, })), @@ -4676,6 +4593,7 @@ fn test_parse_named_window() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }], window_frame: None, }), @@ -7361,12 +7279,14 @@ fn parse_create_index() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }, ]; match verified_stmt(sql) { @@ -7397,12 +7317,14 @@ fn test_create_index_with_using_function() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }, ]; match verified_stmt(sql) { @@ -9658,6 +9580,7 @@ fn test_match_recognize() { asc: None, nulls_first: None, with_fill: None, + interpolate: None, }], measures: vec![ Measure { diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 3475f5013..12e1169ec 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -1884,6 +1884,7 @@ fn parse_delete_with_order_by() { asc: Some(false), nulls_first: None, with_fill: None, + interpolate: None, }], order_by ); From 47ba442bdf6e39a5f7ae3626ca6f34ffb8986424 Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Fri, 5 Jul 2024 13:24:47 -0400 Subject: [PATCH 03/10] Simplify parsing, remove InterpolateArg::Empty --- src/ast/mod.rs | 17 ++++++++--------- src/ast/query.rs | 24 +++++++++++++----------- src/parser/mod.rs | 26 ++++++++++---------------- tests/sqlparser_clickhouse.rs | 31 ++++++++++++++----------------- 4 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index d664f40c0..0b3dd9fc8 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -43,15 +43,14 @@ pub use self::operator::{BinaryOperator, UnaryOperator}; pub use self::query::{ AfterMatchSkip, ConnectBy, Cte, CteAsMaterialized, Distinct, EmptyMatchesMode, ExceptSelectItem, ExcludeSelectItem, ExprWithAlias, Fetch, ForClause, ForJson, ForXml, - GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolation, - InterpolationArg, Join, JoinConstraint, JoinOperator, JsonTableColumn, - JsonTableColumnErrorHandling, LateralView, LockClause, LockType, MatchRecognizePattern, - MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, - OffsetRows, OrderByExpr, PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, - ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, - SetOperator, SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, - TableWithJoins, Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, - WithFill, + GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolate, InterpolateArg, + Join, JoinConstraint, JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, + LockClause, LockType, MatchRecognizePattern, MatchRecognizeSymbol, Measure, + NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, OffsetRows, OrderByExpr, + PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, ReplaceSelectElement, + ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, SetOperator, + SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, TableWithJoins, + Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, WithFill, }; pub use self::value::{ escape_double_quote_string, escape_quoted_string, DateTimeField, DollarQuotedString, diff --git a/src/ast/query.rs b/src/ast/query.rs index 1a7e7cd11..fb5d307da 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -1634,7 +1634,7 @@ pub struct OrderByExpr { /// Supported by [ClickHouse syntax] /// /// [ClickHouse syntax]: - pub interpolate: Option, + pub interpolate: Option, } impl fmt::Display for OrderByExpr { @@ -1655,10 +1655,13 @@ impl fmt::Display for OrderByExpr { } if let Some(ref interpolate) = self.interpolate { match interpolate { - InterpolationArg::NoBody => write!(f, " INTERPOLATE")?, - InterpolationArg::EmptyBody => write!(f, " INTERPOLATE ()")?, - InterpolationArg::Columns(columns) => { - write!(f, " INTERPOLATE ({})", display_comma_separated(columns))?; + InterpolateArg::NoBody => write!(f, " INTERPOLATE")?, + InterpolateArg::Columns(columns) => { + if columns.is_empty() { + write!(f, " INTERPOLATE ()")?; + } else { + write!(f, " INTERPOLATE ({})", display_comma_separated(columns))?; + } } } } @@ -1702,21 +1705,20 @@ impl fmt::Display for WithFill { #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct Interpolation { - pub column: Expr, +pub struct Interpolate { + pub column: Ident, pub formula: Option, } #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub enum InterpolationArg { +pub enum InterpolateArg { NoBody, - EmptyBody, - Columns(Vec), + Columns(Vec), } -impl fmt::Display for Interpolation { +impl fmt::Display for Interpolate { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.column)?; if let Some(ref formula) = self.formula { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 2906c5a8c..05b449205 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -10420,19 +10420,13 @@ impl<'a> Parser<'a> { && self.parse_keyword(Keyword::INTERPOLATE) { if self.consume_token(&Token::LParen) { - if self.peek_token().token == Token::RParen { - // INTERPOLATE () - self.next_token(); - Some(InterpolationArg::EmptyBody) - } else { - // INTERPOLATE ( ... ) - let interpolations = self.parse_interpolations()?; - self.expect_token(&Token::RParen)?; - Some(InterpolationArg::Columns(interpolations)) - } + let interpolations = self.parse_interpolations()?; + self.expect_token(&Token::RParen)?; + // INTERPOLATE () and INTERPOLATE ( ... ) variants + Some(InterpolateArg::Columns(interpolations)) } else { // INTERPOLATE - Some(InterpolationArg::NoBody) + Some(InterpolateArg::NoBody) } } else { None @@ -10473,19 +10467,19 @@ impl<'a> Parser<'a> { // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier - pub fn parse_interpolations(&mut self) -> Result, ParserError> { - self.parse_comma_separated(|p| p.parse_interpolation()) + pub fn parse_interpolations(&mut self) -> Result, ParserError> { + self.parse_comma_separated0(|p| p.parse_interpolation()) } // Parse a INTERPOLATE expression (ClickHouse dialect) - pub fn parse_interpolation(&mut self) -> Result { - let column = self.parse_expr()?; + pub fn parse_interpolation(&mut self) -> Result { + let column = self.parse_identifier(false)?; let formula = if self.parse_keyword(Keyword::AS) { Some(self.parse_expr()?) } else { None }; - Ok(Interpolation { column, formula }) + Ok(Interpolate { column, formula }) } /// Parse a TOP clause, MSSQL equivalent of LIMIT, diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index f5b0b5720..10a74456c 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -712,8 +712,8 @@ fn parse_select_order_by_with_fill_interpolate() { to: Some(Expr::Value(number("40"))), step: Some(Expr::Value(number("3"))), }), - interpolate: Some(InterpolationArg::Columns(vec![Interpolation { - column: Expr::Identifier(Ident::new("col1")), + interpolate: Some(InterpolateArg::Columns(vec![Interpolate { + column: Ident::new("col1"), formula: Some(Expr::BinaryOp { left: Box::new(Expr::Identifier(Ident::new("col1"))), op: BinaryOperator::Plus, @@ -743,26 +743,26 @@ fn parse_with_fill() { } #[test] -fn parse_interpolation_body_with_columns() { +fn parse_interpolate_body_with_columns() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL \ INTERPOLATE (col1 AS col1 + 1, col2 AS col3, col4 AS col4 + 4)"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(InterpolationArg::Columns(vec![ - Interpolation { - column: Expr::Identifier(Ident::new("col1")), + Some(InterpolateArg::Columns(vec![ + Interpolate { + column: Ident::new("col1"), formula: Some(Expr::BinaryOp { left: Box::new(Expr::Identifier(Ident::new("col1"))), op: BinaryOperator::Plus, right: Box::new(Expr::Value(number("1"))), }), }, - Interpolation { - column: Expr::Identifier(Ident::new("col2")), + Interpolate { + column: Ident::new("col2"), formula: Some(Expr::Identifier(Ident::new("col3"))), }, - Interpolation { - column: Expr::Identifier(Ident::new("col4")), + Interpolate { + column: Ident::new("col4"), formula: Some(Expr::BinaryOp { left: Box::new(Expr::Identifier(Ident::new("col4"))), op: BinaryOperator::Plus, @@ -775,21 +775,18 @@ fn parse_interpolation_body_with_columns() { } #[test] -fn parse_interpolation_without_body() { +fn parse_interpolate_without_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE"; let select = clickhouse().verified_query(sql); - assert_eq!( - Some(InterpolationArg::NoBody), - select.order_by[0].interpolate - ); + assert_eq!(Some(InterpolateArg::NoBody), select.order_by[0].interpolate); } #[test] -fn parse_interpolation_with_empty_body() { +fn parse_interpolate_with_empty_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE ()"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(InterpolationArg::EmptyBody), + Some(InterpolateArg::Columns(vec![])), select.order_by[0].interpolate ); } From 4068bb3890e1c2cce25c80e9671ab6d689398331 Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Fri, 5 Jul 2024 14:56:50 -0400 Subject: [PATCH 04/10] Add tests for invalid syntax --- src/parser/mod.rs | 33 +++++++++++++++++- tests/sqlparser_clickhouse.rs | 63 +++++++++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 05b449205..f8ba93be3 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7888,7 +7888,9 @@ impl<'a> Parser<'a> { let body = self.parse_boxed_query_body(0)?; let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) { - self.parse_comma_separated(Parser::parse_order_by_expr)? + let order_by_exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?; + self.validate_order_by_exprs_for_interpolate_and_with_fill(&order_by_exprs)?; + order_by_exprs } else { vec![] }; @@ -10465,6 +10467,35 @@ impl<'a> Parser<'a> { Ok(WithFill { from, to, step }) } + pub fn validate_order_by_exprs_for_interpolate_and_with_fill( + &mut self, + order_by_exprs: &Vec, + ) -> Result<(), ParserError> { + if dialect_of!(self is ClickHouseDialect | GenericDialect) { + let mut has_with_fill = false; + let mut has_interpolate = false; + for order_by_expr in order_by_exprs { + if order_by_expr.with_fill.is_some() { + has_with_fill = true; + } + if order_by_expr.interpolate.is_some() { + if has_interpolate { + return Err(ParserError::ParserError( + "Only the last ORDER BY expression can contain interpolate".to_string(), + )); + } + if !has_with_fill { + return Err(ParserError::ParserError( + "INTERPOLATE requires WITH FILL".to_string(), + )); + } + has_interpolate = true; + } + } + } + Ok(()) + } + // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier pub fn parse_interpolations(&mut self) -> Result, ParserError> { diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 10a74456c..d06965dbe 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -727,10 +727,51 @@ fn parse_select_order_by_with_fill_interpolate() { assert_eq!(Some(Expr::Value(number("2"))), select.limit); } +#[test] +fn parse_select_order_by_with_fill_interpolate_multi_interpolates() { + let sql = "SELECT id, fname, lname FROM customer ORDER BY fname WITH FILL \ + INTERPOLATE (col1 AS col1 + 1) INTERPOLATE (col2 AS col2 + 2)"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("ORDER BY only accepts a single INTERPOLATE clause"); +} + +#[test] +fn parse_select_order_by_with_fill_interpolate_multi_with_fill_interpolates() { + let sql = "SELECT id, fname, lname FROM customer \ + ORDER BY \ + fname WITH FILL INTERPOLATE (col1 AS col1 + 1), \ + lname WITH FILL INTERPOLATE (col2 AS col2 + 2)"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("ORDER BY only accepts a single INTERPOLATE clause"); +} + +#[test] +fn parse_select_order_by_interpolate_missing_with_fill() { + let sql = "SELECT id, fname, lname FROM customer \ + ORDER BY fname, lname \ + INTERPOLATE (col2 AS col2 + 2)"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("ORDER BY INTERPOLATE must have at least one WITH FILL"); +} + +#[test] +fn parse_select_order_by_interpolate_not_last() { + let sql = "SELECT id, fname, lname FROM customer \ + ORDER BY \ + fname INTERPOLATE (col2 AS col2 + 2), + lname"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("ORDER BY INTERPOLATE must be in the last position"); +} + #[test] fn parse_with_fill() { - let sql = "SELECT fname FROM customer \ - ORDER BY fname WITH FILL FROM 10 TO 20 STEP 2"; + let sql = "SELECT fname FROM customer ORDER BY fname \ + WITH FILL FROM 10 TO 20 STEP 2"; let select = clickhouse().verified_query(sql); assert_eq!( Some(WithFill { @@ -742,6 +783,24 @@ fn parse_with_fill() { ); } +#[test] +fn parse_with_fill_missing_single_argument() { + let sql = "SELECT id, fname, lname FROM customer ORDER BY \ + fname WITH FILL FROM TO 20"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("WITH FILL requires expressions for all arguments"); +} + +#[test] +fn parse_with_fill_multiple_incomplete_arguments() { + let sql = "SELECT id, fname, lname FROM customer ORDER BY \ + fname WITH FILL FROM TO 20, lname WITH FILL FROM TO STEP 1"; + clickhouse_and_generic() + .parse_sql_statements(sql) + .expect_err("WITH FILL requires expressions for all arguments"); +} + #[test] fn parse_interpolate_body_with_columns() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL \ From ec263d2b82f596b2f57b187ace865a707c345c92 Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Mon, 8 Jul 2024 09:56:49 -0400 Subject: [PATCH 05/10] Rename and refactor structure --- src/ast/mod.rs | 17 +++++---- src/ast/query.rs | 37 +++++++++--------- src/parser/mod.rs | 14 ++++--- tests/sqlparser_clickhouse.rs | 71 +++++++++++++++++++---------------- 4 files changed, 75 insertions(+), 64 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 0b3dd9fc8..262989460 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -43,14 +43,15 @@ pub use self::operator::{BinaryOperator, UnaryOperator}; pub use self::query::{ AfterMatchSkip, ConnectBy, Cte, CteAsMaterialized, Distinct, EmptyMatchesMode, ExceptSelectItem, ExcludeSelectItem, ExprWithAlias, Fetch, ForClause, ForJson, ForXml, - GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolate, InterpolateArg, - Join, JoinConstraint, JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, - LockClause, LockType, MatchRecognizePattern, MatchRecognizeSymbol, Measure, - NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, OffsetRows, OrderByExpr, - PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, ReplaceSelectElement, - ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, SetOperator, - SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, TableWithJoins, - Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, WithFill, + GroupByExpr, GroupByWithModifier, IdentWithAlias, IlikeSelectItem, Interpolate, + InterpolateExpr, Join, JoinConstraint, JoinOperator, JsonTableColumn, + JsonTableColumnErrorHandling, LateralView, LockClause, LockType, MatchRecognizePattern, + MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, + OffsetRows, OrderByExpr, PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, + ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, + SetOperator, SetQuantifier, SymbolDefinition, Table, TableAlias, TableFactor, TableVersion, + TableWithJoins, Top, TopQuantity, ValueTableMode, Values, WildcardAdditionalOptions, With, + WithFill, }; pub use self::value::{ escape_double_quote_string, escape_quoted_string, DateTimeField, DollarQuotedString, diff --git a/src/ast/query.rs b/src/ast/query.rs index fb5d307da..4924a3d69 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -1634,7 +1634,7 @@ pub struct OrderByExpr { /// Supported by [ClickHouse syntax] /// /// [ClickHouse syntax]: - pub interpolate: Option, + pub interpolate: Option, } impl fmt::Display for OrderByExpr { @@ -1654,15 +1654,17 @@ impl fmt::Display for OrderByExpr { write!(f, " {}", with_fill)? } if let Some(ref interpolate) = self.interpolate { - match interpolate { - InterpolateArg::NoBody => write!(f, " INTERPOLATE")?, - InterpolateArg::Columns(columns) => { - if columns.is_empty() { - write!(f, " INTERPOLATE ()")?; - } else { - write!(f, " INTERPOLATE ({})", display_comma_separated(columns))?; - } - } + match &interpolate.expr { + Some(exprs) => write!( + f, + " INTERPOLATE ({})", + exprs + .iter() + .map(std::string::ToString::to_string) + .collect::>() + .join(", ") + )?, + None => write!(f, " INTERPOLATE")?, } } Ok(()) @@ -1705,24 +1707,23 @@ impl fmt::Display for WithFill { #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct Interpolate { +pub struct InterpolateExpr { pub column: Ident, - pub formula: Option, + pub expr: Option, } #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub enum InterpolateArg { - NoBody, - Columns(Vec), +pub struct Interpolate { + pub expr: Option>, } -impl fmt::Display for Interpolate { +impl fmt::Display for InterpolateExpr { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.column)?; - if let Some(ref formula) = self.formula { - write!(f, " AS {}", formula)?; + if let Some(ref expr) = self.expr { + write!(f, " AS {}", expr)?; } Ok(()) } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index f8ba93be3..31e33b7d8 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -10425,10 +10425,12 @@ impl<'a> Parser<'a> { let interpolations = self.parse_interpolations()?; self.expect_token(&Token::RParen)?; // INTERPOLATE () and INTERPOLATE ( ... ) variants - Some(InterpolateArg::Columns(interpolations)) + Some(Interpolate { + expr: Some(interpolations), + }) } else { // INTERPOLATE - Some(InterpolateArg::NoBody) + Some(Interpolate { expr: None }) } } else { None @@ -10498,19 +10500,19 @@ impl<'a> Parser<'a> { // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier - pub fn parse_interpolations(&mut self) -> Result, ParserError> { + pub fn parse_interpolations(&mut self) -> Result, ParserError> { self.parse_comma_separated0(|p| p.parse_interpolation()) } // Parse a INTERPOLATE expression (ClickHouse dialect) - pub fn parse_interpolation(&mut self) -> Result { + pub fn parse_interpolation(&mut self) -> Result { let column = self.parse_identifier(false)?; - let formula = if self.parse_keyword(Keyword::AS) { + let expr = if self.parse_keyword(Keyword::AS) { Some(self.parse_expr()?) } else { None }; - Ok(Interpolate { column, formula }) + Ok(InterpolateExpr { column, expr }) } /// Parse a TOP clause, MSSQL equivalent of LIMIT, diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index d06965dbe..e7974eed4 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -712,14 +712,16 @@ fn parse_select_order_by_with_fill_interpolate() { to: Some(Expr::Value(number("40"))), step: Some(Expr::Value(number("3"))), }), - interpolate: Some(InterpolateArg::Columns(vec![Interpolate { - column: Ident::new("col1"), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }])) + interpolate: Some(Interpolate { + expr: Some(vec![InterpolateExpr { + column: Ident::new("col1"), + expr: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }]) + }) }, ], select.order_by @@ -807,28 +809,30 @@ fn parse_interpolate_body_with_columns() { INTERPOLATE (col1 AS col1 + 1, col2 AS col3, col4 AS col4 + 4)"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(InterpolateArg::Columns(vec![ - Interpolate { - column: Ident::new("col1"), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }, - Interpolate { - column: Ident::new("col2"), - formula: Some(Expr::Identifier(Ident::new("col3"))), - }, - Interpolate { - column: Ident::new("col4"), - formula: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col4"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("4"))), - }), - }, - ])), + Some(Interpolate { + expr: Some(vec![ + InterpolateExpr { + column: Ident::new("col1"), + expr: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }, + InterpolateExpr { + column: Ident::new("col2"), + expr: Some(Expr::Identifier(Ident::new("col3"))), + }, + InterpolateExpr { + column: Ident::new("col4"), + expr: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col4"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("4"))), + }), + }, + ]) + }), select.order_by[0].interpolate ); } @@ -837,7 +841,10 @@ fn parse_interpolate_body_with_columns() { fn parse_interpolate_without_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE"; let select = clickhouse().verified_query(sql); - assert_eq!(Some(InterpolateArg::NoBody), select.order_by[0].interpolate); + assert_eq!( + Some(Interpolate { expr: None }), + select.order_by[0].interpolate + ); } #[test] @@ -845,7 +852,7 @@ fn parse_interpolate_with_empty_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE ()"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(InterpolateArg::Columns(vec![])), + Some(Interpolate { expr: Some(vec![]) }), select.order_by[0].interpolate ); } From 85fae63f9befbe7f1854d42cd519a95449bb0f2e Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Mon, 8 Jul 2024 13:14:54 -0400 Subject: [PATCH 06/10] Use display_comma_separated to print exprs --- src/ast/query.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/ast/query.rs b/src/ast/query.rs index c080f44a3..1884940d7 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -1684,15 +1684,7 @@ impl fmt::Display for OrderByExpr { } if let Some(ref interpolate) = self.interpolate { match &interpolate.expr { - Some(exprs) => write!( - f, - " INTERPOLATE ({})", - exprs - .iter() - .map(std::string::ToString::to_string) - .collect::>() - .join(", ") - )?, + Some(exprs) => write!(f, " INTERPOLATE ({})", display_comma_separated(exprs))?, None => write!(f, " INTERPOLATE")?, } } From 4a7167bd849cb849242f0f4b0362053235ca7727 Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Tue, 9 Jul 2024 08:52:18 -0400 Subject: [PATCH 07/10] Refactor to move Interpolate to Query.order_by --- src/ast/mod.rs | 10 ++-- src/ast/query.rs | 41 +++++++++------ src/parser/mod.rs | 92 ++++++++++++++-------------------- tests/sqlparser_clickhouse.rs | 93 ++++++++++++++++------------------- tests/sqlparser_common.rs | 53 ++++++++++---------- tests/sqlparser_mssql.rs | 10 +++- tests/sqlparser_mysql.rs | 76 ++++++++++++++++++++++------ tests/sqlparser_postgres.rs | 25 ++++++++-- 8 files changed, 227 insertions(+), 173 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 2a13eb3e4..9afe0ec69 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -47,11 +47,11 @@ pub use self::query::{ InterpolateExpr, Join, JoinConstraint, JoinOperator, JsonTableColumn, JsonTableColumnErrorHandling, LateralView, LockClause, LockType, MatchRecognizePattern, MatchRecognizeSymbol, Measure, NamedWindowDefinition, NamedWindowExpr, NonBlock, Offset, - OffsetRows, OrderByExpr, PivotValueSource, Query, RenameSelectItem, RepetitionQuantifier, - ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, SelectInto, SelectItem, SetExpr, - SetOperator, SetQuantifier, Setting, SymbolDefinition, Table, TableAlias, TableFactor, - TableVersion, TableWithJoins, Top, TopQuantity, ValueTableMode, Values, - WildcardAdditionalOptions, With, WithFill, + OffsetRows, OrderBy, OrderByExpr, PivotValueSource, Query, RenameSelectItem, + RepetitionQuantifier, ReplaceSelectElement, ReplaceSelectItem, RowsPerMatch, Select, + SelectInto, SelectItem, SetExpr, SetOperator, SetQuantifier, Setting, SymbolDefinition, Table, + TableAlias, TableFactor, TableVersion, TableWithJoins, Top, TopQuantity, ValueTableMode, + Values, WildcardAdditionalOptions, With, WithFill, }; pub use self::value::{ escape_double_quote_string, escape_quoted_string, DateTimeField, DollarQuotedString, diff --git a/src/ast/query.rs b/src/ast/query.rs index 1884940d7..03978b61f 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -33,7 +33,7 @@ pub struct Query { /// SELECT or UNION / EXCEPT / INTERSECT pub body: Box, /// ORDER BY - pub order_by: Vec, + pub order_by: OrderBy, /// `LIMIT { | ALL }` pub limit: Option, @@ -62,8 +62,18 @@ impl fmt::Display for Query { write!(f, "{with} ")?; } write!(f, "{}", self.body)?; - if !self.order_by.is_empty() { - write!(f, " ORDER BY {}", display_comma_separated(&self.order_by))?; + if !self.order_by.exprs.is_empty() { + write!( + f, + " ORDER BY {}", + display_comma_separated(&self.order_by.exprs) + )?; + if let Some(ref interpolate) = self.order_by.interpolate { + match &interpolate.exprs { + Some(exprs) => write!(f, " INTERPOLATE ({})", display_comma_separated(exprs))?, + None => write!(f, " INTERPOLATE")?, + } + } } if let Some(ref limit) = self.limit { write!(f, " LIMIT {limit}")?; @@ -1646,6 +1656,18 @@ pub enum JoinConstraint { None, } +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct OrderBy { + pub exprs: Vec, + /// Optional: `INTERPOLATE` + /// Supported by [ClickHouse syntax] + /// + /// [ClickHouse syntax]: + pub interpolate: Option, +} + /// An `ORDER BY` expression #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -1659,11 +1681,6 @@ pub struct OrderByExpr { /// Optional: `WITH FILL` /// Supported by [ClickHouse syntax]: pub with_fill: Option, - /// Optional: `INTERPOLATE` - /// Supported by [ClickHouse syntax] - /// - /// [ClickHouse syntax]: - pub interpolate: Option, } impl fmt::Display for OrderByExpr { @@ -1682,12 +1699,6 @@ impl fmt::Display for OrderByExpr { if let Some(ref with_fill) = self.with_fill { write!(f, " {}", with_fill)? } - if let Some(ref interpolate) = self.interpolate { - match &interpolate.expr { - Some(exprs) => write!(f, " INTERPOLATE ({})", display_comma_separated(exprs))?, - None => write!(f, " INTERPOLATE")?, - } - } Ok(()) } } @@ -1737,7 +1748,7 @@ pub struct InterpolateExpr { #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] pub struct Interpolate { - pub expr: Option>, + pub exprs: Option>, } impl fmt::Display for InterpolateExpr { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 6534c3347..81cb63395 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7890,7 +7890,10 @@ impl<'a> Parser<'a> { body: self.parse_insert_setexpr_boxed()?, limit: None, limit_by: vec![], - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, offset: None, fetch: None, locks: vec![], @@ -7903,7 +7906,10 @@ impl<'a> Parser<'a> { body: self.parse_update_setexpr_boxed()?, limit: None, limit_by: vec![], - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, offset: None, fetch: None, locks: vec![], @@ -7915,10 +7921,33 @@ impl<'a> Parser<'a> { let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) { let order_by_exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?; - self.validate_order_by_exprs_for_interpolate_and_with_fill(&order_by_exprs)?; - order_by_exprs + let interpolate = if dialect_of!(self is ClickHouseDialect | GenericDialect) + && self.parse_keyword(Keyword::INTERPOLATE) + { + if self.consume_token(&Token::LParen) { + let interpolations = self.parse_interpolations()?; + self.expect_token(&Token::RParen)?; + // INTERPOLATE () and INTERPOLATE ( ... ) variants + Some(Interpolate { + exprs: Some(interpolations), + }) + } else { + // INTERPOLATE + Some(Interpolate { exprs: None }) + } + } else { + None + }; + + OrderBy { + exprs: order_by_exprs, + interpolate, + } } else { - vec![] + OrderBy { + exprs: vec![], + interpolate: None, + } }; let mut limit = None; @@ -9136,7 +9165,10 @@ impl<'a> Parser<'a> { subquery: Box::new(Query { with: None, body: Box::new(values), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, limit: None, limit_by: vec![], offset: None, @@ -10464,30 +10496,11 @@ impl<'a> Parser<'a> { None }; - let interpolate = if dialect_of!(self is ClickHouseDialect | GenericDialect) - && self.parse_keyword(Keyword::INTERPOLATE) - { - if self.consume_token(&Token::LParen) { - let interpolations = self.parse_interpolations()?; - self.expect_token(&Token::RParen)?; - // INTERPOLATE () and INTERPOLATE ( ... ) variants - Some(Interpolate { - expr: Some(interpolations), - }) - } else { - // INTERPOLATE - Some(Interpolate { expr: None }) - } - } else { - None - }; - Ok(OrderByExpr { expr, asc, nulls_first, with_fill, - interpolate, }) } @@ -10515,35 +10528,6 @@ impl<'a> Parser<'a> { Ok(WithFill { from, to, step }) } - pub fn validate_order_by_exprs_for_interpolate_and_with_fill( - &mut self, - order_by_exprs: &Vec, - ) -> Result<(), ParserError> { - if dialect_of!(self is ClickHouseDialect | GenericDialect) { - let mut has_with_fill = false; - let mut has_interpolate = false; - for order_by_expr in order_by_exprs { - if order_by_expr.with_fill.is_some() { - has_with_fill = true; - } - if order_by_expr.interpolate.is_some() { - if has_interpolate { - return Err(ParserError::ParserError( - "Only the last ORDER BY expression can contain interpolate".to_string(), - )); - } - if !has_with_fill { - return Err(ParserError::ParserError( - "INTERPOLATE requires WITH FILL".to_string(), - )); - } - has_interpolate = true; - } - } - } - Ok(()) - } - // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier pub fn parse_interpolations(&mut self) -> Result, ParserError> { diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 250cfa723..d98291fa4 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -728,39 +728,40 @@ fn parse_select_order_by_with_fill_interpolate() { LIMIT 2"; let select = clickhouse().verified_query(sql); assert_eq!( - vec![ - OrderByExpr { - expr: Expr::Identifier(Ident::new("fname")), - asc: Some(true), - nulls_first: Some(true), - with_fill: Some(WithFill { - from: Some(Expr::Value(number("10"))), - to: Some(Expr::Value(number("20"))), - step: Some(Expr::Value(number("2"))), - }), - interpolate: None, - }, - OrderByExpr { - expr: Expr::Identifier(Ident::new("lname")), - asc: Some(false), - nulls_first: Some(false), - with_fill: Some(WithFill { - from: Some(Expr::Value(number("30"))), - to: Some(Expr::Value(number("40"))), - step: Some(Expr::Value(number("3"))), - }), - interpolate: Some(Interpolate { - expr: Some(vec![InterpolateExpr { - column: Ident::new("col1"), - expr: Some(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("col1"))), - op: BinaryOperator::Plus, - right: Box::new(Expr::Value(number("1"))), - }), - }]) - }) - }, - ], + OrderBy { + exprs: vec![ + OrderByExpr { + expr: Expr::Identifier(Ident::new("fname")), + asc: Some(true), + nulls_first: Some(true), + with_fill: Some(WithFill { + from: Some(Expr::Value(number("10"))), + to: Some(Expr::Value(number("20"))), + step: Some(Expr::Value(number("2"))), + }), + }, + OrderByExpr { + expr: Expr::Identifier(Ident::new("lname")), + asc: Some(false), + nulls_first: Some(false), + with_fill: Some(WithFill { + from: Some(Expr::Value(number("30"))), + to: Some(Expr::Value(number("40"))), + step: Some(Expr::Value(number("3"))), + }), + }, + ], + interpolate: Some(Interpolate { + exprs: Some(vec![InterpolateExpr { + column: Ident::new("col1"), + expr: Some(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("col1"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("1"))), + }), + }]) + }) + }, select.order_by ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); @@ -786,16 +787,6 @@ fn parse_select_order_by_with_fill_interpolate_multi_with_fill_interpolates() { .expect_err("ORDER BY only accepts a single INTERPOLATE clause"); } -#[test] -fn parse_select_order_by_interpolate_missing_with_fill() { - let sql = "SELECT id, fname, lname FROM customer \ - ORDER BY fname, lname \ - INTERPOLATE (col2 AS col2 + 2)"; - clickhouse_and_generic() - .parse_sql_statements(sql) - .expect_err("ORDER BY INTERPOLATE must have at least one WITH FILL"); -} - #[test] fn parse_select_order_by_interpolate_not_last() { let sql = "SELECT id, fname, lname FROM customer \ @@ -818,7 +809,7 @@ fn parse_with_fill() { to: Some(Expr::Value(number("20"))), step: Some(Expr::Value(number("2"))), }), - select.order_by[0].with_fill + select.order_by.exprs[0].with_fill ); } @@ -847,7 +838,7 @@ fn parse_interpolate_body_with_columns() { let select = clickhouse().verified_query(sql); assert_eq!( Some(Interpolate { - expr: Some(vec![ + exprs: Some(vec![ InterpolateExpr { column: Ident::new("col1"), expr: Some(Expr::BinaryOp { @@ -870,7 +861,7 @@ fn parse_interpolate_body_with_columns() { }, ]) }), - select.order_by[0].interpolate + select.order_by.interpolate ); } @@ -879,8 +870,8 @@ fn parse_interpolate_without_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(Interpolate { expr: None }), - select.order_by[0].interpolate + Some(Interpolate { exprs: None }), + select.order_by.interpolate ); } @@ -889,8 +880,10 @@ fn parse_interpolate_with_empty_body() { let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL INTERPOLATE ()"; let select = clickhouse().verified_query(sql); assert_eq!( - Some(Interpolate { expr: Some(vec![]) }), - select.order_by[0].interpolate + Some(Interpolate { + exprs: Some(vec![]) + }), + select.order_by.interpolate ); } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 5f11a7c51..965168aa9 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -407,7 +407,10 @@ fn parse_update_set_from() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -2051,24 +2054,21 @@ fn parse_select_order_by() { asc: Some(true), nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("id")), asc: None, nulls_first: None, with_fill: None, - interpolate: None, }, ], - select.order_by + select.order_by.exprs ); } chk("SELECT id, fname, lname FROM customer WHERE id < 5 ORDER BY lname ASC, fname DESC, id"); @@ -2089,17 +2089,15 @@ fn parse_select_order_by_limit() { asc: Some(true), nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }, ], - select.order_by + select.order_by.exprs ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); } @@ -2116,17 +2114,15 @@ fn parse_select_order_by_nulls_order() { asc: Some(true), nulls_first: Some(true), with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("fname")), asc: Some(false), nulls_first: Some(false), with_fill: None, - interpolate: None, }, ], - select.order_by + select.order_by.exprs ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); } @@ -2219,7 +2215,6 @@ fn parse_select_qualify() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }], window_frame: None, })), @@ -2581,7 +2576,6 @@ fn parse_listagg() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident { @@ -2591,7 +2585,6 @@ fn parse_listagg() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }, ] }), @@ -3442,7 +3435,10 @@ fn parse_create_table_as_table() { table_name: Some("old_table".to_string()), schema_name: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, limit: None, limit_by: vec![], offset: None, @@ -3468,7 +3464,10 @@ fn parse_create_table_as_table() { table_name: Some("old_table".to_string()), schema_name: Some("schema_name".to_string()), }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, limit: None, limit_by: vec![], offset: None, @@ -4388,7 +4387,6 @@ fn parse_window_functions() { asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }], window_frame: None, })), @@ -4598,7 +4596,6 @@ fn test_parse_named_window() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }], window_frame: None, }), @@ -5019,7 +5016,10 @@ fn parse_interval_and_or_xor() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None, + }, limit: None, limit_by: vec![], offset: None, @@ -7287,14 +7287,12 @@ fn parse_create_index() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }, ]; match verified_stmt(sql) { @@ -7325,14 +7323,12 @@ fn test_create_index_with_using_function() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }, OrderByExpr { expr: Expr::Identifier(Ident::new("age")), asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }, ]; match verified_stmt(sql) { @@ -7683,7 +7679,10 @@ fn parse_merge() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -9210,7 +9209,10 @@ fn parse_unload() { fetch: None, locks: vec![], for_clause: None, - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, settings: None, }), to: Ident { @@ -9606,7 +9608,6 @@ fn test_match_recognize() { asc: None, nulls_first: None, with_fill: None, - interpolate: None, }], measures: vec![ Measure { diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index e0e0f7c70..57f5aa112 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -102,7 +102,10 @@ fn parse_create_procedure() { fetch: None, locks: vec![], for_clause: None, - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, settings: None, body: Box::new(SetExpr::Select(Box::new(Select { distinct: None, @@ -542,7 +545,10 @@ fn parse_substring_in_select() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 96daca4f3..a08452eb3 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -919,7 +919,10 @@ fn parse_escaped_quote_identifiers_with_escape() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -968,7 +971,10 @@ fn parse_escaped_quote_identifiers_with_no_escape() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1014,7 +1020,10 @@ fn parse_escaped_backticks_with_escape() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1060,7 +1069,10 @@ fn parse_escaped_backticks_with_no_escape() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1265,7 +1277,10 @@ fn parse_simple_insert() { ] ] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1308,7 +1323,10 @@ fn parse_ignore_insert() { Expr::Value(number("1")) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1351,7 +1369,10 @@ fn parse_priority_insert() { Expr::Value(number("1")) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1391,7 +1412,10 @@ fn parse_priority_insert() { Expr::Value(number("1")) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1439,7 +1463,10 @@ fn parse_insert_as() { "2024-01-01".to_string() ))]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1499,7 +1526,10 @@ fn parse_insert_as() { Expr::Value(Value::SingleQuotedString("2024-01-01".to_string())) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1543,7 +1573,10 @@ fn parse_replace_insert() { Expr::Value(number("1")) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1581,7 +1614,10 @@ fn parse_empty_row_insert() { explicit_row: false, rows: vec![vec![], vec![]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1642,7 +1678,10 @@ fn parse_insert_with_on_duplicate_update() { Expr::Value(Value::Boolean(true)), ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -1903,7 +1942,6 @@ fn parse_delete_with_order_by() { asc: Some(false), nulls_first: None, with_fill: None, - interpolate: None, }], order_by ); @@ -2288,7 +2326,10 @@ fn parse_substring_in_select() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -2595,7 +2636,10 @@ fn parse_hex_string_introducer() { into: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 2d3097cf9..dcc240ed6 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -1087,7 +1087,10 @@ fn parse_copy_to() { value_table_mode: None, connect_by: None, }))), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -2418,7 +2421,10 @@ fn parse_array_subquery_expr() { connect_by: None, }))), }), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -3940,7 +3946,10 @@ fn test_simple_postgres_insert_with_alias() { Expr::Value(Value::Number("123".to_string(), false)) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -4008,7 +4017,10 @@ fn test_simple_postgres_insert_with_alias() { )) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, @@ -4072,7 +4084,10 @@ fn test_simple_insert_with_quoted_alias() { Expr::Value(Value::SingleQuotedString("0123".to_string())) ]] })), - order_by: vec![], + order_by: OrderBy { + exprs: vec![], + interpolate: None + }, limit: None, limit_by: vec![], offset: None, From 44deb69db2ddb4c9448e9417f7e98fcd3056aa90 Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Thu, 11 Jul 2024 09:57:06 -0400 Subject: [PATCH 08/10] Switch ORDER BY to Option --- src/ast/query.rs | 15 ++++--- src/parser/mod.rs | 62 ++++++++++++----------------- tests/sqlparser_clickhouse.rs | 10 ++--- tests/sqlparser_common.rs | 36 +++++------------ tests/sqlparser_mssql.rs | 10 +---- tests/sqlparser_mysql.rs | 75 +++++++---------------------------- tests/sqlparser_postgres.rs | 20 ++-------- 7 files changed, 67 insertions(+), 161 deletions(-) diff --git a/src/ast/query.rs b/src/ast/query.rs index da0a71534..d32353a1e 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -33,7 +33,7 @@ pub struct Query { /// SELECT or UNION / EXCEPT / INTERSECT pub body: Box, /// ORDER BY - pub order_by: OrderBy, + pub order_by: Option, /// `LIMIT { | ALL }` pub limit: Option, @@ -67,13 +67,12 @@ impl fmt::Display for Query { write!(f, "{with} ")?; } write!(f, "{}", self.body)?; - if !self.order_by.exprs.is_empty() { - write!( - f, - " ORDER BY {}", - display_comma_separated(&self.order_by.exprs) - )?; - if let Some(ref interpolate) = self.order_by.interpolate { + if let Some(ref order_by) = self.order_by { + write!(f, " ORDER BY")?; + if !order_by.exprs.is_empty() { + write!(f, " {}", display_comma_separated(&order_by.exprs))?; + } + if let Some(ref interpolate) = order_by.interpolate { match &interpolate.exprs { Some(exprs) => write!(f, " INTERPOLATE ({})", display_comma_separated(exprs))?, None => write!(f, " INTERPOLATE")?, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 7b49bcf82..af9ba8e89 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7931,10 +7931,7 @@ impl<'a> Parser<'a> { body: self.parse_insert_setexpr_boxed()?, limit: None, limit_by: vec![], - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, offset: None, fetch: None, locks: vec![], @@ -7948,10 +7945,7 @@ impl<'a> Parser<'a> { body: self.parse_update_setexpr_boxed()?, limit: None, limit_by: vec![], - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, offset: None, fetch: None, locks: vec![], @@ -7964,33 +7958,14 @@ impl<'a> Parser<'a> { let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) { let order_by_exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?; - let interpolate = if dialect_of!(self is ClickHouseDialect | GenericDialect) - && self.parse_keyword(Keyword::INTERPOLATE) - { - if self.consume_token(&Token::LParen) { - let interpolations = self.parse_interpolations()?; - self.expect_token(&Token::RParen)?; - // INTERPOLATE () and INTERPOLATE ( ... ) variants - Some(Interpolate { - exprs: Some(interpolations), - }) - } else { - // INTERPOLATE - Some(Interpolate { exprs: None }) - } - } else { - None - }; + let interpolate = self.parse_interpolations()?; - OrderBy { + Some(OrderBy { exprs: order_by_exprs, interpolate, - } + }) } else { - OrderBy { - exprs: vec![], - interpolate: None, - } + None }; let mut limit = None; @@ -9221,10 +9196,7 @@ impl<'a> Parser<'a> { subquery: Box::new(Query { with: None, body: Box::new(values), - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -10587,8 +10559,24 @@ impl<'a> Parser<'a> { // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier - pub fn parse_interpolations(&mut self) -> Result, ParserError> { - self.parse_comma_separated0(|p| p.parse_interpolation()) + pub fn parse_interpolations(&mut self) -> Result, ParserError> { + if dialect_of!(self is ClickHouseDialect | GenericDialect) + && self.parse_keyword(Keyword::INTERPOLATE) + { + if self.consume_token(&Token::LParen) { + let interpolations = self.parse_comma_separated0(|p| p.parse_interpolation())?; + self.expect_token(&Token::RParen)?; + // INTERPOLATE () and INTERPOLATE ( ... ) variants + Ok(Some(Interpolate { + exprs: Some(interpolations), + })) + } else { + // INTERPOLATE + Ok(Some(Interpolate { exprs: None })) + } + } else { + Ok(None) + } } // Parse a INTERPOLATE expression (ClickHouse dialect) diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 774d165e3..5e2604c63 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -762,7 +762,7 @@ fn parse_select_order_by_with_fill_interpolate() { }]) }) }, - select.order_by + select.order_by.expect("ORDER BY expected") ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); } @@ -809,7 +809,7 @@ fn parse_with_fill() { to: Some(Expr::Value(number("20"))), step: Some(Expr::Value(number("2"))), }), - select.order_by.exprs[0].with_fill + select.order_by.expect("ORDER BY expected").exprs[0].with_fill ); } @@ -861,7 +861,7 @@ fn parse_interpolate_body_with_columns() { }, ]) }), - select.order_by.interpolate + select.order_by.expect("ORDER BY expected").interpolate ); } @@ -871,7 +871,7 @@ fn parse_interpolate_without_body() { let select = clickhouse().verified_query(sql); assert_eq!( Some(Interpolate { exprs: None }), - select.order_by.interpolate + select.order_by.expect("ORDER BY expected").interpolate ); } @@ -883,7 +883,7 @@ fn parse_interpolate_with_empty_body() { Some(Interpolate { exprs: Some(vec![]) }), - select.order_by.interpolate + select.order_by.expect("ORDER BY expected").interpolate ); } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 2724ab180..e44a7334d 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -407,10 +407,7 @@ fn parse_update_set_from() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -2069,7 +2066,7 @@ fn parse_select_order_by() { with_fill: None, }, ], - select.order_by.exprs + select.order_by.expect("ORDER BY expected").exprs ); } chk("SELECT id, fname, lname FROM customer WHERE id < 5 ORDER BY lname ASC, fname DESC, id"); @@ -2098,7 +2095,7 @@ fn parse_select_order_by_limit() { with_fill: None, }, ], - select.order_by.exprs + select.order_by.expect("ORDER BY expected").exprs ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); } @@ -2123,7 +2120,7 @@ fn parse_select_order_by_nulls_order() { with_fill: None, }, ], - select.order_by.exprs + select.order_by.expect("ORDER BY expeccted").exprs ); assert_eq!(Some(Expr::Value(number("2"))), select.limit); } @@ -3436,10 +3433,7 @@ fn parse_create_table_as_table() { table_name: Some("old_table".to_string()), schema_name: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -3466,10 +3460,7 @@ fn parse_create_table_as_table() { table_name: Some("old_table".to_string()), schema_name: Some("schema_name".to_string()), }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -5019,10 +5010,7 @@ fn parse_interval_and_or_xor() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None, - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -7683,10 +7671,7 @@ fn parse_merge() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -9214,10 +9199,7 @@ fn parse_unload() { fetch: None, locks: vec![], for_clause: None, - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, settings: None, format_clause: None, }), diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 2dc10482a..1f18cc041 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -102,10 +102,7 @@ fn parse_create_procedure() { fetch: None, locks: vec![], for_clause: None, - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, settings: None, format_clause: None, body: Box::new(SetExpr::Select(Box::new(Select { @@ -546,10 +543,7 @@ fn parse_substring_in_select() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 48a968c2b..68f88ca9c 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -919,10 +919,7 @@ fn parse_escaped_quote_identifiers_with_escape() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -972,10 +969,7 @@ fn parse_escaped_quote_identifiers_with_no_escape() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1022,10 +1016,7 @@ fn parse_escaped_backticks_with_escape() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1072,10 +1063,7 @@ fn parse_escaped_backticks_with_no_escape() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1281,10 +1269,7 @@ fn parse_simple_insert() { ] ] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1328,10 +1313,7 @@ fn parse_ignore_insert() { Expr::Value(number("1")) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1375,10 +1357,7 @@ fn parse_priority_insert() { Expr::Value(number("1")) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1419,10 +1398,7 @@ fn parse_priority_insert() { Expr::Value(number("1")) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1471,10 +1447,7 @@ fn parse_insert_as() { "2024-01-01".to_string() ))]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1535,10 +1508,7 @@ fn parse_insert_as() { Expr::Value(Value::SingleQuotedString("2024-01-01".to_string())) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1583,10 +1553,7 @@ fn parse_replace_insert() { Expr::Value(number("1")) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1625,10 +1592,7 @@ fn parse_empty_row_insert() { explicit_row: false, rows: vec![vec![], vec![]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -1690,10 +1654,7 @@ fn parse_insert_with_on_duplicate_update() { Expr::Value(Value::Boolean(true)), ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -2339,10 +2300,7 @@ fn parse_substring_in_select() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -2650,10 +2608,7 @@ fn parse_hex_string_introducer() { into: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 26ad98b35..f8f515297 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -1159,10 +1159,7 @@ fn parse_copy_to() { value_table_mode: None, connect_by: None, }))), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -2494,10 +2491,7 @@ fn parse_array_subquery_expr() { connect_by: None, }))), }), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -4122,10 +4116,7 @@ fn test_simple_postgres_insert_with_alias() { Expr::Value(Value::Number("123".to_string(), false)) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, @@ -4262,10 +4253,7 @@ fn test_simple_insert_with_quoted_alias() { Expr::Value(Value::SingleQuotedString("0123".to_string())) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None, From 000e08d3f0849b93b70f056614b51539f3adebfe Mon Sep 17 00:00:00 2001 From: Nick Presta Date: Thu, 11 Jul 2024 16:13:20 -0400 Subject: [PATCH 09/10] Refactor parser and early return to reduce nesting --- src/parser/mod.rs | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index af9ba8e89..91bf8d513 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7958,7 +7958,11 @@ impl<'a> Parser<'a> { let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) { let order_by_exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?; - let interpolate = self.parse_interpolations()?; + let interpolate = if dialect_of!(self is ClickHouseDialect | GenericDialect) { + self.parse_interpolations()? + } else { + None + }; Some(OrderBy { exprs: order_by_exprs, @@ -10560,23 +10564,21 @@ impl<'a> Parser<'a> { // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect) // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier pub fn parse_interpolations(&mut self) -> Result, ParserError> { - if dialect_of!(self is ClickHouseDialect | GenericDialect) - && self.parse_keyword(Keyword::INTERPOLATE) - { - if self.consume_token(&Token::LParen) { - let interpolations = self.parse_comma_separated0(|p| p.parse_interpolation())?; - self.expect_token(&Token::RParen)?; - // INTERPOLATE () and INTERPOLATE ( ... ) variants - Ok(Some(Interpolate { - exprs: Some(interpolations), - })) - } else { - // INTERPOLATE - Ok(Some(Interpolate { exprs: None })) - } - } else { - Ok(None) + if !self.parse_keyword(Keyword::INTERPOLATE) { + return Ok(None); } + + if self.consume_token(&Token::LParen) { + let interpolations = self.parse_comma_separated0(|p| p.parse_interpolation())?; + self.expect_token(&Token::RParen)?; + // INTERPOLATE () and INTERPOLATE ( ... ) variants + return Ok(Some(Interpolate { + exprs: Some(interpolations), + })); + } + + // INTERPOLATE + Ok(Some(Interpolate { exprs: None })) } // Parse a INTERPOLATE expression (ClickHouse dialect) From 6ee4257591f952840424076e4b20070900712fb2 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Sat, 20 Jul 2024 06:43:22 -0400 Subject: [PATCH 10/10] Fix test --- tests/sqlparser_postgres.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 766107f76..5ac421da0 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -4231,10 +4231,7 @@ fn test_simple_postgres_insert_with_alias() { )) ]] })), - order_by: OrderBy { - exprs: vec![], - interpolate: None - }, + order_by: None, limit: None, limit_by: vec![], offset: None,