Skip to content

Commit

Permalink
Fix table alias parsing regression in 0.31.0 by backing out redshift …
Browse files Browse the repository at this point in the history 
…column definition list (#827)

* Fix table alias parsing regression

* Revert "Support redshift's columns definition list for system information functions (#769)"

This reverts commit c35dcc9.
  • Loading branch information
@alamb
alamb authored Mar 6, 2023
1 parent d69b875 commit 7f4c913
Show file tree
Hide file tree
Showing 14 changed files with 37 additions and 194 deletions.
29 changes: 0 additions & 29 deletions src/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<IdentPair>,
}

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::*;
Expand Down
7 changes: 0 additions & 7 deletions src/ast/query.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<FunctionArg>>,
/// 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<TableAliasDefinition>,
/// MSSQL-specific `WITH (...)` hints such as NOLOCK.
with_hints: Vec<Expr>,
},
Expand Down Expand Up @@ -682,7 +679,6 @@ impl fmt::Display for TableFactor {
name,
alias,
args,
columns_definition,
with_hints,
} => {
write!(f, "{name}")?;
Expand All @@ -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))?;
}
Expand Down
2 changes: 0 additions & 2 deletions src/keywords.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <expr> alias`
Expand Down Expand Up @@ -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,
];
46 changes: 0 additions & 46 deletions src/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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![];
Expand All @@ -5748,56 +5747,11 @@ impl<'a> Parser<'a> {
name,
alias,
args,
columns_definition,
with_hints,
})
}
}

fn parse_redshift_columns_definition_list(
&mut self,
) -> Result<Option<TableAliasDefinition>, 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<Ident> {
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<IdentPair, ParserError> {
Ok(IdentPair(
self.parse_identifier()?,
self.parse_identifier()?,
))
}

pub fn parse_derived_table_factor(
&mut self,
lateral: IsLateral,
Expand Down
1 change: 0 additions & 1 deletion src/test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ pub fn table(name: impl Into<String>) -> TableFactor {
name: ObjectName(vec![Ident::new(name.into())]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
}
}
Expand Down
1 change: 0 additions & 1 deletion tests/sqlparser_bigquery.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ fn parse_table_identifiers() {
name: ObjectName(expected),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
joins: vec![]
Expand Down
3 changes: 0 additions & 3 deletions tests/sqlparser_clickhouse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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![]
Expand Down Expand Up @@ -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"),
Expand Down
58 changes: 37 additions & 21 deletions tests/sqlparser_common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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![],
Expand All @@ -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![],
Expand Down Expand Up @@ -300,7 +298,6 @@ fn parse_update_with_table_alias() {
columns: vec![],
}),
args: None,
columns_definition: None,
with_hints: vec![],
},
joins: vec![],
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -424,7 +456,6 @@ fn parse_where_delete_with_alias_statement() {
columns: vec![],
}),
args: None,
columns_definition: None,
with_hints: vec![],
},
table_name,
Expand All @@ -438,7 +469,6 @@ fn parse_where_delete_with_alias_statement() {
columns: vec![],
}),
args: None,
columns_definition: None,
with_hints: vec![],
}),
using
Expand Down Expand Up @@ -3465,7 +3495,6 @@ fn parse_interval_and_or_xor() {
}]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
joins: vec![],
Expand Down Expand Up @@ -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![],
Expand All @@ -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![],
Expand All @@ -4009,15 +4036,13 @@ fn parse_implicit_join() {
name: ObjectName(vec!["t1a".into()]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
joins: vec![Join {
relation: TableFactor::Table {
name: ObjectName(vec!["t1b".into()]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
join_operator: JoinOperator::Inner(JoinConstraint::Natural),
Expand All @@ -4028,15 +4053,13 @@ fn parse_implicit_join() {
name: ObjectName(vec!["t2a".into()]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
joins: vec![Join {
relation: TableFactor::Table {
name: ObjectName(vec!["t2b".into()]),
alias: None,
args: None,
columns_definition: None,
with_hints: vec![],
},
join_operator: JoinOperator::Inner(JoinConstraint::Natural),
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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()])),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -5767,7 +5785,6 @@ fn parse_merge() {
columns: vec![],
}),
args: None,
columns_definition: None,
with_hints: vec![],
}
);
Expand All @@ -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![],
Expand Down
Loading

0 comments on commit 7f4c913

Please sign in to comment.