andygrove commented on code in PR #6740:
URL: https://github.com/apache/datafusion-comet/pull/6740#discussion_r4207840688
##########
native/spark-expr/src/kernels/temporal.rs:
##########
@@ -714,16 +731,20 @@ pub(crate) fn timestamp_trunc_dyn(
/// Keep key masking for fallible values and for coarse units with many
distinct values, where
/// masking can avoid expensive calendar work on unused entries. Fine
arithmetic processes the
/// physical values regardless of validity, so infallible fine units never
need a key scan.
+/// MICROSECOND is always infallible; SECOND and MILLISECOND are only while
they wrap.
fn timestamp_trunc_dictionary_needs_mask(
values: &dyn Array,
keys_len: usize,
format: &str,
+ wrap_second_millisecond_overflow: bool,
) -> Result<bool, SparkError> {
let Some(values) =
values.as_any().downcast_ref::<TimestampMicrosecondArray>() else {
return Ok(true);
};
let granularity = normalize_timestamp_trunc_format(format)?;
- if matches!(granularity, "microsecond" | "millisecond" | "second") {
+ if granularity == "microsecond"
+ || (wrap_second_millisecond_overflow && matches!(granularity,
"millisecond" | "second"))
Review Comment:
Fixed in 9ade55e6ce. SECOND and MILLISECOND now run the values-only range
check before the non-UTC guard, so in check mode a dictionary keeps key masking
only when one of its values is near the lower bound, whatever the timezone.
With your shape (32 distinct values, Asia/Tokyo, check mode, the kernel at
opt-level 3 over debug dependencies), the call drops from 18.4 µs to 2.4 µs at
8,192 keys and from 132 µs to 2.4 µs at 65,536 keys, the same as UTC and the
wrapping mode. `test_timestamp_trunc_dictionary_fine_mask_decision` covers the
decision for no timezone, UTC and Asia/Tokyo in both modes.
--
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]
