Skip to content

Support SpatialBench in DataFusion benchmark - #9615

Open
palaska wants to merge 1 commit into
developfrom
bp/datafusion-spatialbench
Open

Support SpatialBench in DataFusion benchmark#9615
palaska wants to merge 1 commit into
developfrom
bp/datafusion-spatialbench

Conversation

@palaska

@palaska palaska commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Adds automatic engine-specific SQL discovery for benchmark queries.

Adds a DataFusion SQL dialect for SpatialBench and registers GeoDataFusion functions. Adds an SF=0.6 pr-spatial preset. The action/bench-spatial label runs the preset. SpatialBench does not run in default presets.

The SQL corpus keeps all twelve queries. The performance preset excludes Q5, Q7, and Q9 because GeoDataFusion lacks their required functions.

Validation:

  • cargo test -p vortex-bench discovers_engine_query_corpora
  • cargo clippy -p vortex-bench --all-targets --all-features -- -D warnings
  • 24 focused benchmark orchestrator tests
  • Ruff format and lint checks
  • Workflow YAML lint

Prepared with Codex.

@codspeed-hq

codspeed-hq Bot commented Aug 25, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
❌ 1 regressed benchmark
✅ 2107 untouched benchmarks
⏩ 106 skipped benchmarks1
🗄️ 4 archived benchmarks run2

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime arrow_checked_add_u32_avx2[16384] 17.7 µs 21.3 µs -16.96%
WallTime arrow_checked_add_u32_neon[16384] 20.4 µs 13.5 µs +50.71%
WallTime arrow_checked_add_u32_avx512[16384] 21.3 µs 17.6 µs +20.72%
Simulation take[duplicates/repeated/primitive/nonnull/chunks=16/indices=1000] 245 µs 207 µs +18.39%
WallTime mul_u32_nonnull_avx512 6.3 µs 5.6 µs +11.03%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing bp/datafusion-spatialbench (a55abdd) with develop (ee1ac25)

Open in CodSpeed

Footnotes

  1. 106 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.

  2. 4 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them.

@palaska
palaska force-pushed the bp/datafusion-spatialbench branch from ffe767d to e1a9cef Compare August 25, 2026 14:51
@palaska palaska added changelog/feature A new feature ext/datafusion Relates to the DataFusion integration codex labels Aug 25, 2026
Comment thread benchmarks/datafusion-bench/src/main.rs Outdated
Comment on lines +185 to +187
if benchmark.dataset_name() == "spatialbench" {
geodatafusion::register(&session);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have a setup func

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this into datafusion_bench::setup_session

Comment thread vortex-bench/src/benchmark.rs Outdated
fn queries(&self) -> anyhow::Result<Vec<(usize, String)>>;

/// Get all available queries for one engine.
fn queries_for_engine(&self, _engine: Engine) -> anyhow::Result<Vec<(usize, String)>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would I filter like this?


fn queries_for_engine(&self, engine: Engine) -> anyhow::Result<Vec<(usize, String)>> {
if engine != Engine::DataFusion {
return self.queries();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this works for duckdb?

Comment on lines +88 to +94
Ok(include_str!("../../sql/spatialbench-datafusion.sql")
.split_terminator(';')
.map(str::trim)
.filter(|statement| !statement.is_empty())
.enumerate()
.map(|(index, statement)| (index + 1, statement.to_owned()))
.collect())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we do this?

Comment thread Cargo.toml
geo-types = "0.7.19"
geoarrow = "0.8.0"
geoarrow-cast = "0.8.0"
geodatafusion = "0.5.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this take ages to compile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was fine on my laptop. it adds some geo kernels but reuses datafusion/arrow versions

@joseph-isaacs

Copy link
Copy Markdown
Contributor

How come we never actually run it

@palaska
palaska force-pushed the bp/datafusion-spatialbench branch from e1a9cef to 4c1d7fa Compare August 25, 2026 15:21
@palaska

palaska commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

How come we never actually run it

added spatialbench to the matrix. kicked this manual job, lets see how it goes https://github.com/vortex-data/vortex/actions/runs/32867730381

@palaska
palaska force-pushed the bp/datafusion-spatialbench branch 5 times, most recently from 9f90ba6 to d1f40d1 Compare August 26, 2026 10:16
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
@palaska
palaska force-pushed the bp/datafusion-spatialbench branch from d1f40d1 to a55abdd Compare August 26, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature codex ext/datafusion Relates to the DataFusion integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants