diff --git a/src/dialect/keywords.rs b/src/dialect/keywords.rs index a6188c993..c3489ac67 100644 --- a/src/dialect/keywords.rs +++ b/src/dialect/keywords.rs @@ -420,16 +420,16 @@ define_keywords!( /// can be parsed unambiguously without looking ahead. pub const RESERVED_FOR_TABLE_ALIAS: &[&str] = &[ // Reserved as both a table and a column alias: - WITH, SELECT, WHERE, GROUP, ORDER, UNION, EXCEPT, INTERSECT, + WITH, SELECT, WHERE, GROUP, HAVING, ORDER, LIMIT, OFFSET, FETCH, UNION, EXCEPT, INTERSECT, // Reserved only as a table alias in the `FROM`/`JOIN` clauses: - ON, JOIN, INNER, CROSS, FULL, LEFT, RIGHT, NATURAL, USING, LIMIT, OFFSET, FETCH, + ON, JOIN, INNER, CROSS, FULL, LEFT, RIGHT, NATURAL, USING, ]; /// Can't be used as a column alias, so that `SELECT alias` /// can be parsed unambiguously without looking ahead. pub const RESERVED_FOR_COLUMN_ALIAS: &[&str] = &[ // Reserved as both a table and a column alias: - WITH, SELECT, WHERE, GROUP, ORDER, UNION, EXCEPT, INTERSECT, + WITH, SELECT, WHERE, GROUP, HAVING, ORDER, LIMIT, OFFSET, FETCH, UNION, EXCEPT, INTERSECT, // Reserved only as a column alias in the `SELECT` clause: FROM, ]; diff --git a/src/sqlast/query.rs b/src/sqlast/query.rs index f9e9eaf6e..4c23c0af1 100644 --- a/src/sqlast/query.rs +++ b/src/sqlast/query.rs @@ -163,18 +163,13 @@ impl ToString for SQLSelect { /// number of columns in the query matches the number of columns in the query. #[derive(Debug, Clone, PartialEq, Hash)] pub struct Cte { - pub alias: SQLIdent, + pub alias: TableAlias, pub query: SQLQuery, - pub renamed_columns: Vec, } impl ToString for Cte { fn to_string(&self) -> String { - let mut s = self.alias.clone(); - if !self.renamed_columns.is_empty() { - s += &format!(" ({})", comma_separated_string(&self.renamed_columns)); - } - s + &format!(" AS ({})", self.query.to_string()) + format!("{} AS ({})", self.alias.to_string(), self.query.to_string()) } } diff --git a/src/sqlparser.rs b/src/sqlparser.rs index 6e9941801..6924a9329 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -574,13 +574,13 @@ impl Parser { self.expected("IN or BETWEEN after NOT", self.peek_token()) } } - // Can only happen if `get_precedence` got out of sync with this function + // Can only happen if `get_next_precedence` got out of sync with this function _ => panic!("No infix parser for token {:?}", tok), } } else if Token::DoubleColon == tok { self.parse_pg_cast(expr) } else { - // Can only happen if `get_precedence` got out of sync with this function + // Can only happen if `get_next_precedence` got out of sync with this function panic!("No infix parser for token {:?}", tok) } } @@ -636,7 +636,7 @@ impl Parser { /// Get the precedence of the next token pub fn get_next_precedence(&self) -> Result { if let Some(token) = self.peek_token() { - debug!("get_precedence() {:?}", token); + debug!("get_next_precedence() {:?}", token); match &token { Token::SQLWord(k) if k.keyword == "OR" => Ok(5), @@ -1475,14 +1475,15 @@ impl Parser { fn parse_cte_list(&mut self) -> Result, ParserError> { let mut cte = vec![]; loop { - let alias = self.parse_identifier()?; - let renamed_columns = self.parse_parenthesized_column_list(Optional)?; + let alias = TableAlias { + name: self.parse_identifier()?, + columns: self.parse_parenthesized_column_list(Optional)?, + }; self.expect_keyword("AS")?; self.expect_token(&Token::LParen)?; cte.push(Cte { alias, query: self.parse_query()?, - renamed_columns, }); self.expect_token(&Token::RParen)?; if !self.consume_token(&Token::Comma) { @@ -1565,6 +1566,11 @@ impl Parser { } let projection = self.parse_select_list()?; + // Note that for keywords to be properly handled here, they need to be + // added to `RESERVED_FOR_COLUMN_ALIAS` / `RESERVED_FOR_TABLE_ALIAS`, + // otherwise they may be parsed as an alias as part of the `projection` + // or `from`. + let mut from = vec![]; if self.parse_keyword("FROM") { loop { @@ -1605,6 +1611,10 @@ impl Parser { pub fn parse_table_and_joins(&mut self) -> Result { let relation = self.parse_table_factor()?; + + // Note that for keywords to be properly handled here, they need to be + // added to `RESERVED_FOR_TABLE_ALIAS`, otherwise they may be parsed as + // a table alias. let mut joins = vec![]; loop { let join = if self.parse_keyword("CROSS") { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index a32ac6eac..7b4341fb8 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -212,13 +212,13 @@ fn parse_simple_select() { } #[test] -fn parse_select_with_limit_but_no_where() { - let sql = "SELECT id, fname, lname FROM customer LIMIT 5"; - let select = verified_only_select(sql); - assert_eq!(false, select.distinct); - assert_eq!(3, select.projection.len()); - let select = verified_query(sql); - assert_eq!(Some(ASTNode::SQLValue(Value::Long(5))), select.limit); +fn parse_limit_is_not_an_alias() { + // In dialects supporting LIMIT it shouldn't be parsed as a table alias + let ast = verified_query("SELECT id FROM customer LIMIT 1"); + assert_eq!(Some(ASTNode::SQLValue(Value::Long(1))), ast.limit); + + let ast = verified_query("SELECT 1 LIMIT 5"); + assert_eq!(Some(ASTNode::SQLValue(Value::Long(5))), ast.limit); } #[test] @@ -789,6 +789,29 @@ fn parse_select_group_by() { ); } +#[test] +fn parse_select_having() { + let sql = "SELECT foo FROM bar GROUP BY foo HAVING COUNT(*) > 1"; + let select = verified_only_select(sql); + assert_eq!( + Some(ASTNode::SQLBinaryOp { + left: Box::new(ASTNode::SQLFunction(SQLFunction { + name: SQLObjectName(vec!["COUNT".to_string()]), + args: vec![ASTNode::SQLWildcard], + over: None, + distinct: false + })), + op: SQLBinaryOperator::Gt, + right: Box::new(ASTNode::SQLValue(Value::Long(1))) + }), + select.having + ); + + let sql = "SELECT 'foo' HAVING 1 = 1"; + let select = verified_only_select(sql); + assert!(select.having.is_some()); +} + #[test] fn parse_limit_accepts_all() { one_statement_parses_to( @@ -1791,14 +1814,10 @@ fn parse_ctes() { fn assert_ctes_in_select(expected: &[&str], sel: &SQLQuery) { let mut i = 0; for exp in expected { - let Cte { - query, - alias, - renamed_columns, - } = &sel.ctes[i]; + let Cte { alias, query } = &sel.ctes[i]; assert_eq!(*exp, query.to_string()); - assert_eq!(if i == 0 { "a" } else { "b" }, alias); - assert!(renamed_columns.is_empty()); + assert_eq!(if i == 0 { "a" } else { "b" }, alias.name); + assert!(alias.columns.is_empty()); i += 1; } } @@ -1841,7 +1860,7 @@ fn parse_cte_renamed_columns() { let query = all_dialects().verified_query(sql); assert_eq!( vec!["col1", "col2"], - query.ctes.first().unwrap().renamed_columns + query.ctes.first().unwrap().alias.columns ); } @@ -2201,6 +2220,8 @@ fn parse_offset() { }, _ => panic!("Test broke"), } + let ast = verified_query("SELECT 'foo' OFFSET 0 ROWS"); + assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(0)))); } #[test] @@ -2213,15 +2234,15 @@ fn parse_singular_row_offset() { #[test] fn parse_fetch() { + const FETCH_FIRST_TWO_ROWS_ONLY: Fetch = Fetch { + with_ties: false, + percent: false, + quantity: Some(ASTNode::SQLValue(Value::Long(2))), + }; let ast = verified_query("SELECT foo FROM bar FETCH FIRST 2 ROWS ONLY"); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); + let ast = verified_query("SELECT 'foo' FETCH FIRST 2 ROWS ONLY"); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); let ast = verified_query("SELECT foo FROM bar FETCH FIRST ROWS ONLY"); assert_eq!( ast.fetch, @@ -2232,23 +2253,9 @@ fn parse_fetch() { }) ); let ast = verified_query("SELECT foo FROM bar WHERE foo = 4 FETCH FIRST 2 ROWS ONLY"); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); let ast = verified_query("SELECT foo FROM bar ORDER BY baz FETCH FIRST 2 ROWS ONLY"); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); let ast = verified_query( "SELECT foo FROM bar WHERE foo = 4 ORDER BY baz FETCH FIRST 2 ROWS WITH TIES", ); @@ -2273,36 +2280,15 @@ fn parse_fetch() { "SELECT foo FROM bar WHERE foo = 4 ORDER BY baz OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY", ); assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(2)))); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); let ast = verified_query( "SELECT foo FROM (SELECT * FROM bar FETCH FIRST 2 ROWS ONLY) FETCH FIRST 2 ROWS ONLY", ); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); match ast.body { SQLSetExpr::Select(s) => match only(s.from).relation { TableFactor::Derived { subquery, .. } => { - assert_eq!( - subquery.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); } _ => panic!("Test broke"), }, @@ -2310,26 +2296,12 @@ fn parse_fetch() { } let ast = verified_query("SELECT foo FROM (SELECT * FROM bar OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY) OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY"); assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(2)))); - assert_eq!( - ast.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); match ast.body { SQLSetExpr::Select(s) => match only(s.from).relation { TableFactor::Derived { subquery, .. } => { assert_eq!(subquery.offset, Some(ASTNode::SQLValue(Value::Long(2)))); - assert_eq!( - subquery.fetch, - Some(Fetch { - with_ties: false, - percent: false, - quantity: Some(ASTNode::SQLValue(Value::Long(2))), - }) - ); + assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY)); } _ => panic!("Test broke"), },