geoffreyclaude commented on code in PR #26370:
URL: https://github.com/apache/datafusion/pull/26370#discussion_r4315169972


##########
datafusion/core/benches/aggregate_query_sql.rs:
##########
@@ -330,5 +334,185 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 }
 
-criterion_group!(benches, criterion_benchmark);
+/// Measure the multi-column fully ordered grouping path, with data generation
+/// and sorting outside timing. Both keys vary, and the nullable second key
+/// changes type while the shape of the input stays the same.
+fn fully_ordered(c: &mut Criterion) {
+    use DataType::*;
+    use IntervalUnit::{DayTime, MonthDayNano};
+    use TimeUnit::{Microsecond, Millisecond, Nanosecond};
+
+    let cases = [
+        ("int64", "signed_key", Int64, 31),
+        (
+            "utf8_view",
+            "arrow_cast(lpad(CAST(key_value AS VARCHAR), 4, '0'), 'Utf8View')",
+            Utf8View,
+            31,
+        ),
+        (
+            "timestamp_ns",
+            "arrow_cast(signed_key, 'Timestamp(Nanosecond)')",
+            Timestamp(Nanosecond, None),
+            31,
+        ),
+        (
+            "timestamp_us_utc",
+            "arrow_cast(signed_key, 'Timestamp(Microsecond, Some(\"UTC\"))')",
+            Timestamp(Microsecond, Some("UTC".into())),
+            31,
+        ),
+        (
+            "date32",
+            "arrow_cast(CAST(signed_key AS INTEGER), 'Date32')",
+            Date32,
+            31,
+        ),
+        (
+            "time32_ms",
+            "arrow_cast(CAST(key_value AS INTEGER), 'Time32(Millisecond)')",
+            Time32(Millisecond),
+            31,
+        ),
+        (
+            "time64_ns",
+            "arrow_cast(key_value, 'Time64(Nanosecond)')",
+            Time64(Nanosecond),
+            31,
+        ),
+        (
+            "duration_ns",
+            "arrow_cast(signed_key, 'Duration(Nanosecond)')",
+            Duration(Nanosecond),
+            31,
+        ),
+        (
+            "interval_day_time",
+            "arrow_cast(concat(CAST(signed_key AS VARCHAR), ' days'), 
'Interval(DayTime)')",
+            Interval(DayTime),
+            31,
+        ),
+        (
+            "interval_month_day_nano",
+            "arrow_cast(arrow_cast(signed_key, 'Duration(Nanosecond)'), 
'Interval(MonthDayNano)')",
+            Interval(MonthDayNano),
+            31,
+        ),
+        (
+            "decimal128",
+            "arrow_cast(concat('123456789012345678901', lpad(CAST(key_value AS 
VARCHAR), 4, '0'), '.1250'), 'Decimal128(30, 4)')",
+            Decimal128(30, 4),
+            31,
+        ),
+        (
+            "decimal256",
+            
"arrow_cast(concat('123456789012345678901234567890123456789012345678901', 
lpad(CAST(key_value AS VARCHAR), 4, '0'), '.1250'), 'Decimal256(60, 4)')",
+            Decimal256(60, 4),
+            31,
+        ),
+        (
+            "fixed_binary16",
+            "arrow_cast(decode(lpad(to_hex(key_value), 32, '0'), 'hex'), 
'FixedSizeBinary(16)')",
+            FixedSizeBinary(16),
+            31,
+        ),
+        (
+            "fixed_binary7",
+            "arrow_cast(decode(lpad(to_hex(key_value), 14, '0'), 'hex'), 
'FixedSizeBinary(7)')",
+            FixedSizeBinary(7),
+            31,
+        ),
+        (
+            "timestamp_ns_singleton",
+            "arrow_cast(signed_key, 'Timestamp(Nanosecond)')",
+            Timestamp(Nanosecond, None),
+            1,
+        ),
+        (
+            "timestamp_ns_long_runs",
+            "arrow_cast(signed_key, 'Timestamp(Nanosecond)')",
+            Timestamp(Nanosecond, None),
+            512,
+        ),
+    ];
+
+    let rt = Runtime::new().unwrap();
+    let mut group = c.benchmark_group("fully_ordered");
+    for (name, key_expr, key_type, run_length) in cases {
+        let ctx = rt
+            .block_on(fully_ordered_context(key_expr, &key_type, run_length))
+            .unwrap();
+        group.bench_function(name, |b| {
+            b.iter(|| query(ctx.clone(), &rt, FULLY_ORDERED_QUERY))
+        });
+    }
+    group.finish();
+}
+
+const FULLY_ORDERED_QUERY: &str = "SELECT k1, k2 FROM ordered_input GROUP BY 
k1, k2";

Review Comment:
   Added `fully_ordered/timestamp_ns_sum` with a varying value column. The 
[latest bot 
results](https://github.com/apache/datafusion/pull/26370#issuecomment-6266981812)
 show approximately 4× speedup, from 43.9 ms to 11.0 ms.



-- 
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]

Reply via email to