kosiew commented on code in PR #22988:
URL: https://github.com/apache/datafusion/pull/22988#discussion_r3607281710
##########
datafusion/expr/src/logical_plan/dml.rs:
##########
@@ -299,12 +299,129 @@ impl Display for InsertOp {
}
/// Describes a MERGE INTO operation's parameters.
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct MergeIntoOp {
/// The join condition from `ON <expr>`.
pub on: Expr,
/// The WHEN clauses, in the order they appeared in the SQL.
pub clauses: Vec<MergeIntoClause>,
+ /// Schema of the target table (qualified with the alias or table name used
+ /// in the SQL). Stored here so the analyzer can build a combined
+ /// target+source schema for type coercion and function rewrites.
+ pub target_schema: DFSchemaRef,
Review Comment:
Could we avoid adding `target_schema` as a new public field on `MergeIntoOp`?
Because `MergeIntoOp` is a public struct that downstream users can construct
with struct literals, adding a required field is a SemVer-breaking change.
I do not think that break is necessary for this PR. `target_schema` looks
like analyzer and planning context rather than essential public MERGE operation
data. Exposing it here makes downstream callers provide internal state they
should not need to know about.
Can we keep the additional planning state outside the publicly constructible
struct, or introduce a SemVer-compatible constructor or internal representation
before requiring new state from callers?
One possible approach is to keep `MergeIntoOp { on, clauses }`, derive the
target schema in the analyzer, type coercion, or function rewrite passes from
the existing `DmlStatement { table_name, target, .. }`, and canonicalize
alias-qualified target references during SQL planning.
--
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]
