Repository navigation
fix: date_part sub-second parts and result nullability, reusing arrow kernels - #26686
mbutrovich wants to merge 5 commits into
Conversation
…r times, intervals, durations and timezones
…ECOND) for Duration and Interval
|
run benchmark datetime_expressions env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: c920a5b92df0172b23026c3773dc7cd0fbdb8ce6 |
|
run benchmark datetime_expressions env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: c920a5b92df0172b23026c3773dc7cd0fbdb8ce6
resources:
arch: amd64 |
|
run benchmark datetime_expressions env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: c920a5b92df0172b23026c3773dc7cd0fbdb8ce6 |
|
run benchmark datetime_expressions env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: c920a5b92df0172b23026c3773dc7cd0fbdb8ce6
resources:
arch: amd64 |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"
resources:
arch: "amd64"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"
resources:
arch: "amd64"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"
resources:
arch: "amd64"CPU Details (lscpu)Details
Resource Usagedatetime_expressions — base (merge-base)
datetime_expressions — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"CPU Details (lscpu)Details
Resource Usagedatetime_expressions — base (merge-base)
datetime_expressions — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"CPU Details (lscpu)Details
Resource Usagedatetime_expressions — base (merge-base)
datetime_expressions — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing fix-extract-nanosecond-duration-interval (adab085) to c920a5b diff Run configurationrun benchmark datetime_expressions
env:
BENCH_FILTER: "date_part_(second|millisecond|microsecond|nanosecond)_"
RUSTFLAGS: "-C llvm-args=-align-all-functions=6 -C llvm-args=-align-all-nofallthru-blocks=6"
baseline:
ref: "c920a5b92df0172b23026c3773dc7cd0fbdb8ce6"
resources:
arch: "amd64"CPU Details (lscpu)Details
Resource Usagedatetime_expressions — base (merge-base)
datetime_expressions — branch
File an issue against this benchmark runner |
Which issue does this PR close?
EXTRACT(NANOSECOND)onDurationandIntervalcounts whole seconds twice #26678.date_partreturning wrong results due to overflows #14738.date_partdeclares a non-nullable result for non-null input, but can return NULL #26685.Rationale for this change
These fixes depend on each other. #26678 and #14738 come from the same NULL-as-zero branch. Fixing #14738 makes
millisecondandmicrosecondof a long duration return NULL instead of a wrapped value. Without the #26685 fix, that NULL fails withdeclared as non-nullableon a non-nullable input, so fixing #14738 alone would replace a wrong result with an error.What changes are included in this PR?
millisecond,microsecondandnanosecondshare one function that reuses arrow'sdate_partandcastwhere they compute the right value.date_partand the Sparkdate_partare always nullable.epochkeeps the input's nullability.The benchmark bot measured
DurationandIntervalat 3.6x to 41.6x faster, times at 1.5x to 2.2x, and timestamps within noise.Numbers
Speedups over c920a5b (the bench commit) on 1000 rows, with the bot's thin LTO. "Nulls" means 10% of rows are NULL. The aligned runs add
-align-all-functions=6 -align-all-nofallthru-blocks=6.nanosecond,Duration(ns), large valuesnanosecond,Duration(ns), large values, nullsnanosecond,Duration(ns), under 2.1 snanosecond,Interval(DayTime)nanosecond,Interval(MonthDayNano)millisecond,Interval(DayTime)microsecond,Duration(ns), under 2.1 smicrosecond,Time64(ns)millisecond,Timestamp(ns)nanosecond,Timestamp(ns)(unchanged)second,Timestamp(ns)(unchanged)In the default Xeon run the unchanged
secondcases are 0.98x to 1.08x, so gains in that range on timestamps are noise.What is the testing strategy for this PR?
datafusion/sqllogictest/test_files/datetime/date_part.slt.datafusion/functions/src/datetime/date_part.rs.datafusion/spark/src/function/datetime/date_part.rs.Are there any user-facing changes?
EXTRACT(MILLISECOND/MICROSECOND/NANOSECOND)onDurationandIntervalreturn correct results. ADurationwhose total in the requested unit overflows the return type returns NULL, as the otherDurationparts already do, instead of a wrapped value or a panic. The integer results ofdate_partand the Sparkdate_partare nullable even for non-null input. There are no public API changes.AI Disclosure
I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage.