alamb commented on code in PR #11636:
URL: https://github.com/apache/datafusion/pull/11636#discussion_r1691097502
##########
datafusion/sql/src/unparser/plan.rs:
##########
@@ -582,9 +552,62 @@ impl Unparser<'_> {
}
}
+ /// Convert the components of a USING clause to the USING AST
+ fn join_using_to_sql(&self, join_conditions: &[(Expr, Expr)]) ->
Result<ast::JoinConstraint> {
+ let mut idents = Vec::with_capacity(join_conditions.len());
+ for (left, right) in join_conditions {
+ match (left, right) {
+ (Expr::Column(Column { relation: _, name: left_name }),
Expr::Column(Column { relation: _, name: right_name })) => {
+ if left_name != right_name {
+ // USING is only valid when the column names are the
+ // same, so they should never be different
+ return not_impl_err!("Unsupported USING with different
column names");
+ }
+
idents.push(self.new_ident_quoted_if_needs(left_name.to_string()));
+ },
+ // USING is only valid with column names; arbitrary expressions
+ // are not allowed
+ _ => return not_impl_err!("Unsupported USING with non-column
expressions"),
+ }
+ }
+ Ok(ast::JoinConstraint::Using(idents))
+ }
+
+ /// Convert a join constraint and associated conditions and filter to a
SQL AST node
+ fn join_constraint_to_sql(&self, constraint: JoinConstraint, conditions:
&[(Expr, Expr)], filter: Option<&Expr>) -> Result<ast::JoinConstraint> {
+ match (constraint, conditions, filter) {
+ // No constraints
+ (JoinConstraint::On, [], None) => Ok(ast::JoinConstraint::None),
+ // Only equi-join conditions
+ (JoinConstraint::On, conditions, None) => {
+ let expr = self.join_conditions_to_sql(conditions,
ast::BinaryOperator::Eq)?;
+ match expr {
+ Some(expr) => Ok(ast::JoinConstraint::On(expr)),
+ None => Ok(ast::JoinConstraint::None),
+ }
+ },
+ // More complex filter with non-equi-join conditions; so we combine
+ // all conditions into a single AST Expr
+ (JoinConstraint::On, conditions, Some(filter)) => {
+ let filter_expr = self.expr_to_sql(filter)?;
+ let expr = self.join_conditions_to_sql(conditions,
ast::BinaryOperator::Eq)?;
+ match expr {
+ Some(expr) => {
+ let join_expr = self.and_op_to_sql(filter_expr, expr);
+ Ok(ast::JoinConstraint::On(join_expr))
+ },
+ None => Ok(ast::JoinConstraint::On(filter_expr)),
+ }
+ },
+
+ (JoinConstraint::Using, conditions, None) =>
self.join_using_to_sql(conditions),
+ (JoinConstraint::Using, _, Some(_)) => not_impl_err!("Unsupported
USING with filter"),
+ }
+ }
+
fn join_conditions_to_sql(
&self,
- join_conditions: &Vec<(Expr, Expr)>,
+ join_conditions: &[(Expr, Expr)],
Review Comment:
I agree -- this is a nice refactoring and very nicely commented code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
