-
Notifications
You must be signed in to change notification settings - Fork 2.3k
ScalarUdfImpl::strictly_order_preserving: Allow expression to report whether they keep the same ordering of the input
#23807
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
15a6de3
466f279
d60f288
9fd3665
7305547
1f5267a
b5bb7ac
90d6a51
04a6e56
4d60d84
ad7d27b
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -380,6 +380,11 @@ impl ScalarUDF { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.preserves_lex_ordering(inputs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// See [`ScalarUDFImpl::strictly_order_preserving`] for more details. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn strictly_order_preserving(&self, inputs: &[ExprProperties]) -> Result<bool> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.strictly_order_preserving(inputs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// See [`ScalarUDFImpl::coerce_types`] for more details. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.coerce_types(arg_types) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -979,10 +984,20 @@ pub trait ScalarUDFImpl: Debug + DynEq + DynHash + Send + Sync + Any { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Returns true if the function preserves lexicographical ordering based on | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// the input ordering. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. I think it would be ok to keep these descriptions short and refer readers to the very nice SortProperties description Eg. something like
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// See [`ExprProperties::preserves_lex_ordering`] for more details | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn preserves_lex_ordering(&self, _inputs: &[ExprProperties]) -> Result<bool> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Returns true if the function is strictly order-preserving with respect | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. How is this related to the output_ordering method? I am wondering if we can extend that rather tan introduce a new method
Member
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.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// to its `Ordered` inputs, i.e. `a.cmp(b) == f(a).cmp(f(b))`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// See [`ExprProperties::strictly_order_preserving`] for more details | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn strictly_order_preserving(&self, _inputs: &[ExprProperties]) -> Result<bool> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+995
to
+999
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. Similarly I think here it would be ok to point back to ExprProperties to make sure the text stays consistent
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Coerce arguments of a function call to types that the function can evaluate. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// This function is only called if [`ScalarUDFImpl::signature`] returns | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1194,6 +1209,10 @@ impl ScalarUDFImpl for AliasedScalarUDFImpl { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.preserves_lex_ordering(inputs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn strictly_order_preserving(&self, inputs: &[ExprProperties]) -> Result<bool> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.strictly_order_preserving(inputs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.inner.coerce_types(arg_types) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ use arrow::datatypes::TimeUnit::Second; | |
| use arrow::datatypes::{DataType, Field, FieldRef}; | ||
| use datafusion_common::{Result, ScalarValue, exec_err, internal_err}; | ||
| use datafusion_expr::TypeSignature::Exact; | ||
| use datafusion_expr::sort_properties::{ExprProperties, SortProperties}; | ||
| use datafusion_expr::{ | ||
| ColumnarValue, Documentation, ReturnFieldArgs, ScalarFunctionArgs, ScalarUDFImpl, | ||
| Signature, Volatility, | ||
|
|
@@ -147,6 +148,24 @@ impl ScalarUDFImpl for FromUnixtimeFunc { | |
| } | ||
| } | ||
|
|
||
| fn output_ordering(&self, inputs: &[ExprProperties]) -> Result<SortProperties> { | ||
| // The optional timezone argument must be a constant string and only | ||
| // affects the display metadata, not the stored epoch value, so the | ||
| // output ordering follows the first argument. | ||
| Ok(inputs[0].sort_properties) | ||
| } | ||
|
|
||
| fn preserves_lex_ordering(&self, _inputs: &[ExprProperties]) -> Result<bool> { | ||
| Ok(true) | ||
| } | ||
|
|
||
| fn strictly_order_preserving(&self, _inputs: &[ExprProperties]) -> Result<bool> { | ||
| // `from_unixtime` stores the input's exact `Int64` value as a | ||
|
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. 👍 |
||
| // `Timestamp(Second)`: the mapping is one-to-one, order-preserving, | ||
| // and maps nulls to nulls. | ||
| Ok(true) | ||
| } | ||
|
|
||
| fn documentation(&self) -> Option<&Documentation> { | ||
| self.doc() | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.