wirybeaver commented on code in PR #24429:
URL: https://github.com/apache/datafusion/pull/24429#discussion_r3839703210


##########
datafusion/expr/src/logical_plan/dml.rs:
##########
@@ -299,15 +311,71 @@ impl Display for InsertOp {
 }
 
 /// Describes a MERGE INTO operation's parameters.
+///
+/// [`Self::target_qualifier`] is the SQL-visible relation name used by
+/// expressions. The target's catalog/provider identity remains in
+/// [`DmlStatement::table_name`].
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)]
+#[non_exhaustive]

Review Comment:
   Done in a2377f0fc. I added 
`docs/source/library-user-guide/upgrading/55.1.0.md` with the `MergeIntoOp { 
on, clauses }` to `MergeIntoOp::new(target_qualifier, on, clauses)` migration 
and the reason provider identity and the SQL-visible qualifier are separate.



##########
datafusion/proto-models/src/generated/prost.rs:
##########
@@ -527,13 +527,17 @@ pub mod dml_node {
         }
     }
 }
-/// Carries the ON condition and WHEN clauses of a MERGE INTO operation.
+/// Carries the target qualifier, ON condition, and WHEN clauses of a MERGE 
INTO operation.
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct MergeIntoOpNode {
     #[prost(message, optional, boxed, tag = "1")]
     pub on: 
::core::option::Option<::prost::alloc::boxed::Box<LogicalExprNode>>,
     #[prost(message, repeated, tag = "2")]
     pub clauses: ::prost::alloc::vec::Vec<MergeIntoClauseNode>,
+    /// SQL-visible target qualifier. Absent in payloads written before this 
field
+    /// was introduced; readers then fall back to DmlNode.table_name.
+    #[prost(message, optional, tag = "3")]
+    pub target_qualifier: ::core::option::Option<TableReference>,

Review Comment:
   Done in a2377f0fc. The 55.1 guide now shows the generated `MergeIntoOpNode` 
literal migration with `target_qualifier: Some(...)` and documents both 55.0 → 
55.1 fallback compatibility and why 55.1 → 55.0 is unsupported.



##########
datafusion/core/tests/sql/sql_api.rs:
##########


Review Comment:
   Done in a2377f0fc. I moved the remaining NULL boolean-condition case to 
`merge_into.slt` and removed the Rust test and `assert_merge_physical_error` 
helper.



-- 
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]

Reply via email to