This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 849bbe75b4 Remove Expr clones in `select_to_plan` (#12887)
849bbe75b4 is described below
commit 849bbe75b446d38ac95cf09e457cdf4e92d4c9e2
Author: Jonah Gao <[email protected]>
AuthorDate: Mon Oct 14 19:37:55 2024 +0800
Remove Expr clones in `select_to_plan` (#12887)
---
datafusion/expr/src/utils.rs | 9 ++++++---
datafusion/sql/src/select.rs | 13 ++++---------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs
index 02b36d0fea..06cf1ec693 100644
--- a/datafusion/expr/src/utils.rs
+++ b/datafusion/expr/src/utils.rs
@@ -600,7 +600,7 @@ pub fn group_window_expr_by_sort_keys(
/// Collect all deeply nested `Expr::AggregateFunction`.
/// They are returned in order of occurrence (depth
/// first), with duplicates omitted.
-pub fn find_aggregate_exprs(exprs: &[Expr]) -> Vec<Expr> {
+pub fn find_aggregate_exprs<'a>(exprs: impl IntoIterator<Item = &'a Expr>) ->
Vec<Expr> {
find_exprs_in_exprs(exprs, &|nested_expr| {
matches!(nested_expr, Expr::AggregateFunction { .. })
})
@@ -625,12 +625,15 @@ pub fn find_out_reference_exprs(expr: &Expr) -> Vec<Expr>
{
/// Search the provided `Expr`'s, and all of their nested `Expr`, for any that
/// pass the provided test. The returned `Expr`'s are deduplicated and returned
/// in order of appearance (depth first).
-fn find_exprs_in_exprs<F>(exprs: &[Expr], test_fn: &F) -> Vec<Expr>
+fn find_exprs_in_exprs<'a, F>(
+ exprs: impl IntoIterator<Item = &'a Expr>,
+ test_fn: &F,
+) -> Vec<Expr>
where
F: Fn(&Expr) -> bool,
{
exprs
- .iter()
+ .into_iter()
.flat_map(|expr| find_exprs_in_expr(expr, test_fn))
.fold(vec![], |mut acc, expr| {
if !acc.contains(&expr) {
diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs
index 69c7745165..c665dec21d 100644
--- a/datafusion/sql/src/select.rs
+++ b/datafusion/sql/src/select.rs
@@ -137,16 +137,11 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
})
.transpose()?;
- // The outer expressions we will search through for
- // aggregates. Aggregates may be sourced from the SELECT...
- let mut aggr_expr_haystack = select_exprs.clone();
- // ... or from the HAVING.
- if let Some(having_expr) = &having_expr_opt {
- aggr_expr_haystack.push(having_expr.clone());
- }
-
+ // The outer expressions we will search through for aggregates.
+ // Aggregates may be sourced from the SELECT list or from the HAVING
expression.
+ let aggr_expr_haystack =
select_exprs.iter().chain(having_expr_opt.iter());
// All of the aggregate expressions (deduplicated).
- let aggr_exprs = find_aggregate_exprs(&aggr_expr_haystack);
+ let aggr_exprs = find_aggregate_exprs(aggr_expr_haystack);
// All of the group by expressions
let group_by_exprs = if let GroupByExpr::Expressions(exprs, _) =
select.group_by {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
