Phoenix500526 commented on code in PR #22998:
URL: https://github.com/apache/datafusion/pull/22998#discussion_r3432570409
##########
datafusion/expr/src/expr_rewriter/mod.rs:
##########
@@ -80,28 +91,82 @@ pub fn normalize_col(expr: Expr, plan: &LogicalPlan) ->
Result<Expr> {
.data()
}
+/// Resolve a normalized column to itself, or -- when it is the merged key of a
+/// `RIGHT` / `FULL` `USING` / `NATURAL` join referenced *unqualified* -- to
the
+/// equivalent `COALESCE(left, right)`.
+///
+/// The COALESCE is built as `CASE WHEN key IS NOT NULL THEN key ELSE other
END`
+/// (the same form `coalesce` is simplified to, and buildable here without
+/// depending on the functions crate), aliased to the key name so the output
+/// column keeps its name.
+pub(crate) fn merged_using_key_or_column(
+ col: Column,
+ was_unqualified: bool,
+ outer_using_keys: &[(Column, Column)],
+) -> Result<Expr> {
+ if was_unqualified
+ && let Some((l, r)) = outer_using_keys
+ .iter()
+ .find(|(l, r)| l == &col || r == &col)
+ {
+ let other = if l == &col { r } else { l };
+ let case = CaseBuilder::new(
+ None,
+ vec![Expr::Column(col.clone()).is_not_null()],
+ vec![Expr::Column(col.clone())],
+ Some(Box::new(Expr::Column(other.clone()))),
+ )
+ .end()?;
+ return Ok(case.alias(col.name.clone()));
+ }
+ Ok(Expr::Column(col))
+}
+
/// See [`Column::normalize_with_schemas_and_ambiguity_check`] for usage
pub fn normalize_col_with_schemas_and_ambiguity_check(
expr: Expr,
schemas: &[&[&DFSchema]],
using_columns: &[HashSet<Column>],
+) -> Result<Expr> {
+ normalize_col_with_schemas_ambiguity_and_outer_using(
+ expr,
+ schemas,
+ using_columns,
+ &[],
+ )
+}
+
+/// Like [`normalize_col_with_schemas_and_ambiguity_check`], but additionally
+/// resolves the merged key of a `RIGHT` / `FULL` `USING` / `NATURAL` join
(given
+/// as `(left, right)` column pairs) to `COALESCE(left, right)`.
+pub fn normalize_col_with_schemas_ambiguity_and_outer_using(
Review Comment:
I drop this function and resolve the merged key in the SQL planner instead,
so the USING-outer-join detail no longer leaks into the generic expr public
API.
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -525,6 +525,39 @@ impl LogicalPlan {
Ok(using_columns)
}
+ /// Returns the `(left, right)` join-key column pairs of every `RIGHT` /
+ /// `FULL` `USING` / `NATURAL` join in this plan.
+ ///
+ /// For these join types the left key is NULL-padded on rows that exist
only
+ /// on the right, so an unqualified reference to the merged key must
resolve
+ /// to `COALESCE(left, right)` rather than to the left column alone.
+ pub fn outer_using_key_pairs(
Review Comment:
Rename `outer_using_key_pairs` to `right_or_full_using_key_pairs`
##########
datafusion/expr/src/expr_rewriter/mod.rs:
##########
@@ -80,28 +91,82 @@ pub fn normalize_col(expr: Expr, plan: &LogicalPlan) ->
Result<Expr> {
.data()
}
+/// Resolve a normalized column to itself, or -- when it is the merged key of a
+/// `RIGHT` / `FULL` `USING` / `NATURAL` join referenced *unqualified* -- to
the
+/// equivalent `COALESCE(left, right)`.
Review Comment:
Done
--
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]
