-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: skip hash shuffle for date_bin/date_trunc on Range([timestamp]) #24501
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
base: main
Are you sure you want to change the base?
Changes from all commits
35792e5
58dea62
6006096
b26846d
f3a64ff
693cd3e
c36301c
02029fb
0c118e0
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 |
|---|---|---|
|
|
@@ -1319,6 +1319,31 @@ impl EquivalenceProperties { | |
| .unwrap_or_else(|_| ExprProperties::new_unknown()) | ||
| } | ||
|
|
||
| /// Returns true when `expr` is a (possibly non-strict) monotonic function of | ||
| /// `range_key` plus literals, such as `date_bin(interval, timestamp)` or | ||
| /// `date_trunc(unit, timestamp)`. | ||
| /// | ||
| /// The identity `expr == range_key` returns false so callers can treat "emit | ||
| /// the key as-is" separately from "emit a function of the key". | ||
| pub(crate) fn is_monotonic_function_of( | ||
| &self, | ||
| expr: &Arc<dyn PhysicalExpr>, | ||
| range_key: &Arc<dyn PhysicalExpr>, | ||
|
Contributor
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. Rename these to source_expr and transformed_expr? Definately would not call this |
||
| ) -> bool { | ||
| if expr.eq(range_key) { | ||
| return false; | ||
| } | ||
| let dependencies = Dependencies::new(std::iter::once(PhysicalSortExpr::new( | ||
| Arc::clone(range_key), | ||
| Default::default(), | ||
| ))); | ||
| matches!( | ||
| get_expr_properties(expr, &dependencies, &self.schema) | ||
| .map(|properties| properties.sort_properties), | ||
| Ok(SortProperties::Ordered(_)) | ||
|
Contributor
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 doesn't capture the ordering, so a non monotonic function like Let's just assert that the options in |
||
| ) | ||
| } | ||
|
|
||
| /// Transforms this `EquivalenceProperties` by mapping columns in the | ||
| /// original schema to columns in the new schema by index. | ||
| pub fn with_new_schema(mut self, schema: SchemaRef) -> Result<Self> { | ||
|
|
||
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.
I'd rename this.
equivalence_properties.is_monotonic_function_ofsounds like the properies are a monotonic function.Dependenciesrelationship.Maybe rename to
check_monotonic_dependencyorcheck_monotonic_transform?