-
Notifications
You must be signed in to change notification settings - Fork 2k
Refactor into LexOrdering::collapse, LexRequirement::collapse avoid clone
#14038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
178bc51
15558f5
1d2c339
d2fe8a4
a44acfd
149a884
2461f7d
ee1c16f
cf323b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
| use std::sync::Arc; | ||
|
|
||
| use crate::expressions::Column; | ||
| use crate::{LexRequirement, PhysicalExpr, PhysicalSortRequirement}; | ||
| use crate::{LexRequirement, PhysicalExpr}; | ||
|
|
||
| use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode}; | ||
|
|
||
|
|
@@ -41,14 +41,9 @@ pub use properties::{ | |
| /// It will also filter out entries that are ordered if the next entry is; | ||
| /// for instance, `vec![floor(a) Some(ASC), a Some(ASC)]` will be collapsed to | ||
| /// `vec![a Some(ASC)]`. | ||
| #[deprecated(since = "45.0.0", note = "Use LexRequirement::collapse")] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is part of the public API: https://docs.rs/datafusion/latest/datafusion/physical_expr/equivalence/fn.collapse_lex_req.html |
||
| pub fn collapse_lex_req(input: LexRequirement) -> LexRequirement { | ||
| let mut output = Vec::<PhysicalSortRequirement>::new(); | ||
| for item in input { | ||
| if !output.iter().any(|req| req.expr.eq(&item.expr)) { | ||
| output.push(item); | ||
| } | ||
| } | ||
| LexRequirement::new(output) | ||
| input.collapse() | ||
| } | ||
|
|
||
| /// Adds the `offset` value to `Column` indices inside `expr`. This function is | ||
|
|
@@ -80,7 +75,9 @@ mod tests { | |
| use arrow::datatypes::{DataType, Field, Schema}; | ||
| use arrow_schema::{SchemaRef, SortOptions}; | ||
| use datafusion_common::{plan_datafusion_err, Result}; | ||
| use datafusion_physical_expr_common::sort_expr::LexOrdering; | ||
| use datafusion_physical_expr_common::sort_expr::{ | ||
| LexOrdering, PhysicalSortRequirement, | ||
| }; | ||
|
|
||
| pub fn output_schema( | ||
| mapping: &ProjectionMapping, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,8 +159,13 @@ impl OrderingEquivalenceClass { | |
| /// Returns the concatenation of all the orderings. This enables merge | ||
| /// operations to preserve all equivalent orderings simultaneously. | ||
| pub fn output_ordering(&self) -> Option<LexOrdering> { | ||
| let output_ordering = self.orderings.iter().flatten().cloned().collect(); | ||
| let output_ordering = collapse_lex_ordering(output_ordering); | ||
| let output_ordering = self | ||
| .orderings | ||
| .iter() | ||
| .flatten() | ||
| .cloned() | ||
| .collect::<LexOrdering>() | ||
| .collapse(); | ||
| (!output_ordering.is_empty()).then_some(output_ordering) | ||
| } | ||
|
|
||
|
|
@@ -221,19 +226,6 @@ impl IntoIterator for OrderingEquivalenceClass { | |
| } | ||
| } | ||
|
|
||
| /// This function constructs a duplicate-free `LexOrdering` by filtering out | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Despite the function being https://docs.rs/datafusion/latest/datafusion/index.html?search=collapse_lex_ordering (when I marked it deprecated then clippy complained it was dead code) |
||
| /// duplicate entries that have same physical expression inside. For example, | ||
| /// `vec![a ASC, a DESC]` collapses to `vec![a ASC]`. | ||
| pub fn collapse_lex_ordering(input: LexOrdering) -> LexOrdering { | ||
| let mut output = LexOrdering::default(); | ||
| for item in input.iter() { | ||
| if !output.iter().any(|req| req.expr.eq(&item.expr)) { | ||
| output.push(item.clone()); | ||
| } | ||
| } | ||
| output | ||
| } | ||
|
|
||
| /// Trims `orderings[idx]` if some suffix of it overlaps with a prefix of | ||
| /// `orderings[pre_idx]`. Returns `true` if there is any overlap, `false` otherwise. | ||
| fn resolve_overlap(orderings: &mut [LexOrdering], idx: usize, pre_idx: usize) -> bool { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the original code this was
item.clone()-- though it is only cloning an Arc so the performance benefit is likely minimal