From b759a95a47e2f587d9217e2778636586234f479f Mon Sep 17 00:00:00 2001 From: Mustafa Akur Date: Thu, 18 May 2023 13:24:00 +0300 Subject: [PATCH 1/3] Add order by parsing to functions --- src/ast/mod.rs | 10 ++++++++- src/parser.rs | 22 +++++++++++++++++++- tests/sqlparser_bigquery.rs | 1 + tests/sqlparser_clickhouse.rs | 4 ++++ tests/sqlparser_common.rs | 39 +++++++++++++++++++++++++++++++++++ tests/sqlparser_hive.rs | 1 + tests/sqlparser_mssql.rs | 1 + tests/sqlparser_mysql.rs | 6 ++++++ tests/sqlparser_postgres.rs | 8 ++++++- tests/sqlparser_redshift.rs | 1 + tests/sqlparser_snowflake.rs | 1 + 11 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 94b237b4f..af23c14f9 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -3257,6 +3257,8 @@ pub struct Function { // Some functions must be called without trailing parentheses, for example Postgres // do it for current_catalog, current_schema, etc. This flags is used for formatting. pub special: bool, + // Some aggregate functions run with ORDER BY, e.g FIRST_VALUE(a, ORDER BY ts DESC) + pub order_by: Vec, } #[derive(Debug, Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] @@ -3283,12 +3285,18 @@ impl fmt::Display for Function { if self.special { write!(f, "{}", self.name)?; } else { + let order_by = if !self.order_by.is_empty() { + " ORDER BY " + } else { + "" + }; write!( f, - "{}({}{})", + "{}({}{}{order_by}{})", self.name, if self.distinct { "DISTINCT " } else { "" }, display_comma_separated(&self.args), + display_comma_separated(&self.order_by), )?; if let Some(o) = &self.over { diff --git a/src/parser.rs b/src/parser.rs index b06e6bd25..4b842310e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -705,6 +705,7 @@ impl<'a> Parser<'a> { over: None, distinct: false, special: true, + order_by: vec![], })) } Keyword::CURRENT_TIMESTAMP @@ -880,7 +881,7 @@ impl<'a> Parser<'a> { pub fn parse_function(&mut self, name: ObjectName) -> Result { self.expect_token(&Token::LParen)?; let distinct = self.parse_all_or_distinct()?; - let args = self.parse_optional_args()?; + let (args, order_by) = self.parse_optional_args_with_orderby()?; let over = if self.parse_keyword(Keyword::OVER) { // TBD: support window names (`OVER mywin`) in place of inline specification self.expect_token(&Token::LParen)?; @@ -917,6 +918,7 @@ impl<'a> Parser<'a> { over, distinct, special: false, + order_by, })) } @@ -932,6 +934,7 @@ impl<'a> Parser<'a> { over: None, distinct: false, special: false, + order_by: vec![], })) } @@ -6238,6 +6241,23 @@ impl<'a> Parser<'a> { } } + pub fn parse_optional_args_with_orderby( + &mut self, + ) -> Result<(Vec, Vec), ParserError> { + if self.consume_token(&Token::RParen) { + Ok((vec![], vec![])) + } else { + let args = self.parse_comma_separated(Parser::parse_function_args)?; + let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) { + self.parse_comma_separated(Parser::parse_order_by_expr)? + } else { + vec![] + }; + self.expect_token(&Token::RParen)?; + Ok((args, order_by)) + } + } + /// Parse a comma-delimited list of projections after SELECT pub fn parse_select_item(&mut self) -> Result { match self.parse_wildcard_expr()? { diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 11a8c6e5c..e9aa6e313 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -435,6 +435,7 @@ fn parse_map_access_offset() { over: None, distinct: false, special: false, + order_by: vec![], })], }) ); diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index 3ee3fbc03..8a9537bad 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -52,6 +52,7 @@ fn parse_map_access_expr() { over: None, distinct: false, special: false, + order_by: vec![], })], })], into: None, @@ -88,6 +89,7 @@ fn parse_map_access_expr() { over: None, distinct: false, special: false, + order_by: vec![], })] }), op: BinaryOperator::NotEq, @@ -135,6 +137,7 @@ fn parse_array_fn() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(only(&select.projection)) ); @@ -189,6 +192,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 55350e5a6..56c46d338 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -723,6 +723,7 @@ fn parse_select_count_wildcard() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(only(&select.projection)) ); @@ -742,6 +743,7 @@ fn parse_select_count_distinct() { over: None, distinct: true, special: false, + order_by: vec![], }), expr_from_projection(only(&select.projection)) ); @@ -1576,6 +1578,7 @@ fn parse_select_having() { over: None, distinct: false, special: false, + order_by: vec![], })), op: BinaryOperator::Gt, right: Box::new(Expr::Value(number("1"))), @@ -1609,6 +1612,7 @@ fn parse_select_qualify() { }), distinct: false, special: false, + order_by: vec![], })), op: BinaryOperator::Eq, right: Box::new(Expr::Value(number("1"))), @@ -1952,6 +1956,28 @@ fn parse_array_agg_func() { } } +#[test] +fn parse_agg_with_order_by() { + let supported_dialects = TestedDialects { + dialects: vec![ + Box::new(GenericDialect {}), + Box::new(PostgreSqlDialect {}), + Box::new(MsSqlDialect {}), + Box::new(AnsiDialect {}), + Box::new(HiveDialect {}), + ], + }; + + for sql in [ + "SELECT FIRST_VALUE(x ORDER BY x) AS a FROM T", + "SELECT FIRST_VALUE(x ORDER BY x) FROM tbl", + "SELECT LAST_VALUE(x ORDER BY x, y) AS a FROM T", + "SELECT LAST_VALUE(x ORDER BY x ASC, y DESC) AS a FROM T", + ] { + supported_dialects.verified_stmt(sql); + } +} + #[test] fn parse_create_table() { let sql = "CREATE TABLE uk_cities (\ @@ -2993,6 +3019,7 @@ fn parse_scalar_function_in_projection() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(only(&select.projection)) ); @@ -3111,6 +3138,7 @@ fn parse_named_argument_function() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(only(&select.projection)) ); @@ -3149,6 +3177,7 @@ fn parse_window_functions() { }), distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -3547,6 +3576,7 @@ fn parse_at_timezone() { over: None, distinct: false, special: false, + order_by: vec![], })), time_zone: "UTC-06:00".to_string(), }, @@ -3573,6 +3603,7 @@ fn parse_at_timezone() { over: None, distinct: false, special: false, + order_by: vec![], },)), time_zone: "UTC-06:00".to_string(), },),), @@ -3583,6 +3614,7 @@ fn parse_at_timezone() { over: None, distinct: false, special: false, + order_by: vec![], },), alias: Ident { value: "hour".to_string(), @@ -3740,6 +3772,7 @@ fn parse_table_function() { over: None, distinct: false, special: false, + order_by: vec![], }); assert_eq!(expr, expected_expr); assert_eq!(alias, table_alias("a")) @@ -6161,6 +6194,7 @@ fn parse_time_functions() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -6177,6 +6211,7 @@ fn parse_time_functions() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -6193,6 +6228,7 @@ fn parse_time_functions() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -6209,6 +6245,7 @@ fn parse_time_functions() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -6225,6 +6262,7 @@ fn parse_time_functions() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -6689,6 +6727,7 @@ fn parse_pivot_table() { over: None, distinct: false, special: false, + order_by: vec![], }), value_column: vec![Ident::new("a"), Ident::new("MONTH")], pivot_values: vec![ diff --git a/tests/sqlparser_hive.rs b/tests/sqlparser_hive.rs index 064a090f7..6fe452322 100644 --- a/tests/sqlparser_hive.rs +++ b/tests/sqlparser_hive.rs @@ -345,6 +345,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 41b0803e4..2b3589e36 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -177,6 +177,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 0133cc9df..7692a59ba 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -791,6 +791,7 @@ fn parse_insert_with_on_duplicate_update() { over: None, distinct: false, special: false, + order_by: vec![], }) }, Assignment { @@ -803,6 +804,7 @@ fn parse_insert_with_on_duplicate_update() { over: None, distinct: false, special: false, + order_by: vec![], }) }, Assignment { @@ -815,6 +817,7 @@ fn parse_insert_with_on_duplicate_update() { over: None, distinct: false, special: false, + order_by: vec![], }) }, Assignment { @@ -827,6 +830,7 @@ fn parse_insert_with_on_duplicate_update() { over: None, distinct: false, special: false, + order_by: vec![], }) }, Assignment { @@ -839,6 +843,7 @@ fn parse_insert_with_on_duplicate_update() { over: None, distinct: false, special: false, + order_by: vec![], }) }, ])), @@ -1082,6 +1087,7 @@ fn parse_table_colum_option_on_update() { over: None, distinct: false, special: false, + order_by: vec![], })), },], }], diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 471421215..8c5ad0ce2 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -1970,7 +1970,8 @@ fn test_composite_value() { )))], over: None, distinct: false, - special: false + special: false, + order_by: vec![], })))) }), select.projection[0] @@ -2130,6 +2131,7 @@ fn parse_current_functions() { over: None, distinct: false, special: true, + order_by: vec![], }), expr_from_projection(&select.projection[0]) ); @@ -2140,6 +2142,7 @@ fn parse_current_functions() { over: None, distinct: false, special: true, + order_by: vec![], }), expr_from_projection(&select.projection[1]) ); @@ -2150,6 +2153,7 @@ fn parse_current_functions() { over: None, distinct: false, special: true, + order_by: vec![], }), expr_from_projection(&select.projection[2]) ); @@ -2160,6 +2164,7 @@ fn parse_current_functions() { over: None, distinct: false, special: true, + order_by: vec![], }), expr_from_projection(&select.projection[3]) ); @@ -2414,6 +2419,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); diff --git a/tests/sqlparser_redshift.rs b/tests/sqlparser_redshift.rs index 7597ee981..345ad7f80 100644 --- a/tests/sqlparser_redshift.rs +++ b/tests/sqlparser_redshift.rs @@ -132,6 +132,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index ea14cc44b..2849aa995 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -246,6 +246,7 @@ fn parse_delimited_identifiers() { over: None, distinct: false, special: false, + order_by: vec![], }), expr_from_projection(&select.projection[1]), ); From 22b0b7c99b810d562185228d5193dbbaf58ac979 Mon Sep 17 00:00:00 2001 From: Mustafa Akur Date: Thu, 18 May 2023 15:30:10 +0300 Subject: [PATCH 2/3] Fix doc error --- src/ast/visitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index 6787bfd68..81343220a 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -480,7 +480,7 @@ where /// *expr = Expr::Function(Function { /// name: ObjectName(vec![Ident::new("f")]), /// args: vec![FunctionArg::Unnamed(FunctionArgExpr::Expr(old_expr))], -/// over: None, distinct: false, special: false, +/// over: None, distinct: false, special: false, order_by: vec![], /// }); /// } /// ControlFlow::<()>::Continue(()) From db1e631dbb7002ea50c85f62e4c1aaeb2d21fbaf Mon Sep 17 00:00:00 2001 From: Mustafa Akur Date: Thu, 18 May 2023 15:54:53 +0300 Subject: [PATCH 3/3] minor changes --- src/ast/mod.rs | 2 +- src/parser.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 644794294..2d13f2ea7 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -3362,7 +3362,7 @@ pub struct Function { // Some functions must be called without trailing parentheses, for example Postgres // do it for current_catalog, current_schema, etc. This flags is used for formatting. pub special: bool, - // Some aggregate functions run with ORDER BY, e.g FIRST_VALUE(a, ORDER BY ts DESC) + // Required ordering for the function (if empty, there is no requirement). pub order_by: Vec, } diff --git a/src/parser.rs b/src/parser.rs index eec420fa1..c53f07fce 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -923,10 +923,10 @@ impl<'a> Parser<'a> { } pub fn parse_time_functions(&mut self, name: ObjectName) -> Result { - let args = if self.consume_token(&Token::LParen) { - self.parse_optional_args()? + let (args, order_by) = if self.consume_token(&Token::LParen) { + self.parse_optional_args_with_orderby()? } else { - vec![] + (vec![], vec![]) }; Ok(Expr::Function(Function { name, @@ -934,7 +934,7 @@ impl<'a> Parser<'a> { over: None, distinct: false, special: false, - order_by: vec![], + order_by, })) }