alamb commented on code in PR #10427:
URL: https://github.com/apache/datafusion/pull/10427#discussion_r1595719478
##########
datafusion/expr/src/utils.rs:
##########
@@ -909,8 +909,8 @@ pub fn check_all_columns_from_schema(
pub fn find_valid_equijoin_key_pair(
left_key: &Expr,
right_key: &Expr,
- left_schema: DFSchemaRef,
- right_schema: DFSchemaRef,
+ left_schema: &DFSchema,
Review Comment:
I think in general we should use a reference when the underlying callsite
doesn't actually need a owned copy
So in this case, the callsite just needs a `&DFSchema` so there is no need
to go through the overhead of creating the `Arc`
I think the runtime overhead of creating another Arc is likely to be
unmeasurable in all but the most extreme circumstances. However I think the
code is often easier to reason about
--
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
