Add extension constant pushdown rule and fix InnerProduct rule#7507
Add extension constant pushdown rule and fix InnerProduct rule#7507connortsui20 merged 1 commit intodevelopfrom
InnerProduct rule#7507Conversation
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Merging this PR will degrade performance by 16.95%
Performance Changes
Comparing Footnotes
|
Polar Signals Profiling ResultsLatest Run
Previous Runs (1)
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals ProfilingVortex (geomean): 0.944x ➖ datafusion / vortex-file-compressed (0.944x ➖, 3↑ 0↓)
|
File Sizes: PolarSignals ProfilingNo file size changes detected. |
Benchmarks: FineWeb NVMeVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (1.072x ➖, 0↑ 3↓)
datafusion / vortex-compact (1.074x ➖, 0↑ 2↓)
datafusion / parquet (1.046x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (1.093x ➖, 0↑ 4↓)
duckdb / vortex-compact (1.184x ❌, 0↑ 7↓)
duckdb / parquet (1.093x ➖, 0↑ 2↓)
Full attributed analysis
|
File Sizes: FineWeb NVMeNo file size changes detected. |
Benchmarks: TPC-H SF=1 on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (0.991x ➖, 0↑ 0↓)
datafusion / vortex-compact (1.004x ➖, 0↑ 0↓)
datafusion / parquet (1.004x ➖, 0↑ 0↓)
datafusion / arrow (1.059x ➖, 0↑ 2↓)
duckdb / vortex-file-compressed (1.009x ➖, 0↑ 0↓)
duckdb / vortex-compact (1.009x ➖, 0↑ 0↓)
duckdb / parquet (0.977x ➖, 2↑ 0↓)
duckdb / duckdb (0.977x ➖, 0↑ 0↓)
Full attributed analysis
|
File Sizes: TPC-H SF=1 on NVMENo file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVMEVerdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (0.876x ✅, 56↑ 0↓)
datafusion / vortex-compact (0.793x ✅, 93↑ 0↓)
datafusion / parquet (0.941x ➖, 32↑ 14↓)
duckdb / vortex-file-compressed (0.818x ✅, 91↑ 1↓)
duckdb / vortex-compact (0.850x ✅, 84↑ 1↓)
duckdb / parquet (0.862x ✅, 80↑ 0↓)
duckdb / duckdb (0.820x ✅, 88↑ 0↓)
Full attributed analysis
|
File Sizes: TPC-DS SF=1 on NVMENo file size changes detected. |
Benchmarks: FineWeb S3Verdict: No clear signal (low confidence) datafusion / vortex-file-compressed (0.846x ➖, 1↑ 0↓)
datafusion / vortex-compact (0.913x ➖, 1↑ 2↓)
datafusion / parquet (0.927x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (0.936x ➖, 0↑ 0↓)
duckdb / vortex-compact (0.902x ➖, 0↑ 0↓)
duckdb / parquet (0.916x ➖, 0↑ 0↓)
Full attributed analysis
|
Benchmarks: Random AccessVortex (geomean): 0.847x ✅ unknown / unknown (0.931x ➖, 11↑ 0↓)
|
Benchmarks: TPC-H SF=10 on NVMEVerdict: No clear signal (low confidence) datafusion / vortex-file-compressed (1.040x ➖, 0↑ 2↓)
datafusion / vortex-compact (1.065x ➖, 0↑ 9↓)
datafusion / parquet (1.013x ➖, 0↑ 0↓)
datafusion / arrow (1.006x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed (1.084x ➖, 0↑ 8↓)
duckdb / vortex-compact (1.100x ➖, 0↑ 12↓)
duckdb / parquet (1.047x ➖, 0↑ 1↓)
duckdb / duckdb (1.050x ➖, 0↑ 2↓)
Full attributed analysis
|
File Sizes: TPC-H SF=10 on NVMENo file size changes detected. |
Benchmarks: Statistical and Population GeneticsVerdict: No clear signal (low confidence) duckdb / vortex-file-compressed (1.116x ❌, 0↑ 5↓)
duckdb / vortex-compact (1.047x ➖, 0↑ 2↓)
duckdb / parquet (1.097x ➖, 0↑ 8↓)
Full attributed analysis
|
File Sizes: Statistical and Population GeneticsNo file size changes detected. |
Benchmarks: TPC-H SF=1 on S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (0.823x ➖, 4↑ 0↓)
datafusion / vortex-compact (0.845x ➖, 3↑ 1↓)
datafusion / parquet (1.002x ➖, 1↑ 2↓)
duckdb / vortex-file-compressed (0.959x ➖, 0↑ 0↓)
duckdb / vortex-compact (0.945x ➖, 0↑ 0↓)
duckdb / parquet (0.945x ➖, 0↑ 0↓)
Full attributed analysis
|
🚨🚨🚨❌❌❌ SQL BENCHMARK FAILED ❌❌❌🚨🚨🚨Benchmark |
Benchmarks: CompressionVortex (geomean): 1.003x ➖ unknown / unknown (0.999x ➖, 0↑ 1↓)
|
Benchmarks: TPC-H SF=10 on S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (0.918x ➖, 1↑ 1↓)
datafusion / vortex-compact (0.795x ➖, 4↑ 0↓)
datafusion / parquet (0.784x ➖, 6↑ 0↓)
duckdb / vortex-file-compressed (0.969x ➖, 0↑ 0↓)
duckdb / vortex-compact (1.028x ➖, 0↑ 0↓)
duckdb / parquet (0.956x ➖, 0↑ 0↓)
Full attributed analysis
|
## Summary Fixes the regression caused by #7507 The reduction rule would go from `Extension(Constant(..))` into `Constant(Extension(..))`, but the `execute` would just revert that, creating a cycle! ## Testing N/A Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Summary
Most of our optimizer rules interact with
ConstantArray, as often there is an optimized implementation when a child is known to be constant.We can have both of these arrays that mean the same thing, but the second is not easily matched on:
This change adds a rule to convert the second representation into the first. It additionally fixes a bug in the
InnerProductreduction (right now it is inexecutebut it should really be a reduction rule, that will come in the future) where it checks for the second case instead of the first.Testing
Basic tests.