JanKaul commented on code in PR #15862:
URL: https://github.com/apache/datafusion/pull/15862#discussion_r2071316933


##########
datafusion/expr/src/logical_plan/dml.rs:
##########
@@ -241,3 +241,10 @@ fn make_count_schema() -> DFSchemaRef {
             .unwrap(),
     )
 }
+

Review Comment:
   Am I missing something or are not using this struct in you code?
   
   Generally I would argue that this struct should be more general, essentially 
a conversion from the AST representation into Datafusion concepts. Without 
performing the actual MERGE logic. This would mean converting the 
`TableFactor`s into `LogicalPlan::Scan` and the `MergeClauses` into a 
Datafusion structs containing the expressions.
   
   When keeping the struct more general, the actual MERGE logic can then be 
performed when planning the physical plan. Since Datafusion natively doesn't 
support UPDATEs and DELETEs, this leaves more room for extensions to provide 
this functionality.
   
   But that's just my point of view.



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to