Add count() scan exercising SurrealDB's COUNT index - #252
Add count() scan exercising SurrealDB's COUNT index#252emmanuel-keller wants to merge 6 commits into
Conversation
Adds a new `count_count_idx` scan that mirrors the existing `count` scan but attaches a `with_index` block with `index_type = "count"`. The framework runs both a non-indexed baseline leg and an indexed leg, so the two timings sit side-by-side in the output. Engine-side handling: - SurrealDB builds the new `DEFINE INDEX ... ON TABLE record COUNT CONCURRENTLY` index; the optimizer picks it up for the existing `SELECT count() FROM record GROUP ALL` query (no scan SQL change). - Neo4j short-circuits index build and switches the indexed-leg query to the labeled, predicate-free form (`MATCH (n:Record) RETURN count(n)`) so the label count store fires (O(1), exact). - ArangoDB does the same with `RETURN LENGTH(record)` (collection counter, O(1), exact). Adds a `build_index` override since ArangoDB previously had none. - Postgres / MySQL / MariaDB / SQLite / MongoDB short-circuit `build_index` to a no-op for the count case; the indexed leg runs the same query as the baseline (no exact fast-count is available natively on those engines). - KV / engines without a `build_index` override keep the default `NotSupported`; their indexed cells render as `-`. `Index.fields` becomes `#[serde(default)]` so configs can omit it for index types that take no field list. Verified end-to-end against embedded SurrealDB (3.2.0-alpha, RocksDB, 1M rows, 200 samples): the indexed leg runs ~2x faster than the same-scan baseline (2.0s mean vs 4.3s mean; 64 ops vs 31 ops). The absolute speedup floor is set by per-query WebSocket / parse overhead — at the storage layer the index is effectively constant-time.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f808017f52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
After build_index returns Ok(()) for index_type="count" the bench driver unconditionally schedules drop_index, but the named index was never created. MySQL, MariaDB, MongoDB, and ArangoDB had drop paths that would fail in that case and abort the count_count_idx run during cleanup. Make each drop tolerant of a missing index: - MySQL: SHOW INDEX existence check before issuing DROP (no native IF EXISTS). - MariaDB: same SHOW INDEX check for parity with MySQL. - MongoDB: list_index_names() check before drop_index. - ArangoDB: add a drop_index override returning Ok(()); the only build path that succeeds is the COUNT no-op, so there's no real index to drop.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 100e42892c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- src/surrealdb2.rs: add the same no-op `build_index` short-circuit for `index_type = "count"` that the other engines have. Without it, the new `count_count_idx` scan reached the generic `DEFINE INDEX … FIELDS … ` branch with an empty fields list and aborted the run on `-d surrealdb2`. Drop is already safe — `drop_index` uses `REMOVE INDEX IF EXISTS`. - Apply `cargo fmt` to the per-engine count_idx predicates and SHOW INDEX lookups that fmt --check flagged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b0da2ac9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Now that `Index.fields` defaults to an empty vector, a `[scans.with_index]` block with no `fields` and no fieldless `index_type` silently passed config loading and only failed at runtime with backend-specific DDL errors (e.g. `CREATE INDEX … ()`, `FIELDS …`). Extend `validate_scan_index_ids` to enforce: - `fields` must be non-empty unless `index_type` is one of the known fieldless variants (currently just `count`). - A fieldless `index_type` rejects a non-empty `fields` list to flag inconsistent configs. Adds the fieldless-types list as a module-level const and three unit tests covering both directions of the new check.
Summary
count_count_idxscan that mirrors the existingcountscan but attacheswith_index = { index_type = "count" }, so the framework runs a non-indexed baseline leg and an indexed leg side-by-side.DEFINE INDEX … ON TABLE record COUNT CONCURRENTLYindex; the optimizer picks it up for the unchangedSELECT count() FROM record GROUP ALLquery.MATCH (n:Record) RETURN count(n); ArangoDB collection counter viaRETURN LENGTH(record)).build_indexto a no-op forindex_type = "count"; their indexed leg runs the same query as baseline (no exact native fast-count). KV / engines without anybuild_indexoverride keep the defaultNotSupported— their indexed cells render as-.Index.fieldsbecomes#[serde(default)]so configs can omit it for fieldless index types. Paired with a config-time validation extension invalidate_scan_index_ids: emptyfieldsis allowed only whenindex_typeis in aFIELDLESS_INDEX_TYPESlist (currently just"count"), and a non-emptyfieldspaired with a fieldlessindex_typeis also rejected — catches misconfigured TOML before any Docker / connection setup.drop_indexmade tolerant of missing indexes on the engines that lack nativeIF EXISTSsemantics, since the COUNT no-op build leaves nothing to drop: MySQL / MariaDB do aSHOW INDEXexistence check first; MongoDB callslist_index_names()first; ArangoDB ships adrop_indexoverride returningOk(())symmetric to its build no-op. Postgres / SQLite / Neo4j / SurrealDB / SurrealDB 2.x already usedIF EXISTSand are unchanged.Verification
Run against embedded SurrealDB (3.2.0-alpha, RocksDB, 1M rows, 200 samples):
count— no-indexcount_count_idx— no-indexcount_count_idx— indexedWithin
count_count_idx, the indexed leg is ~2× faster than its own non-indexed baseline. The absolute floor is set by per-query WebSocket / parse overhead — at the storage layer the COUNT index is effectively constant-time.Test plan
cargo check --all-featurescleancargo clippy --all-features --no-depscleancargo fmt --checkcleancargo test— 4 new unit tests on the fieldless-index validation pass (scan_with_index_*)