diff --git a/.github/workflows/pr-bench-dispatch.yml b/.github/workflows/pr-bench-dispatch.yml index 3cb60c619bf..47d28e98718 100644 --- a/.github/workflows/pr-bench-dispatch.yml +++ b/.github/workflows/pr-bench-dispatch.yml @@ -151,3 +151,21 @@ jobs: secrets: inherit with: matrix_preset: "pr-compact" + + remove-spatial-label: + runs-on: ubuntu-latest + timeout-minutes: 10 + if: github.event.label.name == 'action/bench-spatial' + steps: + - uses: actions-ecosystem/action-remove-labels@2ce5d41b4b6aa8503e285553f75ed56e0a40bae0 # v1 + if: github.event.pull_request.head.repo.full_name == 'vortex-data/vortex' + with: + labels: action/bench-spatial + fail_on_error: true + + spatial-bench: + needs: remove-spatial-label + uses: ./.github/workflows/pr-bench-sql.yml + secrets: inherit + with: + matrix_preset: "pr-spatial" diff --git a/.github/workflows/pr-bench-sql.yml b/.github/workflows/pr-bench-sql.yml index 40789d631a8..865c0b56551 100644 --- a/.github/workflows/pr-bench-sql.yml +++ b/.github/workflows/pr-bench-sql.yml @@ -29,6 +29,7 @@ on: - "pr-compact" - "pr-all" - "pr-full" + - "pr-spatial" permissions: contents: read diff --git a/.github/workflows/sql-bench-matrix.yml b/.github/workflows/sql-bench-matrix.yml index cae3630d6b8..621357f0cfc 100644 --- a/.github/workflows/sql-bench-matrix.yml +++ b/.github/workflows/sql-bench-matrix.yml @@ -127,6 +127,20 @@ jobs: with: repo-token: ${{ secrets.GITHUB_TOKEN }} enable-sccache: ${{ (inputs.mode != 'pr' || github.event.pull_request.head.repo.fork == false) && 'true' || 'false' }} + - name: Checkout SpatialBench generator + if: matrix.subcommand == 'spatialbench' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + repository: apache/sedona-spatialbench + ref: b9221a9c4b02b10db20611d79b4019d2b3c4b68e + path: spatialbench-generator + persist-credentials: false + - name: Build SpatialBench generator + if: matrix.subcommand == 'spatialbench' + working-directory: spatialbench-generator + run: | + cargo build --release --bin spatialbench-cli + echo "SPATIALBENCH_CLI=${GITHUB_WORKSPACE}/spatialbench-generator/target/release/spatialbench-cli" >> "${GITHUB_ENV}" - name: Install uv uses: spiraldb/actions/.github/actions/setup-uv@a746510eafaa926484c354541cfc49b2ec06cc63 # 0.18.6 with: @@ -206,6 +220,7 @@ jobs: --output results.json \ --ingest-jsonl results.ingest.jsonl \ --no-build \ + ${{ matrix.exclude_queries && format('--exclude-queries {0}', matrix.exclude_queries) || '' }} \ ${{ matrix.iterations && format('--iterations {0}', matrix.iterations) || '' }} \ ${{ matrix.scale_factor && format('--opt scale-factor={0}', matrix.scale_factor) || '' }} @@ -226,6 +241,7 @@ jobs: --output results.json \ --ingest-jsonl results.ingest.jsonl \ --no-build \ + ${{ matrix.exclude_queries && format('--exclude-queries {0}', matrix.exclude_queries) || '' }} \ ${{ matrix.iterations && format('--iterations {0}', matrix.iterations) || '' }} \ --opt remote-data-dir="$REMOTE_STORAGE" \ ${{ matrix.scale_factor && format('--opt scale-factor={0}', matrix.scale_factor) || '' }} @@ -255,7 +271,9 @@ jobs: cat comment.md >> "$GITHUB_STEP_SUMMARY" - name: Comment PR - if: inputs.mode == 'pr' && github.event.pull_request.head.repo.fork == false + if: >- + inputs.mode == 'pr' && github.event_name != 'workflow_dispatch' + && github.event.pull_request.head.repo.fork == false uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3 with: file-path: comment.md @@ -266,7 +284,9 @@ jobs: comment-tag: bench-pr-comment-${{ matrix.id }}-${{ inputs.matrix_preset }} - name: Comment PR on failure - if: failure() && inputs.mode == 'pr' && github.event.pull_request.head.repo.fork == false + if: >- + failure() && inputs.mode == 'pr' && github.event_name != 'workflow_dispatch' + && github.event.pull_request.head.repo.fork == false uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3 with: message: | diff --git a/bench-orchestrator/bench_orchestrator/ci_matrix/catalog.py b/bench-orchestrator/bench_orchestrator/ci_matrix/catalog.py index 9f8f0fee915..6a0f864b860 100644 --- a/bench-orchestrator/bench_orchestrator/ci_matrix/catalog.py +++ b/bench-orchestrator/bench_orchestrator/ci_matrix/catalog.py @@ -17,6 +17,7 @@ "pr-compact": "Pull-request SQL benchmarks for Vortex Compact plus Parquet controls.", "pr-all": "The union of the focused PR and PR Compact benchmark matrices.", "pr-full": "Every regular SQL benchmark at full PR target coverage.", + "pr-spatial": "SpatialBench across DuckDB targets.", "nightly": "Large-scale SF=100 TPC-H on NVMe and S3 at default targets.", } @@ -59,6 +60,11 @@ Format.VORTEX_COMPACT, ) COMPACT_DUCKDB_TARGETS = duck(Format.PARQUET, Format.VORTEX_COMPACT) +SPATIAL_TARGETS = duck( + Format.PARQUET, + Format.VORTEX, + Format.VORTEX_SPATIAL_NATIVE, +) DEFAULT = Coverage(DEFAULT_TARGETS) STANDARD = Coverage(STANDARD_TARGETS) @@ -77,6 +83,10 @@ DATAFUSION_VORTEX = Coverage(DATAFUSION_VORTEX_TARGETS) COMPACT = Coverage(COMPACT_TARGETS) COMPACT_DUCKDB = Coverage(COMPACT_DUCKDB_TARGETS) +SPATIAL = Coverage( + SPATIAL_TARGETS, + data_formats=(Format.PARQUET, Format.VORTEX, Format.VORTEX_SPATIAL_NATIVE), +) # Concrete benchmark cases @@ -196,6 +206,14 @@ "develop": STANDARD_WITH_DUCKDB, }, ), + BenchmarkCase( + id="spatialbench-nvme", + benchmark=Benchmark.SPATIALBENCH, + name="SpatialBench SF=0.6 on NVME", + scale_factor=0.6, + iterations=1, + runs={"pr-spatial": SPATIAL}, + ), BenchmarkCase( id="statpopgen", benchmark=Benchmark.STATPOPGEN, diff --git a/bench-orchestrator/bench_orchestrator/ci_matrix/model.py b/bench-orchestrator/bench_orchestrator/ci_matrix/model.py index 587f7f36da4..b6b5b54c2a3 100644 --- a/bench-orchestrator/bench_orchestrator/ci_matrix/model.py +++ b/bench-orchestrator/bench_orchestrator/ci_matrix/model.py @@ -24,6 +24,7 @@ class Coverage: targets: TargetSet data_formats: tuple[Format, ...] | None = None + exclude_queries: tuple[int, ...] = () @dataclass(frozen=True) diff --git a/bench-orchestrator/bench_orchestrator/ci_matrix/render.py b/bench-orchestrator/bench_orchestrator/ci_matrix/render.py index 8b814349a40..8153635511d 100644 --- a/bench-orchestrator/bench_orchestrator/ci_matrix/render.py +++ b/bench-orchestrator/bench_orchestrator/ci_matrix/render.py @@ -42,6 +42,8 @@ def _matrix_entry(benchmark: BenchmarkCase, coverage: Coverage) -> dict[str, obj entry["local_dir"] = benchmark.local_dir if benchmark.remote_key is not None: entry["remote_key"] = benchmark.remote_key + if coverage.exclude_queries: + entry["exclude_queries"] = ",".join(str(query) for query in coverage.exclude_queries) return entry diff --git a/bench-orchestrator/bench_orchestrator/config.py b/bench-orchestrator/bench_orchestrator/config.py index 4aceaa59ac5..dea90457b89 100644 --- a/bench-orchestrator/bench_orchestrator/config.py +++ b/bench-orchestrator/bench_orchestrator/config.py @@ -83,9 +83,8 @@ class Benchmark(Enum): Engine.LANCE: [Format.LANCE], } -# Engines each benchmark can run on. Benchmarks default to *every* engine; list one here only to -# restrict it. SpatialBench's queries use DuckDB-specific `ST_*` spatial SQL that DataFusion has no -# functions for yet. +# Engines each benchmark can run on. Benchmarks default to every engine. Add an entry only to +# restrict a benchmark. BENCHMARK_ENGINES: dict[Benchmark, frozenset[Engine]] = { Benchmark.SPATIALBENCH: frozenset({Engine.DUCKDB}), } diff --git a/bench-orchestrator/tests/test_config.py b/bench-orchestrator/tests/test_config.py index e2b09b06141..8ce9042eb32 100644 --- a/bench-orchestrator/tests/test_config.py +++ b/bench-orchestrator/tests/test_config.py @@ -58,7 +58,6 @@ def test_resolve_axis_targets_filters_unsupported_combinations() -> None: def test_resolve_axis_targets_skips_engines_a_benchmark_cannot_run() -> None: - # SpatialBench is DuckDB-only (ST_* spatial SQL), so the DataFusion axis is dropped with a warning. targets, warnings = resolve_axis_targets( [Engine.DATAFUSION, Engine.DUCKDB], [Format.PARQUET, Format.VORTEX], diff --git a/bench-orchestrator/tests/test_matrix.py b/bench-orchestrator/tests/test_matrix.py index d0f765be246..3a83b3877bc 100644 --- a/bench-orchestrator/tests/test_matrix.py +++ b/bench-orchestrator/tests/test_matrix.py @@ -47,6 +47,7 @@ "pr-compact": COMPACT_IDS, "pr-all": PR_ALL_IDS, "pr-full": REGULAR_IDS, + "pr-spatial": ("spatialbench-nvme",), "nightly": ("tpch-nvme", "tpch-s3"), } @@ -76,6 +77,7 @@ def test_pr_target_selection() -> None: pr = {entry["id"]: entry for entry in _entries("pr")} pr_compact = {entry["id"]: entry for entry in _entries("pr-compact")} pr_full = {entry["id"]: entry for entry in _entries("pr-full")} + pr_spatial = {entry["id"]: entry for entry in _entries("pr-spatial")} assert _targets(pr["tpch-nvme"]) == { ("datafusion", "parquet"), @@ -86,6 +88,12 @@ def test_pr_target_selection() -> None: assert ("datafusion", "lance") in _targets(develop["tpch-nvme"]) assert all(("datafusion", "lance") not in _targets(entry) for entry in pr_full.values()) assert "vortex-compact" in cast("list[str]", pr_full["clickbench-nvme"]["data_formats"]) + assert _targets(pr_spatial["spatialbench-nvme"]) == { + ("duckdb", "parquet"), + ("duckdb", "vortex"), + ("duckdb", "vortex-spatial-native"), + } + assert "exclude_queries" not in pr_spatial["spatialbench-nvme"] for entry in pr_compact.values(): targets = _targets(entry) assert {file_format for _engine, file_format in targets} == {"parquet", "vortex-compact"} diff --git a/benchmarks/datafusion-bench/src/main.rs b/benchmarks/datafusion-bench/src/main.rs index 2d55f6ab305..6b1a8b06062 100644 --- a/benchmarks/datafusion-bench/src/main.rs +++ b/benchmarks/datafusion-bench/src/main.rs @@ -133,8 +133,9 @@ async fn main() -> anyhow::Result<()> { let benchmark = create_benchmark(args.benchmark, &opts)?; + let query_corpus = benchmark.query_corpus(Engine::DataFusion)?; let filtered_queries = filter_queries( - benchmark.queries()?, + query_corpus, args.queries.as_ref(), args.exclude_queries.as_ref(), ); diff --git a/benchmarks/duckdb-bench/src/main.rs b/benchmarks/duckdb-bench/src/main.rs index c680d62f836..d46b78474c1 100644 --- a/benchmarks/duckdb-bench/src/main.rs +++ b/benchmarks/duckdb-bench/src/main.rs @@ -111,8 +111,9 @@ fn main() -> anyhow::Result<()> { let benchmark = create_benchmark(args.benchmark, &opts)?; + let query_corpus = benchmark.query_corpus(Engine::DuckDB)?; let filtered_queries = filter_queries( - benchmark.queries()?, + query_corpus, args.queries.as_ref(), args.exclude_queries.as_ref(), ); diff --git a/vortex-bench/sql/spatialbench.md b/vortex-bench/sql/spatialbench.md index 4b67405aaeb..85d6a9178ba 100644 --- a/vortex-bench/sql/spatialbench.md +++ b/vortex-bench/sql/spatialbench.md @@ -1,27 +1,30 @@ # SpatialBench benchmark -The [Apache Sedona SpatialBench](https://sedona.apache.org/spatialbench/) spatial -analytics benchmark: twelve queries (Q1 ... Q12 in [`spatialbench.sql`](./spatialbench.sql), -DuckDB dialect) over a trips/zones schema, exercising spatial predicates and functions such -as `ST_DWithin`, `ST_Intersects`, and `ST_Distance`. The query logic matches upstream -`sedona-spatialbench`; only formatting differs. +The [Apache Sedona SpatialBench](https://sedona.apache.org/spatialbench/) benchmark has twelve spatial analytics queries over a trips/zones schema. + +[`spatialbench/duckdb.sql`](./spatialbench/duckdb.sql) contains the DuckDB dialect. +The query logic matches upstream `sedona-spatialbench`. + +Engine dialects use the `sql//.sql` path. +The harness selects the matching file automatically. The harness lives in [`src/spatialbench`](../src/spatialbench). -## Running locally +## Local use ```bash vx-bench run spatialbench ``` -The default command compares the Parquet and Vortex WKB representations with DuckDB. To run the -native Vortex spatial representation explicitly: +The default command compares the Parquet and Vortex WKB representations with DuckDB. + +Run the native Vortex spatial representation: ```bash vx-bench run spatialbench --engine duckdb --format vortex-spatial-native ``` -To compare all three representations in one run: +Compare all three representations: ```bash vx-bench run spatialbench --engine duckdb --format parquet,vortex,vortex-spatial-native diff --git a/vortex-bench/sql/spatialbench.sql b/vortex-bench/sql/spatialbench/duckdb.sql similarity index 100% rename from vortex-bench/sql/spatialbench.sql rename to vortex-bench/sql/spatialbench/duckdb.sql diff --git a/vortex-bench/src/benchmark.rs b/vortex-bench/src/benchmark.rs index 47de30a5faf..d10230751a8 100644 --- a/vortex-bench/src/benchmark.rs +++ b/vortex-bench/src/benchmark.rs @@ -3,6 +3,7 @@ //! Core benchmark trait and types. +use std::fs; use std::path::Path; use arrow_schema::Schema; @@ -12,6 +13,18 @@ use url::Url; use crate::BenchmarkDataset; use crate::Engine; use crate::Format; +use crate::workspace_root; + +pub(crate) fn read_query_file(path: &Path) -> anyhow::Result> { + let contents = fs::read_to_string(path)?; + Ok(contents + .split_terminator(';') + .map(str::trim) + .filter(|statement| !statement.is_empty()) + .enumerate() + .map(|(index, statement)| (index + 1, statement.to_owned())) + .collect()) +} /// Specification for a table in a benchmark dataset. #[derive(Debug)] @@ -35,6 +48,22 @@ pub trait Benchmark: Send + Sync { /// Get all available queries for this benchmark fn queries(&self) -> anyhow::Result>; + /// Get the equivalent query corpus for one engine dialect. + /// + /// If `vortex-bench/sql//.sql` exists, this method loads that file. + /// Otherwise, it uses the benchmark's default queries. + fn query_corpus(&self, engine: Engine) -> anyhow::Result> { + let engine_queries = workspace_root() + .join("vortex-bench") + .join("sql") + .join(self.dataset_name()) + .join(format!("{engine}.sql")); + if engine_queries.try_exists()? { + return read_query_file(&engine_queries); + } + self.queries() + } + /// SQL an `engine` must run before this benchmark's queries (e.g. loading engine /// extensions). Runners replay these after every (re)open. Default: none. fn engine_init_sql(&self, _engine: Engine) -> Vec { diff --git a/vortex-bench/src/bin/data-gen.rs b/vortex-bench/src/bin/data-gen.rs index 35c77d70c48..039b22a61a6 100644 --- a/vortex-bench/src/bin/data-gen.rs +++ b/vortex-bench/src/bin/data-gen.rs @@ -92,6 +92,10 @@ async fn main() -> anyhow::Result<()> { { generate_duckdb(&base_path, &*benchmark)?; } + + for format in &args.formats { + benchmark.prepare_format(*format, &base_path).await?; + } } Ok(()) diff --git a/vortex-bench/src/spatialbench/benchmark.rs b/vortex-bench/src/spatialbench/benchmark.rs index 693a174eae1..e43a30c97b2 100644 --- a/vortex-bench/src/spatialbench/benchmark.rs +++ b/vortex-bench/src/spatialbench/benchmark.rs @@ -3,7 +3,6 @@ //! SpatialBench benchmark implementation -use std::fs; use std::path::Path; use url::Url; @@ -13,6 +12,7 @@ use crate::BenchmarkDataset; use crate::Engine; use crate::Format; use crate::TableSpec; +use crate::benchmark::read_query_file; use crate::spatialbench::datagen; use crate::spatialbench::datagen::Table; use crate::utils::file::resolve_data_url; @@ -62,22 +62,15 @@ impl Benchmark for SpatialBenchBenchmark { "vortex-bench/sql/spatialbench.md" } - /// All SpatialBench queries, numbered started at Q1 in `spatialbench.sql` file order. + /// All SpatialBench queries, numbered from Q1 in `spatialbench/duckdb.sql` file order. fn queries(&self) -> anyhow::Result> { - // `;`-separated; a `;` must not appear in a comment, or it would split a statement in two. - let queries_file = workspace_root() - .join("vortex-bench") - .join("sql") - .join("spatialbench") - .with_extension("sql"); - let contents = fs::read_to_string(queries_file)?; - Ok(contents - .split_terminator(';') - .map(str::trim) - .filter(|stmt| !stmt.is_empty()) - .enumerate() - .map(|(idx, stmt)| (idx + 1, stmt.to_string())) - .collect()) + read_query_file( + &workspace_root() + .join("vortex-bench") + .join("sql") + .join("spatialbench") + .join("duckdb.sql"), + ) } async fn generate_base_data(&self) -> anyhow::Result<()> { @@ -204,3 +197,19 @@ fn zone_parquet_present(parquet_dir: &Path) -> bool { .map(|mut paths| paths.next().is_some()) .unwrap_or(false) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn discovers_engine_query_corpora() -> anyhow::Result<()> { + let benchmark = SpatialBenchBenchmark::new("0.6".to_string(), None)?; + + let duckdb = benchmark.query_corpus(Engine::DuckDB)?; + + assert_eq!(duckdb.len(), 12); + assert!(duckdb[0].1.contains("ST_X(t.t_pickuploc)")); + Ok(()) + } +}