Fix DataFusion expression conversion and pushdown semantics - #9785
Fix DataFusion expression conversion and pushdown semantics#9785AdamGS wants to merge 10 commits into
Conversation
01bc390 to
47d4415
Compare
Merging this PR will degrade performance by 5.69%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | decompress[u64, (4000, 1024)] |
70.8 µs | 86.2 µs | -17.81% |
| ❌ | WallTime | mul_i32_nonnull_avx512 |
7.1 µs | 7.9 µs | -10.04% |
| ⚡ | Simulation | allocate_drop_arrow[0] |
456.9 ns | 402.7 ns | +13.45% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing adamg/df-fix-bugs-conversion (26fd045) with develop (bffdca1)
Footnotes
-
218 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. ↩
47d4415 to
4d7f747
Compare
Polar Signals Profiling ResultsLatest Run
Previous Runs (30)
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.008x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.005x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.004x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.028x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.062x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.030x ➖, 2↑ 2↓)
duckdb / parquet / ns (1.007x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: Clickbench Sorted on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.010x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.013x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.020x ➖, 1↑ 1↓)
duckdb / parquet / ns (0.998x ➖, 0↑ 1↓)
File Size Changes (100 files changed, -0.0% overall, 49↑ 51↓)
Totals:
|
Benchmarks: FineWeb S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.954x ➖, 2↑ 1↓)
datafusion / parquet / ns (1.016x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.974x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.051x ➖, 0↑ 0↓)
|
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.986x ➖, 4↑ 0↓)
duckdb / parquet / ns (1.000x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.003x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.997x ➖, 4↑ 1↓)
duckdb / parquet / ns (1.002x ➖, 2↑ 2↓)
No file size changes detected. |
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.986x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.997x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.005x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.990x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Statistical and Population Genetics 📖Commits: PR How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.990x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.075x ➖, 1↑ 3↓)
datafusion / parquet / ns (1.096x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.011x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.069x ➖, 0↑ 0↓)
|
5a76dc2 to
2dd2e1a
Compare
| | DataType::Decimal128(precision, scale) | ||
| | DataType::Decimal256(precision, scale) => { | ||
| DType::Decimal(DecimalDType::new(*precision, *scale), nullability) | ||
| DType::Decimal(DecimalDType::try_new(*precision, *scale)?, nullability) |
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
55e1679 to
d37c4e1
Compare
|
I'm still working on some of the tests and there are follow ups, but this should be ready for review. |
|
super nit but i'm pretty sure |
|
@a10y I'll rename, might as well. |
|
If we're near a release, I think it might be worth waiting on this PR, I have a bunch of follow ups and related changes |
Summary
Fix native pushdown in
vortex-datafusionto preserve DataFusion semantics and fall back for unsupported expressions. Replace separate eligibility and conversion checks with schema-awaretry_convert, preserving SQL null semantics forIN/NOT INand rejecting pushdown where casts, short-circuit evaluation, or nullable struct access could differ. Partially supported projections now run entirely in DataFusion over deduplicated raw columns.Re-split filters after each file’s schema adaptation: Vortex evaluates supported conjuncts, and DataFusion evaluates residuals before projection and limit. Disable scan limits whenever filters exist, include file and predicate context in errors, and fix
v2pushdown after projection swapping.Add conversion and residual-filter regression tests, plus 16 SQL cases compared against an Arrow
MemTablewith pushdown enabled and disabled.Rename
ConvertortoConverterLLMS were used as part of this work