feat: skip hash shuffle for date_bin/date_trunc on Range([timestamp]) by NGA-TRAN · Pull Request #24501 · apache/datafusion

NGA-TRAN · 2026-08-19T19:55:06Z

Which issue does this PR close?

Closes Support functions date_bin and date_trunc in range partition satisfaction #23569
Depends on test: add range-partitioned sorted time-bin aggregation coverage #24500 (test coverage that pinned today's hash-shuffle plan)

Rationale for this change

A table that is range-partitioned on timestamp (for example by hour) and grouped by date_bin / date_trunc of that timestamp is already partition-disjoint when bins do not straddle split points. DataFusion still planned Partial → hash RepartitionExec → Final.

After this change, Range([timestamp]) subset-satisfies KeyPartitioned([key, f(timestamp)]) when f is monotonic (date_bin, date_trunc) and evaluating f at each split vs. its predecessor shows the bins are disjoint. Aggregation then runs as one streaming SinglePartitioned step with ordering_mode=Sorted.

Bins that do straddle the split (for example date_trunc('day') on hour-partitioned data) still hash-repartition.

#24500's expected plan changes from:

AggregateExec FinalPartitioned
  RepartitionExec Hash([key, date_bin(...)])
    AggregateExec Partial, ordering_mode=Sorted
      DataSourceExec Range([timestamp]), 2 file groups

to:

AggregateExec SinglePartitioned, ordering_mode=Sorted
  DataSourceExec Range([timestamp]), 2 file groups

What changes are included in this PR?

Project Range through monotonic grouping expressions when bins stay disjoint.
Treat that case as subset satisfaction so SanityCheckPlan accepts SinglePartitioned over Range([timestamp]).
Update range_sorted_time_bin_agg.slt for date_bin(60s) and date_trunc('hour') (no shuffle) and date_trunc('day') (shuffle remains).

Are these changes tested?

Unit tests in partitioning.rs for aligned date_bin / date_trunc('hour') vs unaligned split and date_trunc('day').
cargo test --test sqllogictests -- range_sorted_time_bin_agg.slt

Are there any user-facing changes?

Queries that group by date_bin / date_trunc of a range-partitioned timestamp may skip a hash shuffle when bins do not cross file-group boundaries. Results are unchanged.

Test plan

cargo test -p datafusion-physical-expr --lib -- partitioning
cargo test --test sqllogictests -- range_sorted_time_bin_agg.slt
CI

Co-author with Cursor

Pin today's Partial + hash RepartitionExec + Final plan for GROUP BY key, date_bin(timestamp) on a table that is already Range([timestamp]) and sorted on (key, timestamp), so a follow-up can remove the shuffle. Co-authored-by: Cursor <cursoragent@cursor.com>

codecov-commenter · 2026-08-19T20:23:45Z

Codecov Report

❌ Patch coverage is 95.57110% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.33%. Comparing base (8332cfa) to head (0c118e0).

Files with missing lines	Patch %	Lines
datafusion/physical-expr/src/partitioning.rs	95.43%	11 Missing and 7 partials ⚠️
...on/physical-expr/src/equivalence/properties/mod.rs	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24501      +/-   ##
==========================================
+ Coverage   81.32%   81.33%   +0.01%     
==========================================
  Files        1117     1117              
  Lines      396269   396682     +413     
  Branches   396269   396682     +413     
==========================================
+ Hits       322260   322642     +382     
- Misses      55186    55206      +20     
- Partials    18823    18834      +11

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Use generic column and table names so the coverage does not expose a metrics-specific schema. Co-authored-by: Cursor <cursoragent@cursor.com>

Accept RecordBatches and optional file sort order so the time-bin table reuses the same registration path. Co-authored-by: Cursor <cursoragent@cursor.com>

Treat Range([x]) as a subset of KeyPartitioned([..., f(x), ...]) when f is monotonic and bins do not straddle split points, so GROUP BY key, date_bin/date_trunc can stream without a hash RepartitionExec. Co-authored-by: Cursor <cursoragent@cursor.com>

Drop the local order-preserving walk and scalar predecessor so date_bin/date_trunc range projection uses EquivalenceProperties and interval_arithmetic instead of duplicating them. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

…che#24500 Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

NGA-TRAN · 2026-08-20T20:16:36Z

Tag @gene-bordegaray and @jayshrivastava for review

jayshrivastava

Looks good. Far better than my approach in #23536.

I left some comments re. completeness.

jayshrivastava · 2026-08-21T14:37:27Z

-#     -> RepartitionExec Hash([key, date_bin(...)])
-#     -> FinalPartitioned AggregateExec (ordering_mode=Sorted)
+# date_trunc('day') bins straddle the hour split, so that query still
+# hash-repartitions.


nit: can you use actual numbers as an example to show a date_bin/date_trunc which preserves partitioning and one that doesn't?

jayshrivastava · 2026-08-21T14:42:22Z

+ORDER BY key, time_bin;
+----
+k1 2024-01-01T00:00:00 33
+k2 2024-01-01T00:00:00 42


These tests look good 👍🏽

jayshrivastava · 2026-08-21T15:01:49Z

+        matches!(
+            get_expr_properties(expr, &dependencies, &self.schema)
+                .map(|properties| properties.sort_properties),
+            Ok(SortProperties::Ordered(_))


This doesn't capture the ordering, so a non monotonic function like f(x) = -x will return true here even though it flips the ordering from ASC to DESC. Even if -x works for range partitioning, this function may be used for some different use case, in which it will incorrectly return true.

Let's just assert that the options in SortProperties::Ordered(options) are the same as sort_expr.options and add a test for -x.

jayshrivastava · 2026-08-21T15:05:14Z

+        let mut sort_exprs = Vec::with_capacity(self.ordering.len());
+        for (key_idx, sort_expr) in self.ordering.iter().enumerate() {
+            if let Some(projected) =
+                input_eq_properties.project_expr(&sort_expr.expr, mapping)


Can you add a test where we're range partitioned on a and project select a, date_bin(a) from foo?

I think this case will preserve range partitioning on a. This behavior is a bit subtle. Your comment is good though "If a projection drops a range key but keeps a monotonic function of it".

Testing select a as b, date_bin(a) as bucket from foo would be good too. This should preserve the same range partitioning.

jayshrivastava · 2026-08-21T15:12:49Z

+    ///
+    /// 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(


I'd rename this.

Doing equivalence_properties.is_monotonic_function_of sounds like the properies are a monotonic function.

The two expressions aren't necessarily either functions. They are related via the Dependencies relationship.

Maybe rename to check_monotonic_dependency or check_monotonic_transform?

jayshrivastava · 2026-08-21T15:14:26Z

+    pub(crate) fn is_monotonic_function_of(
+        &self,
+        expr: &Arc<dyn PhysicalExpr>,
+        range_key: &Arc<dyn PhysicalExpr>,


Rename these to source_expr and transformed_expr? Definately would not call this range_key because this function has nothing to do with range partitioning.

jayshrivastava · 2026-08-21T15:17:45Z

+    mapping.iter().find_map(|(source, targets)| {
+        eq_properties
+            .is_monotonic_function_of(source, &sort_expr.expr)
+            .then(|| (Arc::clone(&targets.first().0), Arc::clone(source)))


I think it's okay to only look at the first target. Target just contains the alias IIUC. Ex.
select non_monotonic_function(a), date_bin(a) from foo has 2 sources (which contain the projection) with 1 target each, so this function works and should find the monotonic date_bin function. Can you add a test for this query which calls this function and ensure that it correctly finds the first monotonic function?

select non_monotonic_function(a), date_bin(a) as bin1, date_bin(a) as bin2 from foo would also be interesting.

Also note that the way the code is written, select date_bin(a, 45s) as bin1, date_bin(a, 60s) as bin2 from foo will only check the first date_bin.

What if the 45s straddles the partitions but 60s doesn't? Then the range partitioning will fall back to Unknown right? Maybe we should search for ALL the monotonic functions here and then in RangePartitioning::project(...), we check each one to see monotonic_fn_keeps_partitions_disjoint and use the first one that meets both requirements.

jayshrivastava · 2026-08-21T15:32:14Z

-                    )
+                    );
+                    if satisfaction == PartitioningSatisfaction::NotSatisfied
+                        && allow_subset


If partitioned on just timestamp and you do select date_bin(timestamp) from foo, then we should check range_monotonic_fn_satisfies_keys even if allow_subset is false.

jayshrivastava · 2026-08-21T15:37:07Z

+    required_exprs: &[Arc<dyn PhysicalExpr>],
+    eq_properties: &EquivalenceProperties,
+) -> bool {
+    if range.ordering().len() != 1 {


Do you plan to expand this in a follow up?

jayshrivastava · 2026-08-21T15:48:22Z

+    split_points: &[SplitPoint],
+    key_idx: usize,
+) -> bool {
+    split_points.iter().all(|split_point| {


This is pretty clean. My approach would have been to store the previous mapped split point in a variable and check monotonicity that way

gene-bordegaray

all my comments largely ditto @jayshrivastava I can take a review when these are addressed 👍

NGA-TRAN mentioned this pull request Aug 19, 2026

Support functions date_bin and date_trunc in range partition satisfaction #23569

Open

github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Aug 19, 2026

NGA-TRAN and others added 3 commits August 20, 2026 10:27

test: generalize range-sorted time-bin aggregation test names

58dea62

Use generic column and table names so the coverage does not expose a metrics-specific schema. Co-authored-by: Cursor <cursoragent@cursor.com>

test: share parquet listing-table helper for range tests

6006096

Accept RecordBatches and optional file sort order so the time-bin table reuses the same registration path. Co-authored-by: Cursor <cursoragent@cursor.com>

NGA-TRAN force-pushed the feat/issue-23569-range-date-bin-trunc branch from 05705f6 to b26846d Compare August 20, 2026 15:44

github-actions Bot added the logical-expr Logical plan and expressions label Aug 20, 2026

NGA-TRAN and others added 2 commits August 20, 2026 14:47

fix: wrap Range([x]) in backticks to satisfy rustdoc link checks

693cd3e

Co-authored-by: Cursor <cursoragent@cursor.com>

test: cover range date_bin/date_trunc edge cases and checked_predecessor

c36301c

Co-authored-by: Cursor <cursoragent@cursor.com>

NGA-TRAN marked this pull request as ready for review August 20, 2026 19:16

NGA-TRAN and others added 2 commits August 20, 2026 15:34

merge origin/main to pick up range-sorted time-bin agg tests from apa…

02029fb

…che#24500 Co-authored-by: Cursor <cursoragent@cursor.com>

test: fail closed when date_bin range splits are null or invalid

0c118e0

Co-authored-by: Cursor <cursoragent@cursor.com>

xavlee mentioned this pull request Aug 21, 2026

feat: stream aggregates for group-contiguous input #24497

Draft

jayshrivastava approved these changes Aug 21, 2026

View reviewed changes

jayshrivastava mentioned this pull request Aug 21, 2026

consider date_bin in range partition satisfaction #23536

Closed

gene-bordegaray reviewed Aug 22, 2026

View reviewed changes

Uh oh!

Conversation

NGA-TRAN commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Test plan

Uh oh!

codecov-commenter commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NGA-TRAN commented Aug 20, 2026

Uh oh!

jayshrivastava left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gene-bordegaray left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NGA-TRAN commented Aug 19, 2026 •

edited

Loading

codecov-commenter commented Aug 19, 2026 •

edited

Loading