From c8e7c3b3433d489fb9df4f4390a174ec6d4b486f Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sat, 20 Apr 2019 19:54:12 +0300 Subject: [PATCH 1/7] Introduce comma_separated_string() to reduce boilerplate in ToString impls --- src/sqlast/mod.rs | 49 +++++++++++++-------------------------------- src/sqlast/query.rs | 24 +++------------------- 2 files changed, 17 insertions(+), 56 deletions(-) diff --git a/src/sqlast/mod.rs b/src/sqlast/mod.rs index eef10fcdb..28dbeec38 100644 --- a/src/sqlast/mod.rs +++ b/src/sqlast/mod.rs @@ -30,6 +30,14 @@ pub use self::value::Value; pub use self::sql_operator::SQLOperator; +/// Like `vec.join(", ")`, but for any types implementing ToString. +fn comma_separated_string(vec: &Vec) -> String { + vec.iter() + .map(|a| a.to_string()) + .collect::>() + .join(", ") +} + /// Identifier name, in the originally quoted form (e.g. `"id"`) pub type SQLIdent = String; @@ -123,10 +131,7 @@ impl ToString for ASTNode { "{} {}IN ({})", expr.as_ref().to_string(), if *negated { "NOT " } else { "" }, - list.iter() - .map(|a| a.to_string()) - .collect::>() - .join(", ") + comma_separated_string(list) ), ASTNode::SQLInSubquery { expr, @@ -279,11 +284,7 @@ impl ToString for SQLStatement { " VALUES({})", values .iter() - .map(|row| row - .iter() - .map(|c| c.to_string()) - .collect::>() - .join(", ")) + .map(|row| comma_separated_string(row)) .collect::>() .join(", ") ); @@ -297,14 +298,7 @@ impl ToString for SQLStatement { } => { let mut s = format!("COPY {}", table_name.to_string()); if columns.len() > 0 { - s += &format!( - " ({})", - columns - .iter() - .map(|c| c.to_string()) - .collect::>() - .join(", ") - ); + s += &format!(" ({})", comma_separated_string(columns)); } s += " FROM stdin; "; if values.len() > 0 { @@ -327,14 +321,7 @@ impl ToString for SQLStatement { } => { let mut s = format!("UPDATE {}", table_name.to_string()); if assignments.len() > 0 { - s += &format!( - "{}", - assignments - .iter() - .map(|ass| ass.to_string()) - .collect::>() - .join(", ") - ); + s += &format!("{}", comma_separated_string(assignments)); } if let Some(selection) = selection { s += &format!(" WHERE {}", selection.to_string()); @@ -373,11 +360,7 @@ impl ToString for SQLStatement { } if *external => format!( "CREATE EXTERNAL TABLE {} ({}) STORED AS {} LOCATION '{}'", name.to_string(), - columns - .iter() - .map(|c| c.to_string()) - .collect::>() - .join(", "), + comma_separated_string(columns), file_format.as_ref().map(|f| f.to_string()).unwrap(), location.as_ref().unwrap() ), @@ -390,11 +373,7 @@ impl ToString for SQLStatement { } => format!( "CREATE TABLE {} ({})", name.to_string(), - columns - .iter() - .map(|c| c.to_string()) - .collect::>() - .join(", ") + comma_separated_string(columns) ), SQLStatement::SQLAlterTable { name, operation } => { format!("ALTER TABLE {} {}", name.to_string(), operation.to_string()) diff --git a/src/sqlast/query.rs b/src/sqlast/query.rs index dbcb0b6bd..f927f4b35 100644 --- a/src/sqlast/query.rs +++ b/src/sqlast/query.rs @@ -29,14 +29,7 @@ impl ToString for SQLQuery { } s += &self.body.to_string(); if let Some(ref order_by) = self.order_by { - s += &format!( - " ORDER BY {}", - order_by - .iter() - .map(|o| o.to_string()) - .collect::>() - .join(", ") - ); + s += &format!(" ORDER BY {}", comma_separated_string(order_by)); } if let Some(ref limit) = self.limit { s += &format!(" LIMIT {}", limit.to_string()); @@ -130,11 +123,7 @@ impl ToString for SQLSelect { let mut s = format!( "SELECT{} {}", if self.distinct { " DISTINCT" } else { "" }, - self.projection - .iter() - .map(|p| p.to_string()) - .collect::>() - .join(", ") + comma_separated_string(&self.projection) ); if let Some(ref relation) = self.relation { s += &format!(" FROM {}", relation.to_string()); @@ -146,14 +135,7 @@ impl ToString for SQLSelect { s += &format!(" WHERE {}", selection.to_string()); } if let Some(ref group_by) = self.group_by { - s += &format!( - " GROUP BY {}", - group_by - .iter() - .map(|g| g.to_string()) - .collect::>() - .join(", ") - ); + s += &format!(" GROUP BY {}", comma_separated_string(group_by)); } if let Some(ref having) = self.having { s += &format!(" HAVING {}", having.to_string()); From d4de248c7343f1511a1075dcc3f3d359aab20bd8 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Mon, 11 Feb 2019 03:27:55 +0300 Subject: [PATCH 2/7] Support OVER clause for window/analytic functions Since this changes SQLFunction anyway, changed its `id` field to `name`, as we don't seem to use "id" to mean "name" anywhere else. --- src/dialect/keywords.rs | 6 ++ src/sqlast/mod.rs | 131 ++++++++++++++++++++++++++++++++++--- src/sqlparser.rs | 95 ++++++++++++++++++++++++++- tests/sqlparser_generic.rs | 40 +++++++++-- 4 files changed, 257 insertions(+), 15 deletions(-) diff --git a/src/dialect/keywords.rs b/src/dialect/keywords.rs index e270e8a9f..2cf9adbf7 100644 --- a/src/dialect/keywords.rs +++ b/src/dialect/keywords.rs @@ -139,6 +139,7 @@ keyword!( FIRST_VALUE, FLOAT, FLOOR, + FOLLOWING, FOR, FOREIGN, FRAME_ROW, @@ -246,6 +247,7 @@ keyword!( POSITION_REGEX, POWER, PRECEDES, + PRECEDING, PRECISION, PREPARE, PRIMARY, @@ -333,6 +335,7 @@ keyword!( TRIM_ARRAY, TRUE, UESCAPE, + UNBOUNDED, UNION, UNIQUE, UNKNOWN, @@ -488,6 +491,7 @@ pub const ALL_KEYWORDS: &'static [&'static str] = &[ FIRST_VALUE, FLOAT, FLOOR, + FOLLOWING, FOR, FOREIGN, FRAME_ROW, @@ -595,6 +599,7 @@ pub const ALL_KEYWORDS: &'static [&'static str] = &[ POSITION_REGEX, POWER, PRECEDES, + PRECEDING, PRECISION, PREPARE, PRIMARY, @@ -682,6 +687,7 @@ pub const ALL_KEYWORDS: &'static [&'static str] = &[ TRIM_ARRAY, TRUE, UESCAPE, + UNBOUNDED, UNION, UNIQUE, UNKNOWN, diff --git a/src/sqlast/mod.rs b/src/sqlast/mod.rs index 28dbeec38..8af6e75c4 100644 --- a/src/sqlast/mod.rs +++ b/src/sqlast/mod.rs @@ -101,7 +101,11 @@ pub enum ASTNode { SQLValue(Value), /// Scalar function call e.g. `LEFT(foo, 5)` /// TODO: this can be a compound SQLObjectName as well (for UDFs) - SQLFunction { id: SQLIdent, args: Vec }, + SQLFunction { + name: SQLIdent, + args: Vec, + over: Option, + }, /// CASE [] WHEN THEN ... [ELSE ] END SQLCase { // TODO: support optional operand for "simple case" @@ -171,14 +175,13 @@ impl ToString for ASTNode { format!("{} {}", operator.to_string(), expr.as_ref().to_string()) } ASTNode::SQLValue(v) => v.to_string(), - ASTNode::SQLFunction { id, args } => format!( - "{}({})", - id, - args.iter() - .map(|a| a.to_string()) - .collect::>() - .join(", ") - ), + ASTNode::SQLFunction { name, args, over } => { + let mut s = format!("{}({})", name, comma_separated_string(args)); + if let Some(o) = over { + s += &format!(" OVER ({})", o.to_string()) + } + s + } ASTNode::SQLCase { conditions, results, @@ -203,6 +206,116 @@ impl ToString for ASTNode { } } +/// A window specification (i.e. `OVER (PARTITION BY .. ORDER BY .. etc.)`) +#[derive(Debug, Clone, PartialEq)] +pub struct SQLWindowSpec { + pub partition_by: Vec, + pub order_by: Vec, + pub window_frame: Option, +} + +impl ToString for SQLWindowSpec { + fn to_string(&self) -> String { + let mut clauses = vec![]; + if !self.partition_by.is_empty() { + clauses.push(format!( + "PARTITION BY {}", + comma_separated_string(&self.partition_by) + )) + }; + if !self.order_by.is_empty() { + clauses.push(format!( + "ORDER BY {}", + comma_separated_string(&self.order_by) + )) + }; + if let Some(window_frame) = &self.window_frame { + if let Some(end_bound) = &window_frame.end_bound { + clauses.push(format!( + "{} BETWEEN {} AND {}", + window_frame.units.to_string(), + window_frame.start_bound.to_string(), + end_bound.to_string() + )); + } else { + clauses.push(format!( + "{} {}", + window_frame.units.to_string(), + window_frame.start_bound.to_string() + )); + } + } + clauses.join(" ") + } +} + +/// Specifies the data processed by a window function, e.g. +/// `RANGE UNBOUNDED PRECEDING` or `ROWS BETWEEN 5 PRECEDING AND CURRENT ROW`. +#[derive(Debug, Clone, PartialEq)] +pub struct SQLWindowFrame { + pub units: SQLWindowFrameUnits, + pub start_bound: SQLWindowFrameBound, + /// The right bound of the `BETWEEN .. AND` clause. + pub end_bound: Option, + // TBD: EXCLUDE +} + +#[derive(Debug, Clone, PartialEq)] +pub enum SQLWindowFrameUnits { + Rows, + Range, + Groups, +} + +impl ToString for SQLWindowFrameUnits { + fn to_string(&self) -> String { + match self { + SQLWindowFrameUnits::Rows => "ROWS".to_string(), + SQLWindowFrameUnits::Range => "RANGE".to_string(), + SQLWindowFrameUnits::Groups => "GROUPS".to_string(), + } + } +} + +impl FromStr for SQLWindowFrameUnits { + type Err = ParserError; + + fn from_str(s: &str) -> Result { + match s { + "ROWS" => Ok(SQLWindowFrameUnits::Rows), + "RANGE" => Ok(SQLWindowFrameUnits::Range), + "GROUPS" => Ok(SQLWindowFrameUnits::Groups), + _ => Err(ParserError::ParserError(format!( + "Expected ROWS, RANGE, or GROUPS, found: {}", + s + ))), + } + } +} + +#[derive(Debug, Clone, PartialEq)] +pub enum SQLWindowFrameBound { + /// "CURRENT ROW" + CurrentRow, + /// " PRECEDING" or "UNBOUNDED PRECEDING" + Preceding(Option), + /// " FOLLOWING" or "UNBOUNDED FOLLOWING". This can only appear in + /// SQLWindowFrame::end_bound. + Following(Option), +} + +impl ToString for SQLWindowFrameBound { + fn to_string(&self) -> String { + match self { + SQLWindowFrameBound::CurrentRow => "CURRENT ROW".to_string(), + SQLWindowFrameBound::Preceding(None) => "UNBOUNDED PRECEDING".to_string(), + SQLWindowFrameBound::Following(None) => "UNBOUNDED FOLLOWING".to_string(), + SQLWindowFrameBound::Preceding(Some(n)) => format!("{} PRECEDING", n), + SQLWindowFrameBound::Following(Some(n)) => format!("{} FOLLOWING", n), + } + } +} + /// A top-level statement (SELECT, INSERT, CREATE, etc.) #[derive(Debug, Clone, PartialEq)] pub enum SQLStatement { diff --git a/src/sqlparser.rs b/src/sqlparser.rs index d59048c8a..00f7d15ab 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -237,7 +237,7 @@ impl Parser { } } - pub fn parse_function(&mut self, id: SQLIdent) -> Result { + pub fn parse_function(&mut self, name: SQLIdent) -> Result { self.expect_token(&Token::LParen)?; let args = if self.consume_token(&Token::RParen) { vec![] @@ -246,7 +246,98 @@ impl Parser { self.expect_token(&Token::RParen)?; args }; - Ok(ASTNode::SQLFunction { id, args }) + let over = if self.parse_keyword("OVER") { + // TBD: support window names (`OVER mywin`) in place of inline specification + self.expect_token(&Token::LParen)?; + let partition_by = if self.parse_keywords(vec!["PARTITION", "BY"]) { + // a list of possibly-qualified column names + self.parse_expr_list()? + } else { + vec![] + }; + let order_by = if self.parse_keywords(vec!["ORDER", "BY"]) { + self.parse_order_by_expr_list()? + } else { + vec![] + }; + let window_frame = self.parse_window_frame()?; + + Some(SQLWindowSpec { + partition_by, + order_by, + window_frame, + }) + } else { + None + }; + + Ok(ASTNode::SQLFunction { name, args, over }) + } + + pub fn parse_window_frame(&mut self) -> Result, ParserError> { + let window_frame = match self.peek_token() { + Some(Token::SQLWord(w)) => { + let units = w.keyword.parse::()?; + self.next_token(); + if self.parse_keyword("BETWEEN") { + let start_bound = self.parse_window_frame_bound()?; + self.expect_keyword("AND")?; + let end_bound = Some(self.parse_window_frame_bound()?); + Some(SQLWindowFrame { + units, + start_bound, + end_bound, + }) + } else { + let start_bound = self.parse_window_frame_bound()?; + let end_bound = None; + Some(SQLWindowFrame { + units, + start_bound, + end_bound, + }) + } + } + Some(Token::RParen) => None, + unexpected => { + return parser_err!(format!( + "Expected 'ROWS', 'RANGE', 'GROUPS', or ')', got {:?}", + unexpected + )); + } + }; + self.expect_token(&Token::RParen)?; + Ok(window_frame) + } + + /// "CURRENT ROW" | ( ( | "UNBOUNDED") ("PRECEDING" | FOLLOWING) ) + pub fn parse_window_frame_bound(&mut self) -> Result { + if self.parse_keywords(vec!["CURRENT", "ROW"]) { + Ok(SQLWindowFrameBound::CurrentRow) + } else { + let rows = if self.parse_keyword("UNBOUNDED") { + None + } else { + let rows = self.parse_literal_int()?; + if rows < 0 { + parser_err!(format!( + "The number of rows must be non-negative, got {}", + rows + ))?; + } + Some(rows as u64) + }; + if self.parse_keyword("PRECEDING") { + Ok(SQLWindowFrameBound::Preceding(rows)) + } else if self.parse_keyword("FOLLOWING") { + Ok(SQLWindowFrameBound::Following(rows)) + } else { + parser_err!(format!( + "Expected PRECEDING or FOLLOWING, found {:?}", + self.peek_token() + )) + } + } } pub fn parse_case_expression(&mut self) -> Result { diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_generic.rs index ea60e6d47..67e2e56cd 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_generic.rs @@ -119,8 +119,9 @@ fn parse_select_count_wildcard() { let select = verified_only_select(sql); assert_eq!( &ASTNode::SQLFunction { - id: "COUNT".to_string(), + name: "COUNT".to_string(), args: vec![ASTNode::SQLWildcard], + over: None, }, expr_from_projection(only(&select.projection)) ); @@ -532,13 +533,43 @@ fn parse_scalar_function_in_projection() { let select = verified_only_select(sql); assert_eq!( &ASTNode::SQLFunction { - id: String::from("sqrt"), + name: String::from("sqrt"), args: vec![ASTNode::SQLIdentifier(String::from("id"))], + over: None, }, expr_from_projection(only(&select.projection)) ); } +#[test] +fn parse_window_functions() { + let sql = "SELECT row_number() OVER (ORDER BY dt DESC), \ + sum(foo) OVER (PARTITION BY a, b ORDER BY c, d \ + ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW), \ + avg(bar) OVER (ORDER BY a \ + RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING), \ + max(baz) OVER (ORDER BY a \ + ROWS UNBOUNDED PRECEDING) \ + FROM foo"; + let select = verified_only_select(sql); + assert_eq!(4, select.projection.len()); + assert_eq!( + &ASTNode::SQLFunction { + name: "row_number".to_string(), + args: vec![], + over: Some(SQLWindowSpec { + partition_by: vec![], + order_by: vec![SQLOrderByExpr { + expr: ASTNode::SQLIdentifier("dt".to_string()), + asc: Some(false) + }], + window_frame: None, + }) + }, + expr_from_projection(&select.projection[0]) + ); +} + #[test] fn parse_aggregate_with_group_by() { let sql = "SELECT a, COUNT(1), MIN(b), MAX(b) FROM foo GROUP BY a"; @@ -605,8 +636,9 @@ fn parse_delimited_identifiers() { ); assert_eq!( &ASTNode::SQLFunction { - id: r#""myfun""#.to_string(), - args: vec![] + name: r#""myfun""#.to_string(), + args: vec![], + over: None, }, expr_from_projection(&select.projection[1]), ); From 4a5dc8dd4b9d7d7647ab0d3cddd2826cff66b6a4 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sat, 20 Apr 2019 22:20:45 +0300 Subject: [PATCH 3/7] Support qualified function names ...e.g. `db.schema.func()` --- src/sqlast/mod.rs | 7 +++---- src/sqlparser.rs | 15 ++++++++++----- tests/sqlparser_generic.rs | 8 ++++---- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/sqlast/mod.rs b/src/sqlast/mod.rs index 8af6e75c4..a888d82cb 100644 --- a/src/sqlast/mod.rs +++ b/src/sqlast/mod.rs @@ -54,7 +54,7 @@ pub enum ASTNode { /// Qualified wildcard, e.g. `alias.*` or `schema.table.*`. /// (Same caveats apply to SQLQualifiedWildcard as to SQLWildcard.) SQLQualifiedWildcard(Vec), - /// Multi part identifier e.g. `myschema.dbo.mytable` + /// Multi-part identifier, e.g. `table_alias.column` or `schema.table.col` SQLCompoundIdentifier(Vec), /// `IS NULL` expression SQLIsNull(Box), @@ -100,9 +100,8 @@ pub enum ASTNode { /// SQLValue SQLValue(Value), /// Scalar function call e.g. `LEFT(foo, 5)` - /// TODO: this can be a compound SQLObjectName as well (for UDFs) SQLFunction { - name: SQLIdent, + name: SQLObjectName, args: Vec, over: Option, }, @@ -176,7 +175,7 @@ impl ToString for ASTNode { } ASTNode::SQLValue(v) => v.to_string(), ASTNode::SQLFunction { name, args, over } => { - let mut s = format!("{}({})", name, comma_separated_string(args)); + let mut s = format!("{}({})", name.to_string(), comma_separated_string(args)); if let Some(o) = over { s += &format!(" OVER ({})", o.to_string()) } diff --git a/src/sqlparser.rs b/src/sqlparser.rs index 00f7d15ab..ad97bb6fe 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -175,10 +175,10 @@ impl Parser { expr: Box::new(self.parse_subexpr(p)?), }) } - // another SQLWord: + // Here `w` is a word, check if it's a part of a multi-part + // identifier, a function call, or a simple identifier: _ => match self.peek_token() { - Some(Token::LParen) => self.parse_function(w.as_sql_ident()), - Some(Token::Period) => { + Some(Token::LParen) | Some(Token::Period) => { let mut id_parts: Vec = vec![w.as_sql_ident()]; let mut ends_with_wildcard = false; while self.consume_token(&Token::Period) { @@ -198,7 +198,12 @@ impl Parser { if ends_with_wildcard { Ok(ASTNode::SQLQualifiedWildcard(id_parts)) } else { - Ok(ASTNode::SQLCompoundIdentifier(id_parts)) + if self.consume_token(&Token::LParen) { + self.prev_token(); + self.parse_function(SQLObjectName(id_parts)) + } else { + Ok(ASTNode::SQLCompoundIdentifier(id_parts)) + } } } _ => Ok(ASTNode::SQLIdentifier(w.as_sql_ident())), @@ -237,7 +242,7 @@ impl Parser { } } - pub fn parse_function(&mut self, name: SQLIdent) -> Result { + pub fn parse_function(&mut self, name: SQLObjectName) -> Result { self.expect_token(&Token::LParen)?; let args = if self.consume_token(&Token::RParen) { vec![] diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_generic.rs index 67e2e56cd..1f8c46e4f 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_generic.rs @@ -119,7 +119,7 @@ fn parse_select_count_wildcard() { let select = verified_only_select(sql); assert_eq!( &ASTNode::SQLFunction { - name: "COUNT".to_string(), + name: SQLObjectName(vec!["COUNT".to_string()]), args: vec![ASTNode::SQLWildcard], over: None, }, @@ -533,7 +533,7 @@ fn parse_scalar_function_in_projection() { let select = verified_only_select(sql); assert_eq!( &ASTNode::SQLFunction { - name: String::from("sqrt"), + name: SQLObjectName(vec![String::from("sqrt")]), args: vec![ASTNode::SQLIdentifier(String::from("id"))], over: None, }, @@ -555,7 +555,7 @@ fn parse_window_functions() { assert_eq!(4, select.projection.len()); assert_eq!( &ASTNode::SQLFunction { - name: "row_number".to_string(), + name: SQLObjectName(vec!["row_number".to_string()]), args: vec![], over: Some(SQLWindowSpec { partition_by: vec![], @@ -636,7 +636,7 @@ fn parse_delimited_identifiers() { ); assert_eq!( &ASTNode::SQLFunction { - name: r#""myfun""#.to_string(), + name: SQLObjectName(vec![r#""myfun""#.to_string()]), args: vec![], over: None, }, From 085e8a6b0405526b07ee882d75f071b06f5ce635 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sat, 20 Apr 2019 22:33:22 +0300 Subject: [PATCH 4/7] Use named fields in ExpressionWithAlias instead of a tuple (This produces more natural JSON representation when serializing AST with serde.) --- src/sqlast/query.rs | 4 ++-- src/sqlparser.rs | 2 +- tests/sqlparser_generic.rs | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/sqlast/query.rs b/src/sqlast/query.rs index f927f4b35..c5396d420 100644 --- a/src/sqlast/query.rs +++ b/src/sqlast/query.rs @@ -157,7 +157,7 @@ pub enum SQLSelectItem { /// Any expression, not followed by `[ AS ] alias` UnnamedExpression(ASTNode), /// An expression, followed by `[ AS ] alias` - ExpressionWithAlias(ASTNode, SQLIdent), + ExpressionWithAlias { expr: ASTNode, alias: SQLIdent }, /// `alias.*` or even `schema.table.*` QualifiedWildcard(SQLObjectName), /// An unqualified `*` @@ -168,7 +168,7 @@ impl ToString for SQLSelectItem { fn to_string(&self) -> String { match &self { SQLSelectItem::UnnamedExpression(expr) => expr.to_string(), - SQLSelectItem::ExpressionWithAlias(expr, alias) => { + SQLSelectItem::ExpressionWithAlias { expr, alias } => { format!("{} AS {}", expr.to_string(), alias) } SQLSelectItem::QualifiedWildcard(prefix) => format!("{}.*", prefix.to_string()), diff --git a/src/sqlparser.rs b/src/sqlparser.rs index ad97bb6fe..33eecbe8c 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -1660,7 +1660,7 @@ impl Parser { if let Some(alias) = self.parse_optional_alias(keywords::RESERVED_FOR_COLUMN_ALIAS)? { - projections.push(SQLSelectItem::ExpressionWithAlias(expr, alias)); + projections.push(SQLSelectItem::ExpressionWithAlias { expr, alias }); } else { projections.push(SQLSelectItem::UnnamedExpression(expr)); } diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_generic.rs index 1f8c46e4f..1a6bbb8bb 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_generic.rs @@ -95,12 +95,12 @@ fn parse_select_wildcard() { fn parse_column_aliases() { let sql = "SELECT a.col + 1 AS newname FROM foo AS a"; let select = verified_only_select(sql); - if let SQLSelectItem::ExpressionWithAlias( - ASTNode::SQLBinaryExpr { + if let SQLSelectItem::ExpressionWithAlias { + expr: ASTNode::SQLBinaryExpr { ref op, ref right, .. }, ref alias, - ) = only(&select.projection) + } = only(&select.projection) { assert_eq!(&SQLOperator::Plus, op); assert_eq!(&ASTNode::SQLValue(Value::Long(1)), right.as_ref()); @@ -643,7 +643,7 @@ fn parse_delimited_identifiers() { expr_from_projection(&select.projection[1]), ); match &select.projection[2] { - &SQLSelectItem::ExpressionWithAlias(ref expr, ref alias) => { + SQLSelectItem::ExpressionWithAlias { expr, alias } => { assert_eq!(&ASTNode::SQLIdentifier(r#""simple id""#.to_string()), expr); assert_eq!(r#""column alias""#, alias); } From 8c7e1d1c54907644220029571e2973bca9b5a8b4 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sat, 20 Apr 2019 22:49:29 +0300 Subject: [PATCH 5/7] Update sample output in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index dc6b17899..31a2eeee1 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ println!("AST: {:?}", ast); This outputs ```rust -AST: [SQLSelect(SQLSelect { projection: [SQLIdentifier("a"), SQLIdentifier("b"), SQLValue(Long(123)), SQLFunction { id: "myfunc", args: [SQLIdentifier("b")] }], relation: Some(Table { name: SQLObjectName(["table_1"]), alias: None }), joins: [], selection: Some(SQLBinaryExpr { left: SQLBinaryExpr { left: SQLIdentifier("a"), op: Gt, right: SQLIdentifier("b") }, op: And, right: SQLBinaryExpr { left: SQLIdentifier("b"), op: Lt, right: SQLValue(Long(100)) } }), order_by: Some([SQLOrderByExpr { expr: SQLIdentifier("a"), asc: Some(false) }, SQLOrderByExpr { expr: SQLIdentifier("b"), asc: None }]), group_by: None, having: None, limit: None })] +AST: [SQLSelect(SQLQuery { ctes: [], body: Select(SQLSelect { distinct: false, projection: [UnnamedExpression(SQLIdentifier("a")), UnnamedExpression(SQLIdentifier("b")), UnnamedExpression(SQLValue(Long(123))), UnnamedExpression(SQLFunction { name: SQLObjectName(["myfunc"]), args: [SQLIdentifier("b")], over: None })], relation: Some(Table { name: SQLObjectName(["table_1"]), alias: None }), joins: [], selection: Some(SQLBinaryExpr { left: SQLBinaryExpr { left: SQLIdentifier("a"), op: Gt, right: SQLIdentifier("b") }, op: And, right: SQLBinaryExpr { left: SQLIdentifier("b"), op: Lt, right: SQLValue(Long(100)) } }), group_by: None, having: None }), order_by: Some([SQLOrderByExpr { expr: SQLIdentifier("a"), asc: Some(false) }, SQLOrderByExpr { expr: SQLIdentifier("b"), asc: None }]), limit: None })] ``` ## Design From de057deff68a5b5bcd57acdee54ecce4c32d2a70 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sun, 21 Apr 2019 01:41:10 +0300 Subject: [PATCH 6/7] Fix clippy lints in comma_separated_string --- src/sqlast/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sqlast/mod.rs b/src/sqlast/mod.rs index a888d82cb..d12fd5a5c 100644 --- a/src/sqlast/mod.rs +++ b/src/sqlast/mod.rs @@ -31,9 +31,9 @@ pub use self::value::Value; pub use self::sql_operator::SQLOperator; /// Like `vec.join(", ")`, but for any types implementing ToString. -fn comma_separated_string(vec: &Vec) -> String { +fn comma_separated_string(vec: &[T]) -> String { vec.iter() - .map(|a| a.to_string()) + .map(T::to_string) .collect::>() .join(", ") } @@ -409,7 +409,7 @@ impl ToString for SQLStatement { values, } => { let mut s = format!("COPY {}", table_name.to_string()); - if columns.len() > 0 { + if !columns.is_empty() { s += &format!(" ({})", comma_separated_string(columns)); } s += " FROM stdin; "; @@ -432,8 +432,8 @@ impl ToString for SQLStatement { selection, } => { let mut s = format!("UPDATE {}", table_name.to_string()); - if assignments.len() > 0 { - s += &format!("{}", comma_separated_string(assignments)); + if !assignments.is_empty() { + s += &comma_separated_string(assignments); } if let Some(selection) = selection { s += &format!(" WHERE {}", selection.to_string()); @@ -473,7 +473,7 @@ impl ToString for SQLStatement { "CREATE EXTERNAL TABLE {} ({}) STORED AS {} LOCATION '{}'", name.to_string(), comma_separated_string(columns), - file_format.as_ref().map(|f| f.to_string()).unwrap(), + file_format.as_ref().unwrap().to_string(), location.as_ref().unwrap() ), SQLStatement::SQLCreateTable { From 9a244e059e508711bf3812c12b9a0f72e1628591 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sun, 21 Apr 2019 01:41:21 +0300 Subject: [PATCH 7/7] Fix clippy lints in "Support qualified function names" --- src/sqlparser.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/sqlparser.rs b/src/sqlparser.rs index 33eecbe8c..60728d599 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -188,22 +188,21 @@ impl Parser { ends_with_wildcard = true; break; } - _ => { + unexpected => { return parser_err!(format!( - "Error parsing compound identifier" + "Expected an identifier or a '*' after '.', got: {:?}", + unexpected )); } } } if ends_with_wildcard { Ok(ASTNode::SQLQualifiedWildcard(id_parts)) + } else if self.consume_token(&Token::LParen) { + self.prev_token(); + self.parse_function(SQLObjectName(id_parts)) } else { - if self.consume_token(&Token::LParen) { - self.prev_token(); - self.parse_function(SQLObjectName(id_parts)) - } else { - Ok(ASTNode::SQLCompoundIdentifier(id_parts)) - } + Ok(ASTNode::SQLCompoundIdentifier(id_parts)) } } _ => Ok(ASTNode::SQLIdentifier(w.as_sql_ident())),