apache / apache/datafusion-comet

Consolidate date/timestamp truncation and xxhash64 with upstream DataFusion implementations

Open
#5,103 2 comments 0 reactions 1 assignee Claimed by @sam-1112 View on GitHub
area:expressions enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

### What is the problem the feature request solves?

Beyond arrow kernels, two areas duplicate functionality now available upstream in DataFusion / datafusion-spark (54.1):

1. **Date/timestamp truncation**: `native/spark-expr/src/kernels/temporal.rs` implements `trunc(date, fmt)` and `date_trunc(fmt, ts)` by hand. Arrow has no truncation kernel, but `datafusion_functions::datetime::date_trunc` is timezone-aware (chrono-tz), supports all the granularities Spark needs with Monday-based weeks, and preserves the input tz annotation. Upstream datafusion-spark's `trunc` already composes it with the same alias map (yy/yyyy to year, mm/mon to month) via a cast sandwich.
2. **xxhash64**: datafusion-spark 54.1 ships a Spark-compatible `SparkXxhash64` (twox-hash based, with its own type-dispatch machinery). Comet's `hash_funcs/xxhash64.rs` could potentially be rewired to it. Note datafusion-spark has no murmur3, so Comet's `murmur3.rs` stays.

### Describe the potential solution

- Truncation: wire `date_trunc` at the physical level for the scalar-format paths (datafusion-spark's `trunc`/`date_diff` are simplify-only UDFs whose `invoke_with_args` returns an internal error, so they cannot be called from Comet's physical-plan pipeline directly). Keep custom code for: per-row-format variants (DataFusion requires scalar granularity), dictionary inputs, and the Spark session-timezone pre-shift.
- Requires differential DST-boundary testing first: Comet resolves ambiguous local times to the earliest occurrence and falls back to the original instant on gaps (`test_timestamp_trunc_dst_boundary`); DataFusion's fine-versus-coarse granularity handling needs case-by-case verification.
- xxhash64: verify type coverage before rewiring (Comet's macro additionally handles the small/large Decimal128 split, Map, FixedSizeList, and Struct fast paths).

### Additional context

Related in spirit: `string_funcs/contains.rs` and `array_funcs/array_slice.rs` each document an upstream DataFusion issue that, once fixed, lets the Comet file be dropped entirely.

Found during an audit of native code that replicates existing arrow-rs kernels.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.