chore(bq|sf|rs|pg|db|ora|benchmark): cross-cloud spatial-index benchmark suite [sc-550438]#606
Open
vdelacruzb wants to merge 26 commits into
Open
chore(bq|sf|rs|pg|db|ora|benchmark): cross-cloud spatial-index benchmark suite [sc-550438]#606vdelacruzb wants to merge 26 commits into
vdelacruzb wants to merge 26 commits into
Conversation
Adds the infrastructure for running per-function timing benchmarks on Oracle modules, paralleling the existing test target. - bench() helper added to test_utils/__init__.py: single timed run, prints markdown row to stdout + appends to benchmark_results.md with per-file ### filename sub-sections. - list_functions.js extended with --type=benchmark to discover modules/benchmarks/<module>/benchmark_<FUNCTION>.py files. - New `make benchmark` target in core/clouds/oracle/modules/Makefile mirrors `make test`'s modules=/functions= filter shape, prepends a timestamped ## Benchmark run header to benchmark_results.md, then invokes each matching benchmark via python. - benchmark_results.md added to .gitignore. - One example benchmark: benchmark_H3_KRING.py. - README at clouds/oracle/modules/benchmarks/README.md documents layout, running, output format, and how to author new benchmarks. This is Oracle scaffolding only; BigQuery and Snowflake equivalents follow in subsequent commits/PRs once the Oracle pattern is verified. The 18 H3 + 18 quadbin per-function benchmark files follow once the scaffolding is approved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a benchmarking framework for Oracle modules, featuring a discovery script for benchmark files, a timing utility in the test helpers, and a new Makefile target. Review feedback identifies a missing variable declaration in the JavaScript discovery script and suggests refactoring the timing logic to exclude database connection overhead, ensuring more accurate benchmark results.
Two fixes responding to review feedback:
1. bench() doesn't belong in test_utils — different concern. Moved
to a new clouds/oracle/common/benchmark_utils/ package, sibling of
test_utils. benchmark_utils imports run_query from test_utils to
reuse its connection caching.
2. Per-file sys.path.insert bootstrap was ugly. Removed in favor of
Make setting PYTHONPATH=$(COMMON_DIR) when invoking benchmarks.
Mirrors how pytest auto-loads modules/test/__init__.py for tests:
the orchestrator (make / pytest) handles path setup so per-file
code stays clean.
Per-function benchmark files now look like:
from benchmark_utils import bench
SOURCE_TABLE = '@@ORA_SCHEMA@@.SAMPLE_TABLE'
bench(name='...', sql='...')
For single-file re-runs (after a fix), use the Make filter:
`make benchmark functions=H3_KRING`. Direct `python <file>`
requires the user to set PYTHONPATH manually; the Make form is
the preferred path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small alignments after review: 1. Rename modules/benchmarks/ → modules/benchmark/ (singular). The project's existing convention is singular collective nouns: modules/test/, modules/sql/, modules/doc/. Industry leans plural (asv, Google Benchmark) but internal consistency wins. 2. Add modules/benchmark/__init__.py with the same sys.path bootstrap that modules/test/__init__.py uses. With this, `python -m clouds.oracle.modules.benchmark.h3.benchmark_H3_KRING` works without manual PYTHONPATH; full structural parity with tests. 3. Replace per-file docstring with the standard `# Copyright (c) 2026, CARTO` header, matching the format used in test_*.py files. Updated list_functions.js subdir = 'benchmark', Makefile and README references accordingly. Side-by-side parity: Tests Benchmarks ----- ---------- common/test_utils/__init__.py common/benchmark_utils/__init__.py modules/test/__init__.py (path setup) modules/benchmark/__init__.py (path setup) modules/test/<m>/test_<F>.py modules/benchmark/<m>/benchmark_<F>.py make test modules=X functions=Y make benchmark modules=X functions=Y list_functions.js list_functions.js --type=benchmark pytest runner direct python runner Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nore intent) Working-tree copy preserved for local benchmarking; existing gitignore rule was anchored to the wrong path and never took effect, so the file was committed by accident.
vdelacruzb
changed the title
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
1 participant
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
Cross-cloud spatial-index benchmark suite. Replaces the Oracle-only scaffolding originally scoped for sc-550438 — now covers all six supported clouds (BigQuery, Snowflake, Redshift, Postgres, Databricks, Oracle) with per-function timing benchmarks for H3 and Quadbin modules.
What's in this PR
Per cloud (
core/clouds/<cloud>/):common/benchmark_utils/(Py) orcommon/benchmark-utils.js(JS) —bench()/benchmark()helpers: cached per-process connection (so auth/setup isn't in the timer), CTAS output pattern (real tables, notCOUNT(*)),@@SCHEMA@@placeholder resolution, env-var-driven config dir.common/list_functions.js— extended with--type=benchmarkand--base-dirs="p1,p2"for cross-repo discovery (mirrors howbuild_modules.jsaggregates viaMODULES_DIRS).modules/benchmark/<module>/benchmark_<FN>.{py,bench.js}— one ~10-line file per function (19 H3 + 19 Quadbin per applicable cloud).modules/benchmark/config.template.json(committed) +config.json(gitignored).modules/Makefile—benchmarktarget with the samemodules=/functions=filters asmake test, pluskeep=1(preserve output tables) andverbose=1(full errors).Makefile—benchmark/benchmark-modulesper-cloud targets.Top-level (
core/Makefile) —make benchmark cloud=<cloud>mirroringmake test/make deploy.Each run appends a markdown table to
dist/benchmark_<ts>.md:The
Output Tablecolumn appears only withkeep=1.Cross-cloud alignment: same row counts / resolutions / modes everywhere (e.g. H3_COMPACT and H3_UNCOMPACT capped at 100 rows because Oracle's H3_UNCOMPACT hits an internal VARCHAR2(4000) ceiling on larger arrays).
Docs: each cloud's
README.mdgains amake benchmarkbullet +benchmark-modulesentry in the existing Filtering section..claude/rules/<cloud>.mddocuments the placeholder convention (<my-{ns}>.<my-table>/<my-{ns}>.<my-output-table>) used across docs and benchmark templates.Usage
Test plan
make benchmark cloud=<cloud>runs every configured benchmarkmodules=,functions=) work as inmake testkeep=1preserves output tables and shows the extra columnverbose=1writes the full error into the table cell (no truncation)make test/make deployunaffected; lint passes across all 6 clouds🤖 Generated with Claude Code