jayzhan211 commented on code in PR #26111:
URL: https://github.com/apache/datafusion/pull/26111#discussion_r4295062372
##########
datafusion/functions/src/datetime/to_timestamp.rs:
##########
@@ -581,6 +582,16 @@ impl ScalarUDFImpl for ToTimestampSecondsFunc {
}
}
+ fn output_ordering(&self, input: &[ExprProperties]) ->
Result<SortProperties> {
Review Comment:
The integer path is a one-to-one cast (same as `from_unixtime`, which sets
both flags), but without `strictly_order_preserving` a multi-key sort still
plans a `SortExec`:
```sql
EXPLAIN SELECT to_timestamp_micros(micros) AS t, id
FROM ordered_timestamps ORDER BY t ASC NULLS LAST, id;
-- SortExec: expr=[t@0 ASC NULLS LAST, id@1 ASC NULLS LAST]
```
The precision-loss reason only applies to `to_unixtime`. Adding both flags
for the four functions removes the sort (checked locally, results unchanged).
Fine as a follow-up.
```rs
/// The single-argument integer form casts the value to `Timestamp(unit, tz)`
/// unchanged: one-to-one, order-preserving, and nulls map to nulls.
fn is_integer_input(input: &[ExprProperties]) -> bool {
matches!(input, [value] if value.range.data_type().is_integer())
}
```
```rs
fn output_ordering(&self, input: &[ExprProperties]) ->
Result<SortProperties> {
Ok(if is_integer_input(input) {
input[0].sort_properties
} else {
SortProperties::Unordered
})
}
fn preserves_lex_ordering(&self, input: &[ExprProperties]) ->
Result<bool> {
Ok(is_integer_input(input))
}
fn strictly_order_preserving(&self, input: &[ExprProperties]) ->
Result<bool> {
Ok(is_integer_input(input))
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
