From 7f4c9132d71029636f00a5c7ebe1d21898887c47 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 6 Mar 2023 17:43:22 +0100 Subject: [PATCH] Fix table alias parsing regression in 0.31.0 by backing out redshift column definition list (#827) * Fix table alias parsing regression * Revert "Support redshift's columns definition list for system information functions (#769)" This reverts commit c35dcc93a78f72a9060d76bba5ee6c292909e54a. --- src/ast/mod.rs | 29 -------------- src/ast/query.rs | 7 ---- src/keywords.rs | 2 - src/parser.rs | 46 ---------------------- src/test_utils.rs | 1 - tests/sqlparser_bigquery.rs | 1 - tests/sqlparser_clickhouse.rs | 3 -- tests/sqlparser_common.rs | 58 ++++++++++++++++++---------- tests/sqlparser_hive.rs | 2 - tests/sqlparser_mssql.rs | 2 - tests/sqlparser_mysql.rs | 3 -- tests/sqlparser_postgres.rs | 2 - tests/sqlparser_redshift.rs | 73 ----------------------------------- tests/sqlparser_snowflake.rs | 2 - 14 files changed, 37 insertions(+), 194 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 0d2303ca..73dd1f5b 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4150,35 +4150,6 @@ impl fmt::Display for SearchModifier { } } -/// A result table definition i.e. `cols(view_schema name, view_name name, col_name name, col_type varchar, col_num int)` -/// used for redshift functions: pg_get_late_binding_view_cols, pg_get_cols, pg_get_grantee_by_iam_role,pg_get_iam_role_by_user -#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct TableAliasDefinition { - pub name: Ident, - pub args: Vec, -} - -impl fmt::Display for TableAliasDefinition { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}({})", self.name, display_comma_separated(&self.args))?; - Ok(()) - } -} - -#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct IdentPair(pub Ident, pub Ident); - -impl fmt::Display for IdentPair { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} {}", self.0, self.1)?; - Ok(()) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/ast/query.rs b/src/ast/query.rs index 1feb7a4a..361edf0b 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -631,9 +631,6 @@ pub enum TableFactor { /// vector of arguments, in the case of a table-valued function call, /// whereas it's `None` in the case of a regular table name. args: Option>, - /// A table alias definition i.e. `cols(view_schema name, view_name name, col_name name, col_type varchar, col_num int)` - /// used for redshift functions: pg_get_late_binding_view_cols, pg_get_cols, pg_get_grantee_by_iam_role,pg_get_iam_role_by_user) - columns_definition: Option, /// MSSQL-specific `WITH (...)` hints such as NOLOCK. with_hints: Vec, }, @@ -682,7 +679,6 @@ impl fmt::Display for TableFactor { name, alias, args, - columns_definition, with_hints, } => { write!(f, "{name}")?; @@ -692,9 +688,6 @@ impl fmt::Display for TableFactor { if let Some(alias) = alias { write!(f, " AS {alias}")?; } - if let Some(columns_definition) = columns_definition { - write!(f, " {columns_definition}")?; - } if !with_hints.is_empty() { write!(f, " WITH ({})", display_comma_separated(with_hints))?; } diff --git a/src/keywords.rs b/src/keywords.rs index af741aee..dca9375e 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -671,7 +671,6 @@ pub const RESERVED_FOR_TABLE_ALIAS: &[Keyword] = &[ Keyword::OUTER, Keyword::SET, Keyword::QUALIFY, - Keyword::AS, ]; /// Can't be used as a column alias, so that `SELECT alias` @@ -701,5 +700,4 @@ pub const RESERVED_FOR_COLUMN_ALIAS: &[Keyword] = &[ // Reserved only as a column alias in the `SELECT` clause Keyword::FROM, Keyword::INTO, - Keyword::AS, ]; diff --git a/src/parser.rs b/src/parser.rs index f2009d31..f85e8d1b 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -5731,7 +5731,6 @@ impl<'a> Parser<'a> { } else { None }; - let columns_definition = self.parse_redshift_columns_definition_list()?; let alias = self.parse_optional_table_alias(keywords::RESERVED_FOR_TABLE_ALIAS)?; // MSSQL-specific table hints: let mut with_hints = vec![]; @@ -5748,56 +5747,11 @@ impl<'a> Parser<'a> { name, alias, args, - columns_definition, with_hints, }) } } - fn parse_redshift_columns_definition_list( - &mut self, - ) -> Result, ParserError> { - if !dialect_of!(self is RedshiftSqlDialect | GenericDialect) { - return Ok(None); - } - - if let Some(col_definition_list_name) = self.parse_optional_columns_definition_list_alias() - { - if self.consume_token(&Token::LParen) { - let names = self.parse_comma_separated(Parser::parse_ident_pair)?; - self.expect_token(&Token::RParen)?; - Ok(Some(TableAliasDefinition { - name: col_definition_list_name, - args: names, - })) - } else { - self.prev_token(); - Ok(None) - } - } else { - Ok(None) - } - } - - fn parse_optional_columns_definition_list_alias(&mut self) -> Option { - match self.next_token().token { - Token::Word(w) if !keywords::RESERVED_FOR_TABLE_ALIAS.contains(&w.keyword) => { - Some(w.to_ident()) - } - _ => { - self.prev_token(); - None - } - } - } - - fn parse_ident_pair(&mut self) -> Result { - Ok(IdentPair( - self.parse_identifier()?, - self.parse_identifier()?, - )) - } - pub fn parse_derived_table_factor( &mut self, lateral: IsLateral, diff --git a/src/test_utils.rs b/src/test_utils.rs index b2bff081..d5bafd90 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -185,7 +185,6 @@ pub fn table(name: impl Into) -> TableFactor { name: ObjectName(vec![Ident::new(name.into())]), alias: None, args: None, - columns_definition: None, with_hints: vec![], } } diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index ac3a55cc..11a8c6e5 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -99,7 +99,6 @@ fn parse_table_identifiers() { name: ObjectName(expected), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![] diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index dfea7f18..3ee3fbc0 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -60,7 +60,6 @@ fn parse_map_access_expr() { name: ObjectName(vec![Ident::new("foos")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![] @@ -165,13 +164,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"), diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 1e56b216..55690da3 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -210,7 +210,6 @@ fn parse_update_set_from() { name: ObjectName(vec![Ident::new("t1")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -237,7 +236,6 @@ fn parse_update_set_from() { name: ObjectName(vec![Ident::new("t1")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -300,7 +298,6 @@ fn parse_update_with_table_alias() { columns: vec![], }), args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -333,6 +330,43 @@ fn parse_update_with_table_alias() { } } +#[test] +fn parse_select_with_table_alias_as() { + // AS is optional + one_statement_parses_to( + "SELECT a, b, c FROM lineitem l (A, B, C)", + "SELECT a, b, c FROM lineitem AS l (A, B, C)", + ); +} + +#[test] +fn parse_select_with_table_alias() { + let select = verified_only_select("SELECT a, b, c FROM lineitem AS l (A, B, C)"); + assert_eq!( + select.projection, + vec![ + SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("a")),), + SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("b")),), + SelectItem::UnnamedExpr(Expr::Identifier(Ident::new("c")),), + ] + ); + assert_eq!( + select.from, + vec![TableWithJoins { + relation: TableFactor::Table { + name: ObjectName(vec![Ident::new("lineitem")]), + alias: Some(TableAlias { + name: Ident::new("l"), + columns: vec![Ident::new("A"), Ident::new("B"), Ident::new("C"),], + }), + args: None, + with_hints: vec![], + }, + joins: vec![], + }] + ); +} + #[test] fn parse_invalid_table_name() { let ast = all_dialects() @@ -356,7 +390,6 @@ fn parse_delete_statement() { name: ObjectName(vec![Ident::with_quote('"', "table")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, table_name @@ -383,7 +416,6 @@ fn parse_where_delete_statement() { name: ObjectName(vec![Ident::new("foo")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, table_name, @@ -424,7 +456,6 @@ fn parse_where_delete_with_alias_statement() { columns: vec![], }), args: None, - columns_definition: None, with_hints: vec![], }, table_name, @@ -438,7 +469,6 @@ fn parse_where_delete_with_alias_statement() { columns: vec![], }), args: None, - columns_definition: None, with_hints: vec![], }), using @@ -3465,7 +3495,6 @@ fn parse_interval_and_or_xor() { }]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -3981,7 +4010,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t1".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -3991,7 +4019,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t2".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -4009,7 +4036,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t1a".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![Join { @@ -4017,7 +4043,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t1b".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: JoinOperator::Inner(JoinConstraint::Natural), @@ -4028,7 +4053,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t2a".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![Join { @@ -4036,7 +4060,6 @@ fn parse_implicit_join() { name: ObjectName(vec!["t2b".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: JoinOperator::Inner(JoinConstraint::Natural), @@ -4057,7 +4080,6 @@ fn parse_cross_join() { name: ObjectName(vec![Ident::new("t2")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: JoinOperator::CrossJoin, @@ -4078,7 +4100,6 @@ fn parse_joins_on() { name: ObjectName(vec![Ident::new(relation.into())]), alias, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: f(JoinConstraint::On(Expr::BinaryOp { @@ -4148,7 +4169,6 @@ fn parse_joins_using() { name: ObjectName(vec![Ident::new(relation.into())]), alias, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: f(JoinConstraint::Using(vec!["c1".into()])), @@ -4210,7 +4230,6 @@ fn parse_natural_join() { name: ObjectName(vec![Ident::new("t2")]), alias, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: f(JoinConstraint::Natural), @@ -4475,7 +4494,6 @@ fn parse_derived_tables() { name: ObjectName(vec!["t2".into()]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, join_operator: JoinOperator::Inner(JoinConstraint::Natural), @@ -5767,7 +5785,6 @@ fn parse_merge() { columns: vec![], }), args: None, - columns_definition: None, with_hints: vec![], } ); @@ -5791,7 +5808,6 @@ fn parse_merge() { name: ObjectName(vec![Ident::new("s"), Ident::new("foo")]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], diff --git a/tests/sqlparser_hive.rs b/tests/sqlparser_hive.rs index 7c17ea12..064a090f 100644 --- a/tests/sqlparser_hive.rs +++ b/tests/sqlparser_hive.rs @@ -320,13 +320,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"), diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 874dccfa..41b0803e 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -152,13 +152,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"), diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index b4ae5cd5..04c86cb1 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -870,7 +870,6 @@ fn parse_update_with_joins() { columns: vec![] }), args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![Join { @@ -881,7 +880,6 @@ fn parse_update_with_joins() { columns: vec![] }), args: None, - columns_definition: None, with_hints: vec![], }, join_operator: JoinOperator::Inner(JoinConstraint::On(Expr::BinaryOp { @@ -1003,7 +1001,6 @@ fn parse_substring_in_select() { }]), alias: None, args: None, - columns_definition: None, with_hints: vec![] }, joins: vec![] diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 72f36465..6a3f88e3 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -2189,13 +2189,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"), diff --git a/tests/sqlparser_redshift.rs b/tests/sqlparser_redshift.rs index fdbd66e7..7597ee98 100644 --- a/tests/sqlparser_redshift.rs +++ b/tests/sqlparser_redshift.rs @@ -44,7 +44,6 @@ fn test_square_brackets_over_db_schema_table_name() { ]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -89,7 +88,6 @@ fn test_double_quotes_over_db_schema_table_name() { ]), alias: None, args: None, - columns_definition: None, with_hints: vec![], }, joins: vec![], @@ -109,13 +107,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"), @@ -276,72 +272,3 @@ fn test_sharp() { select.projection[0] ); } - -#[test] -fn test_parse_pg_get_late_binding_view_cols() { - let sql = "select * from pg_get_late_binding_view_cols() some_name_cols(view_schema name, view_name name, col_name name)"; - let expected = "SELECT * FROM pg_get_late_binding_view_cols() some_name_cols(view_schema name, view_name name, col_name name)"; - redshift().one_statement_parses_to(sql, expected); - - let select = redshift().verified_only_select(expected); - assert_eq!( - TableFactor::Table { - name: ObjectName(vec![Ident::new("pg_get_late_binding_view_cols")],), - args: Some(vec![]), - alias: None, - columns_definition: Some(TableAliasDefinition { - name: Ident::new("some_name_cols"), - args: vec![ - IdentPair(Ident::new("view_schema"), Ident::new("name")), - IdentPair(Ident::new("view_name"), Ident::new("name")), - IdentPair(Ident::new("col_name"), Ident::new("name")) - ] - }), - with_hints: vec![] - }, - select.from[0].relation - ); -} - -#[test] -fn test_parse_pg_get_cols() { - let sql = - "SELECT * FROM pg_get_cols() some_name(view_schema name, view_name name, col_name name)"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_grantee_by_iam_role() { - let sql = "SELECT grantee, grantee_type, cmd_type FROM pg_get_grantee_by_iam_role('arn:aws:iam::123456789012:role/Redshift-S3-Write') res_grantee(grantee text, grantee_type text, cmd_type text)"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_iam_role_by_user() { - let sql = "SELECT username, iam_role, cmd FROM pg_get_iam_role_by_user('reg_user1') res_iam_role(username text, iam_role text, cmd text)"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_late_binding_view_cols_in_select() { - let sql = "SELECT pg_get_late_binding_view_cols()"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_cols_in_select() { - let sql = "SELECT pg_get_cols()"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_grantee_by_iam_role_in_select() { - let sql = "SELECT pg_get_grantee_by_iam_role()"; - redshift().verified_stmt(sql); -} - -#[test] -fn test_parse_pg_get_iam_role_by_user_in_select() { - let sql = "SELECT pg_get_iam_role_by_user()"; - redshift().verified_stmt(sql); -} diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index eeaebf11..ec43e030 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -220,13 +220,11 @@ fn parse_delimited_identifiers() { name, alias, args, - columns_definition, with_hints, } => { assert_eq!(vec![Ident::with_quote('"', "a table")], name.0); assert_eq!(Ident::with_quote('"', "alias"), alias.unwrap().name); assert!(args.is_none()); - assert!(columns_definition.is_none()); assert!(with_hints.is_empty()); } _ => panic!("Expecting TableFactor::Table"),