Repository navigation
Push between on extension arrays down to storage - #10408
Draft
joseph-isaacs wants to merge 4 commits into
Draft
joseph-isaacs wants to merge 4 commits into
joseph-isaacs wants to merge 4 commits into
Conversation
Dates and timestamps had no between pushdown, so between decompressed the storage and fell back to two compares plus AND. Evaluate between on the storage array instead, as compare already does, so the storage encoding's between path runs. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MiogmvywXKrRVCtvnyzDpE
Merging this PR will degrade performance by 21.13%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | density_sweep_dense_runs[0.001] |
30.2 µs | 47.8 µs | -36.8% |
| ❌ | Simulation | filter_powerlaw_by_mostly_true[250000] |
104 µs | 150 µs | -30.69% |
| ❌ | Simulation | take[routing/same_chunk/primitive/nonnull/chunks=32/indices=1000] |
176 µs | 223.4 µs | -21.19% |
| ⚡ | WallTime | bitpack_blocked_compress_avx2 |
7.6 µs | 6.7 µs | +12.05% |
| Simulation | bench_compare_primitive[(10000, 2)] |
64.2 µs | < 1 ns | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ji/extension-between-pushdown (514a1de) with develop (4c894d2)
Footnotes
-
534 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Ordering storage values is only valid for an extension type whose logical order matches its storage order, which is a property of the specific type. The datetime types (Date, Time, Timestamp) have it. Other extension types, such as JSON text, UUIDs, WKB geometry or foreign types, may not, so the rule now declines anything that is not `AnyTemporal`. `AnyTemporal` matches the registered type behind the extension id, so a foreign type that only carries a datetime id is also declined. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MiogmvywXKrRVCtvnyzDpE
Decide whether between may be evaluated on the storage array by comparing the array's extension id against the datetime ids (vortex.date, vortex.time, vortex.timestamp) instead of downcasting with AnyTemporal. Signed-off-by: Joseph Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GbpTTiVEx2YjR1FjgfwRDp Signed-off-by: Joseph Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joseph Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GbpTTiVEx2YjR1FjgfwRDp Signed-off-by: Joseph Isaacs <joe.isaacs@live.co.uk>
joseph-isaacs
added
the
action/bench-sql
github-actions
Bot
removed
the
action/bench-sql
Contributor
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Contributor
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.016x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.005x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.962x ➖, 2↑ 1↓)
duckdb / parquet / ns (1.014x ➖, 0↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.999x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.994x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.991x ➖, 4↑ 2↓)
duckdb / parquet / ns (0.994x ➖, 2↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.002x ➖, 0↑ 2↓)
duckdb / parquet / ns (0.988x ➖, 5↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.995x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.003x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.122x ➖, 0↑ 3↓)
datafusion / parquet / ns (0.995x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.065x ➖, 1↑ 2↓)
duckdb / parquet / ns (1.030x ➖, 0↑ 0↓)
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
betweenon an extension array (dates, timestamps) had no pushdown, so it decompressed the storage and fell back to two compares plusAND. Compare already evaluates extension arrays through their storage; this adds the same forbetweenas aBetweenReducerule onExtension, so the storage encoding's ownbetweenpath runs instead.The rule only fires for the datetime extension types (
Date,Time,Timestamp, matched withAnyTemporal), because ordering storage values is only valid for a type whose logical order matches its storage order, and that is a property of the specific type. Other extension types, such as JSON text, UUIDs, WKB geometry or foreign types, are declined.AnyTemporalmatches the registered type behind the extension id, so a foreign type that merely carries a datetime id is declined too.It also only fires when both bounds have the same extension dtype as the array (ignoring nullability), so storage values with different meanings, e.g. a microsecond timestamp against a millisecond bound, are never compared. Constant bounds become constant storage scalars; extension-array bounds use their storage array.
TPC-H's date filters are the main beneficiary:
l_shipdate,o_orderdateandl_receiptdatecompress toExtension(Date)overFoRoverBitPacked, andfind_betweenturns their>= … AND < …intobetween.Benchmarks
Kernel, one 64Ki-row
l_shipdate-style chunk (Extension → FoR → BitPacked), median: 102.5 µs → 16.9 µs.TPC-H SF1, DataFusion, Vortex files. One binary with the rule toggled at runtime, six alternating rounds of 10 iterations each, median ms:
Measured on a noisy 2-core x86 VM, not the benchmark runners; only Q6's before/after ranges are fully separated. ClickBench
EventDateis unaffected becauseDateTimePartsalready pushesbetweeninto its days child.Checks
betweenand the equivalent two compares agree on date and timestamp columns, before and after this change.between.rs(constant bounds × all four strictness combinations, nullable storage, extension-array bounds, timestamps, and a non-datetime extension type being declined) were not run.cargo clippyandcargo fmtwere not run.🤖 Generated with Claude Code
https://claude.ai/code/session_01MiogmvywXKrRVCtvnyzDpE
Generated by Claude Code