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/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 881d03f72c [Minor] Avoid mem copy in generate window exprs (#8718)
881d03f72c is described below
commit 881d03f72cddec7e1cd659ef0c748760c6177b1c
Author: Yang Jiang <[email protected]>
AuthorDate: Thu Jan 4 06:02:38 2024 +0800
[Minor] Avoid mem copy in generate window exprs (#8718)
---
datafusion/expr/src/logical_plan/builder.rs | 4 ++--
datafusion/expr/src/utils.rs | 30 ++++++++++++++---------------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/datafusion/expr/src/logical_plan/builder.rs
b/datafusion/expr/src/logical_plan/builder.rs
index cfc052cfc1..a684f3e974 100644
--- a/datafusion/expr/src/logical_plan/builder.rs
+++ b/datafusion/expr/src/logical_plan/builder.rs
@@ -292,7 +292,7 @@ impl LogicalPlanBuilder {
window_exprs: Vec<Expr>,
) -> Result<LogicalPlan> {
let mut plan = input;
- let mut groups = group_window_expr_by_sort_keys(&window_exprs)?;
+ let mut groups = group_window_expr_by_sort_keys(window_exprs)?;
// To align with the behavior of PostgreSQL, we want the sort_keys
sorted as same rule as PostgreSQL that first
// we compare the sort key themselves and if one window's sort keys
are a prefix of another
// put the window with more sort keys first. so more deeply sorted
plans gets nested further down as children.
@@ -314,7 +314,7 @@ impl LogicalPlanBuilder {
key_b.len().cmp(&key_a.len())
});
for (_, exprs) in groups {
- let window_exprs = exprs.into_iter().cloned().collect::<Vec<_>>();
+ let window_exprs = exprs.into_iter().collect::<Vec<_>>();
// Partition and sorting is done at physical level, see the
EnforceDistribution
// and EnforceSorting rules.
plan = LogicalPlanBuilder::from(plan)
diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs
index e3ecdf154e..914b354d29 100644
--- a/datafusion/expr/src/utils.rs
+++ b/datafusion/expr/src/utils.rs
@@ -575,14 +575,14 @@ pub fn compare_sort_expr(
/// group a slice of window expression expr by their order by expressions
pub fn group_window_expr_by_sort_keys(
- window_expr: &[Expr],
-) -> Result<Vec<(WindowSortKey, Vec<&Expr>)>> {
+ window_expr: Vec<Expr>,
+) -> Result<Vec<(WindowSortKey, Vec<Expr>)>> {
let mut result = vec![];
- window_expr.iter().try_for_each(|expr| match expr {
- Expr::WindowFunction(WindowFunction{ partition_by, order_by, .. }) => {
+ window_expr.into_iter().try_for_each(|expr| match &expr {
+ Expr::WindowFunction( WindowFunction{ partition_by, order_by, .. }) =>
{
let sort_key = generate_sort_key(partition_by, order_by)?;
if let Some((_, values)) = result.iter_mut().find(
- |group: &&mut (WindowSortKey, Vec<&Expr>)| matches!(group,
(key, _) if *key == sort_key),
+ |group: &&mut (WindowSortKey, Vec<Expr>)| matches!(group,
(key, _) if *key == sort_key),
) {
values.push(expr);
} else {
@@ -1239,8 +1239,8 @@ mod tests {
#[test]
fn test_group_window_expr_by_sort_keys_empty_case() -> Result<()> {
- let result = group_window_expr_by_sort_keys(&[])?;
- let expected: Vec<(WindowSortKey, Vec<&Expr>)> = vec![];
+ let result = group_window_expr_by_sort_keys(vec![])?;
+ let expected: Vec<(WindowSortKey, Vec<Expr>)> = vec![];
assert_eq!(expected, result);
Ok(())
}
@@ -1276,10 +1276,10 @@ mod tests {
WindowFrame::new(false),
));
let exprs = &[max1.clone(), max2.clone(), min3.clone(), sum4.clone()];
- let result = group_window_expr_by_sort_keys(exprs)?;
+ let result = group_window_expr_by_sort_keys(exprs.to_vec())?;
let key = vec![];
- let expected: Vec<(WindowSortKey, Vec<&Expr>)> =
- vec![(key, vec![&max1, &max2, &min3, &sum4])];
+ let expected: Vec<(WindowSortKey, Vec<Expr>)> =
+ vec![(key, vec![max1, max2, min3, sum4])];
assert_eq!(expected, result);
Ok(())
}
@@ -1320,7 +1320,7 @@ mod tests {
));
// FIXME use as_ref
let exprs = &[max1.clone(), max2.clone(), min3.clone(), sum4.clone()];
- let result = group_window_expr_by_sort_keys(exprs)?;
+ let result = group_window_expr_by_sort_keys(exprs.to_vec())?;
let key1 = vec![(age_asc.clone(), false), (name_desc.clone(), false)];
let key2 = vec![];
@@ -1330,10 +1330,10 @@ mod tests {
(created_at_desc, false),
];
- let expected: Vec<(WindowSortKey, Vec<&Expr>)> = vec![
- (key1, vec![&max1, &min3]),
- (key2, vec![&max2]),
- (key3, vec![&sum4]),
+ let expected: Vec<(WindowSortKey, Vec<Expr>)> = vec![
+ (key1, vec![max1, min3]),
+ (key2, vec![max2]),
+ (key3, vec![sum4]),
];
assert_eq!(expected, result);
Ok(())
