joroKr21 commented on code in PR #17675:
URL: https://github.com/apache/datafusion/pull/17675#discussion_r2385166841


##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -175,7 +177,7 @@ impl LogicalPlanBuilder {
     pub fn to_recursive_query(
         self,
         name: String,
-        recursive_term: LogicalPlan,
+        recursive_term: impl Into<Arc<LogicalPlan>>,

Review Comment:
   It would work but it would make it more restrictive on the caller side. with 
`impl Into<Arc<LogicalPlan>>` they can pass in either `LogicalPlan` or 
`Arc<LogicalPlan>`. I guess there's also an argument to be made that if we 
force callers to change maybe they would discover some redundant cloning but I 
feel like it's unnecessary breakage.



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