Skip to content

Support NestedJoin with an alias #551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/ast/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,10 @@ pub enum TableFactor {
///
/// The parser may also accept non-standard nesting of bare tables for some
/// dialects, but the information about such nesting is stripped from AST.
NestedJoin(Box<TableWithJoins>),
NestedJoin {
table_with_joins: Box<TableWithJoins>,
alias: Option<TableAlias>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

},
}

impl fmt::Display for TableFactor {
Expand Down Expand Up @@ -438,7 +441,16 @@ impl fmt::Display for TableFactor {
}
Ok(())
}
TableFactor::NestedJoin(table_reference) => write!(f, "({})", table_reference),
TableFactor::NestedJoin {
table_with_joins,
alias,
} => {
write!(f, "({})", table_with_joins)?;
if let Some(alias) = alias {
write!(f, " AS {}", alias)?;
}
Ok(())
}
}
}
}
Expand Down
22 changes: 17 additions & 5 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3869,12 +3869,24 @@ impl<'a> Parser<'a> {
#[allow(clippy::if_same_then_else)]
if !table_and_joins.joins.is_empty() {
self.expect_token(&Token::RParen)?;
Ok(TableFactor::NestedJoin(Box::new(table_and_joins))) // (A)
} else if let TableFactor::NestedJoin(_) = &table_and_joins.relation {
let alias = self.parse_optional_table_alias(keywords::RESERVED_FOR_TABLE_ALIAS)?;
Ok(TableFactor::NestedJoin {
table_with_joins: Box::new(table_and_joins),
alias,
}) // (A)
} else if let TableFactor::NestedJoin {
table_with_joins: _,
alias: _,
} = &table_and_joins.relation
{
// (B): `table_and_joins` (what we found inside the parentheses)
// is a nested join `(foo JOIN bar)`, not followed by other joins.
self.expect_token(&Token::RParen)?;
Ok(TableFactor::NestedJoin(Box::new(table_and_joins)))
let alias = self.parse_optional_table_alias(keywords::RESERVED_FOR_TABLE_ALIAS)?;
Ok(TableFactor::NestedJoin {
table_with_joins: Box::new(table_and_joins),
alias,
})
} else if dialect_of!(self is SnowflakeDialect | GenericDialect) {
// Dialect-specific behavior: Snowflake diverges from the
// standard and from most of the other implementations by
Expand All @@ -3893,7 +3905,8 @@ impl<'a> Parser<'a> {
TableFactor::Derived { alias, .. }
| TableFactor::Table { alias, .. }
| TableFactor::UNNEST { alias, .. }
| TableFactor::TableFunction { alias, .. } => {
| TableFactor::TableFunction { alias, .. }
| TableFactor::NestedJoin { alias, .. } => {
// but not `FROM (mytable AS alias1) AS alias2`.
if let Some(inner_alias) = alias {
return Err(ParserError::ParserError(format!(
Expand All @@ -3906,7 +3919,6 @@ impl<'a> Parser<'a> {
// `(mytable AS alias)`
alias.replace(outer_alias);
}
TableFactor::NestedJoin(_) => unreachable!(),
};
}
// Do not store the extra set of parens in the AST
Expand Down
79 changes: 55 additions & 24 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3446,32 +3446,45 @@ fn parse_joins_using() {

#[test]
fn parse_natural_join() {
fn natural_join(f: impl Fn(JoinConstraint) -> JoinOperator) -> Join {
fn natural_join(f: impl Fn(JoinConstraint) -> JoinOperator, alias: Option<TableAlias>) -> Join {
Join {
relation: TableFactor::Table {
name: ObjectName(vec![Ident::new("t2")]),
alias: None,
alias,
args: None,
with_hints: vec![],
},
join_operator: f(JoinConstraint::Natural),
}
}

// if not specified, inner join as default
assert_eq!(
only(&verified_only_select("SELECT * FROM t1 NATURAL JOIN t2").from).joins,
vec![natural_join(JoinOperator::Inner)]
vec![natural_join(JoinOperator::Inner, None)]
);
// left join explicitly
assert_eq!(
only(&verified_only_select("SELECT * FROM t1 NATURAL LEFT JOIN t2").from).joins,
vec![natural_join(JoinOperator::LeftOuter)]
vec![natural_join(JoinOperator::LeftOuter, None)]
);

// right join explicitly
assert_eq!(
only(&verified_only_select("SELECT * FROM t1 NATURAL RIGHT JOIN t2").from).joins,
vec![natural_join(JoinOperator::RightOuter)]
vec![natural_join(JoinOperator::RightOuter, None)]
);

// full join explicitly
assert_eq!(
only(&verified_only_select("SELECT * FROM t1 NATURAL FULL JOIN t2").from).joins,
vec![natural_join(JoinOperator::FullOuter)]
vec![natural_join(JoinOperator::FullOuter, None)]
);

// natural join another table with alias
assert_eq!(
only(&verified_only_select("SELECT * FROM t1 NATURAL JOIN t2 AS t3").from).joins,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an interesting one -- it adds the alias to the entire NaturalJoin rather than aliasing t2 to t3 on one side of the input.

Is that what other implementations do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it alias t2 to t3 but not alias to the entire NaturalJoin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps i should delete this test? i thought at we need to protect the case that it incorrectly alias the whole natural join to t3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a good test and that we should leave it in

vec![natural_join(JoinOperator::Inner, table_alias("t3"))]
);

let sql = "SELECT * FROM t1 natural";
Expand Down Expand Up @@ -3519,6 +3532,21 @@ fn parse_join_nesting() {
from.joins,
vec![join(nest!(nest!(nest!(table("b"), table("c")))))]
);

let sql = "SELECT * FROM (a NATURAL JOIN b) AS c";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense to me 👍

let select = verified_only_select(sql);
let from = only(select.from);
assert_eq!(
from.relation,
TableFactor::NestedJoin {
table_with_joins: Box::new(TableWithJoins {
relation: table("a"),
joins: vec![join(table("b"))],
}),
alias: table_alias("c")
}
);
assert_eq!(from.joins, vec![]);
}

#[test]
Expand Down Expand Up @@ -3676,25 +3704,28 @@ fn parse_derived_tables() {
let from = only(select.from);
assert_eq!(
from.relation,
TableFactor::NestedJoin(Box::new(TableWithJoins {
relation: TableFactor::Derived {
lateral: false,
subquery: Box::new(verified_query("(SELECT 1) UNION (SELECT 2)")),
alias: Some(TableAlias {
name: "t1".into(),
columns: vec![],
})
},
joins: vec![Join {
relation: TableFactor::Table {
name: ObjectName(vec!["t2".into()]),
alias: None,
args: None,
with_hints: vec![],
TableFactor::NestedJoin {
table_with_joins: Box::new(TableWithJoins {
relation: TableFactor::Derived {
lateral: false,
subquery: Box::new(verified_query("(SELECT 1) UNION (SELECT 2)")),
alias: Some(TableAlias {
name: "t1".into(),
columns: vec![],
})
},
join_operator: JoinOperator::Inner(JoinConstraint::Natural),
}],
}))
joins: vec![Join {
relation: TableFactor::Table {
name: ObjectName(vec!["t2".into()]),
alias: None,
args: None,
with_hints: vec![],
},
join_operator: JoinOperator::Inner(JoinConstraint::Natural),
}],
}),
alias: None
}
);
}

Expand Down
7 changes: 3 additions & 4 deletions tests/sqlparser_snowflake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,9 @@ fn test_single_table_in_parenthesis_with_alias() {
"SELECT * FROM (a AS alias1 NATURAL JOIN b AS c)",
);

let res = snowflake_and_generic().parse_sql_statements("SELECT * FROM (a NATURAL JOIN b) c");
assert_eq!(
ParserError::ParserError("Expected end of statement, found: c".to_string()),
res.unwrap_err()
snowflake_and_generic().one_statement_parses_to(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

"SELECT * FROM (a NATURAL JOIN b) c",
"SELECT * FROM (a NATURAL JOIN b) AS c",
);

let res = snowflake().parse_sql_statements("SELECT * FROM (a b) c");
Expand Down
4 changes: 2 additions & 2 deletions tests/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ pub use sqlparser::test_utils::*;
#[macro_export]
macro_rules! nest {
($base:expr $(, $join:expr)*) => {
TableFactor::NestedJoin(Box::new(TableWithJoins {
TableFactor::NestedJoin { table_with_joins: Box::new(TableWithJoins {
relation: $base,
joins: vec![$(join($join)),*]
}))
}), alias: None}
};
}