mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-12 01:45:14 +02:00
* MR-854: convert engine call sites to &dyn TableStorage; demote legacy methods
Phase 1b: every db.table_store.X(...) call site converts to
db.storage().X(...), reaching the storage layer through the sealed
TableStorage trait (returns &dyn TableStorage). Opaque SnapshotHandle
and StagedHandle replace bare lance::Dataset and Transaction in the
threaded values.
Phase 9: the inherent inline-commit methods on TableStore
(append_batch, merge_insert_batch{,es}, overwrite_batch,
create_btree_index, create_inverted_index) demote from pub to
pub(crate). Their only remaining direct users are table_store.rs
itself and the bulk loader's LoadMode::{Append, Overwrite, Merge}
concurrent fast-paths in loader::write_batch_to_dataset (no
two-phase shape in Lance 4.0.0 — closes after lance#6658 and #6666).
Docs:
- invariants.md \u00a7VI.23: drop "at the writer-trait surface"
qualifier; staged primitives are now the only engine surface.
- runs.md: residual matrix shrinks to delete_where and
create_vector_index (the two upstream-blocked residuals).
- forbidden_apis.rs: replace transitional language with the
current allow-list shape (table_store.rs + loader concurrent
fast-path only).
Files touched:
- changes/mod.rs, db/omnigraph.rs (+export/optimize/schema_apply/
table_ops.rs), exec/{merge,mod,mutation,staging}.rs,
loader/mod.rs, storage_layer.rs, table_store.rs,
tests/forbidden_apis.rs, docs/{invariants,runs}.md.
Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>
* MR-854: replace test-only inline-commit append callers with local Lance helpers
After demoting TableStore::append_batch from pub to pub(crate), the
integration tests in tests/recovery.rs and tests/staged_writes.rs
that previously called store.append_batch(...) directly to simulate
HEAD-ahead-of-manifest drift can no longer access the inherent
method. Replace those calls with small in-test helpers that do a raw
Dataset::append (the same body the inherent method runs).
- tests/helpers/mod.rs gains lance_append_inline (shared helper).
- tests/staged_writes.rs gets a file-local lance_append_inline_local
(staged_writes.rs does not import helpers::).
- tests/recovery.rs drops the unused TableStore import in the one
function whose store binding became unused after the conversion.
Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>
* MR-854: retrigger CI for flaky Test Workspace job
Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>
* MR-854: convert remaining table_store call sites in export.rs / read_blob
Two leftover `self.table_store.X` / `db.table_store.X` call sites were
missed in the initial sweep — flagged by Devin Review on PR #86. Both
now go through the trait surface:
- `entity_from_snapshot` (db/omnigraph/export.rs): switch from
`db.table_store.open_snapshot_table` + `db.table_store.scan` to
`db.storage().open_snapshot_at_table` + `db.storage().scan`.
- `read_blob` (db/omnigraph.rs): replace
`snapshot.open(table_key)` + `self.table_store.first_row_id_for_filter`
with `self.storage().open_snapshot_at_table` +
`self.storage().first_row_id_for_filter`. The follow-up
`take_blobs` call still needs an `Arc<Dataset>` (it's a Lance blob
accessor not surfaced through the trait), so we hand off via
`SnapshotHandle::into_arc()` with a comment.
After this commit, no engine code outside `table_store.rs` reaches the
inherent `TableStore` API — the docs/runs.md and docs/invariants.md
claim is now uniformly true.
Co-Authored-By: Ragnor Comerford <ragnor.comerford@gmail.com>
* MR-854: post-rebase doc fixes (Lance 6.0.1, MR-A framing, into_dataset note)
Reviewer feedback on the rebased PR:
* docs/dev/writes.md residuals matrix: drop demoted methods from the trait-surface table (now `pub(crate)`); keep only the two genuine trait-surface residuals (`delete_where`, `create_vector_index`); reframe under MR-A (Lance v7.x bump) per docs/dev/lance.md.
* tests/forbidden_apis.rs: update transitional allow-list header to (a) drop the truncate_table mislabel (truncate_table is a Lance Dataset method, not a TableStore method — overwrite_batch's internal call), (b) reframe trait-surface residuals under MR-A / Lance #6666.
* crates/omnigraph/src/storage_layer.rs::SnapshotHandle::{into_arc, into_dataset}: add single-ref invariant doc — both consume Arc via try_unwrap-or-clone; sibling SnapshotHandle clones across an await point force a deep Dataset clone.
* Replace lance-4.0.0 version refs with lance-6.0.1 in active source/test/dev-doc comments (storage_layer.rs, table_store.rs, table_ops.rs, schema_apply.rs, merge.rs, recovery.rs, staged_writes.rs, consistency.rs, docs/dev/execution.md, docs/user/query-language.md). Historical refs in docs/releases/v0.4.1.md and the canonical "Lance 4.0.0 → 6.0.1 migration" line in docs/dev/lance.md left intact.
No engine code changes.
* MR-854: update docs/dev/invariants.md Storage trait row + gap entry
Reviewer feedback: the docs reorg landed; the invariant row now lives in
docs/dev/invariants.md with stable headings (no more numbered §VI.23).
Update two pieces to reflect MR-854 completion:
* Status table 'Storage trait' row: was 'full call-site migration ... incomplete';
now 'engine call sites all route through db.storage() (MR-854); inline-commit
inherent methods are pub(crate)-demoted; capability/stat surfaces are roadmap'.
* 'Known Gaps' 'Storage abstraction' entry: was 'older inherent TableStore call
sites and inline residuals remain'; now names the closed scope (MR-854 — call
sites migrated, methods demoted, loader fast-paths) and the remaining
trait-surface residuals under MR-A (Lance v7.x bump) and Lance #6666.
Cross-links to docs/dev/lance.md and docs/dev/writes.md so the framing stays
co-located with the canonical Lance surface tracking.
* MR-854: remove dead inline-commit methods from the storage surface
The loader concurrent fast-path (write_batch_to_dataset) is only reached
for LoadMode::Overwrite — Append/Merge route through MutationStaging — so
its Append/Merge arms were unreachable. Collapse it to overwrite-only and
drop the now-unused mode params, which removes the only callers of:
- TableStorage::append_batch + TableStorage::merge_insert_batches (trait)
- TableStore::merge_insert_batch + merge_insert_batches (inherent)
create_btree_index / create_inverted_index had zero callers anywhere
(scalar index builds use the stage_* primitives). Remove both from the
trait and the inherent impl.
Inherent append_batch stays pub(crate): overwrite_batch and recovery
tests use it. Migrate the one trait-append_batch test caller
(seed_person_row) to stage_append + commit_staged. The merge_insert
FirstSeen-workaround rationale moves from the deleted merge_insert_batch
into stage_merge_insert (now the sole merge path). No behavior change.
Also corrects the inaccurate loader residual comment (the prior text
blamed Lance #6658/#6666, which are the delete and vector-index issues,
for keeping overwrite inline; a stage_overwrite primitive already exists
and schema_apply uses it).
* MR-854: seal db.storage() to staged-only; move residuals to InlineCommitResidual
Split the three remaining inline-commit writes (overwrite_batch,
delete_where, create_vector_index) off the TableStorage trait onto a new
sealed InlineCommitResidual trait, reachable only via the explicit
Omnigraph::storage_inline_residual() accessor. db.storage() now exposes
only staged primitives + reads, so engine code cannot couple a write
with a Lance HEAD advance through the default surface — MR-793 acceptance
§1 ("no public method commits as a side effect of writing") now holds by
construction, not by review + naming.
Call sites moved to storage_inline_residual(): loader overwrite
fast-path, the three mutation delete_where paths, the branch-merge
delete, and the vector-index build. Impl bodies are unchanged (same
delegation to the pub(crate) inherent methods); this is a pure surface
reshape with no behavior change.
The residual trait holds two genuinely upstream-blocked methods
(delete_where -> Lance #6658/v7.x, create_vector_index -> Lance #6666)
plus overwrite_batch, kept for the loader's cross-table bulk-overwrite
concurrency until its staged migration lands (tracked follow-up).
* MR-854 docs: describe the staged-only seal; fix stale Lance index URLs
- writes.md / invariants.md / AGENTS.md: the inline-commit residuals now
live on InlineCommitResidual behind db.storage_inline_residual(), so
acceptance §1 holds by construction rather than 'option (b)' per-method
enumeration. Drop the inaccurate 'until Lance exposes
Operation::Overwrite { fragments }' claim (that op exists; stage_overwrite
already builds it) and reframe overwrite_batch as a removable legacy
residual gated on the loader's bulk-overwrite concurrency.
- forbidden_apis.rs: rewrite the allow-list doc for the split surface.
- lance.md: the index spec pages moved from /format/table/index/ to
/format/index/ in Lance 6.x (the old paths 404). Fix all 13 URLs.
* MR-854: fix stale lance-4.0.0 comment refs flagged in review
Addresses greptile (exec/merge.rs) and aaltshuler's stale-version blocker:
update lance-4.0.0 -> 6.0.1 in the comment/doc refs within this PR's
footprint (exec/merge.rs, exec/mutation.rs, docs/dev/writes.md). Also
corrects exec/merge.rs to cite lance#6666 (not #6658) for
build_index_metadata_from_segments — that is the vector-index segment-commit
API; #6658 is the two-phase delete. (Pre-existing 4.0.0 refs in untouched
files like architecture.md/storage.md are main's incomplete migration
cleanup, left out of scope.)
* fix(storage): stage loader overwrites
* fix(storage): stage empty schema rewrites
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Ragnor Comerford <ragnor.comerford@gmail.com>
Co-authored-by: Ragnor Comerford <hello@ragnor.co>
180 lines
9.7 KiB
Markdown
180 lines
9.7 KiB
Markdown
# Query Execution, Mutations, and Loading
|
|
|
|
## Query execution (`exec/query.rs`)
|
|
|
|
Pipeline:
|
|
|
|
1. Parse + typecheck via `omnigraph-compiler`.
|
|
2. Lower to IR.
|
|
3. If `Expand` or `AntiJoin` is present, build (or fetch from `RuntimeCache`) a `GraphIndex`.
|
|
4. Run `execute_query` against the snapshot.
|
|
|
|
### Read flow — sequence
|
|
|
|
```mermaid
|
|
sequenceDiagram
|
|
autonumber
|
|
participant client as Client
|
|
participant og as Omnigraph::query<br/>(query.rs:7)
|
|
participant cmp as omnigraph-compiler
|
|
participant exec as execute_query<br/>(query.rs:347)
|
|
participant gi as GraphIndex<br/>(RuntimeCache)
|
|
participant ts as table_store
|
|
participant lance as Lance scanner
|
|
|
|
client->>og: query(target, source, name, params)
|
|
og->>og: ensure_schema_state_valid()<br/>resolve target → snapshot
|
|
og->>cmp: parse + typecheck_query (typecheck.rs:83)
|
|
cmp-->>og: CheckedQuery
|
|
og->>cmp: lower_query (lower.rs:11)
|
|
cmp-->>og: QueryIR (pipeline of IROp)
|
|
og->>exec: extract_search_mode + dispatch (query.rs:110)
|
|
exec->>gi: build / fetch GraphIndex<br/>(if Expand or AntiJoin)
|
|
gi-->>exec: CSR / CSC topology
|
|
loop for each IROp in pipeline
|
|
exec->>ts: scan with predicate / SIP
|
|
ts->>lance: filter · nearest · full_text_search
|
|
lance-->>ts: Stream of RecordBatch
|
|
ts-->>exec: RecordBatch stream
|
|
exec->>exec: factorize · expand · fuse · project
|
|
end
|
|
exec-->>og: QueryResult (RecordBatches)
|
|
og-->>client: serialized result
|
|
```
|
|
|
|
**Code paths:**
|
|
|
|
- Entry: `Omnigraph::query` at `crates/omnigraph/src/exec/query.rs:7`
|
|
- Search-mode extraction: `extract_search_mode` at `crates/omnigraph/src/exec/query.rs:110`
|
|
- Pipeline runner: `execute_query` at `crates/omnigraph/src/exec/query.rs:347`
|
|
- RRF fan-out: `execute_rrf_query` at `crates/omnigraph/src/exec/query.rs:393`
|
|
- Per-source-row BFS: `execute_expand` at `crates/omnigraph/src/exec/query.rs:675`
|
|
- Lance scan + pushdown: `execute_node_scan` at `crates/omnigraph/src/exec/query.rs:1027`
|
|
- Filter → SQL pushdown: `build_lance_filter` at `crates/omnigraph/src/exec/query.rs:1158`
|
|
|
|
### Multi-modal search modes (`SearchMode`)
|
|
|
|
The executor recognizes three modes that may be combined in a single query:
|
|
|
|
- **`nearest`** — vector ANN (uses Lance vector index; `LIMIT` required).
|
|
- **`bm25`** — BM25 over an inverted index.
|
|
- **`rrf`** — Reciprocal Rank Fusion of two rankings, with k (default 60).
|
|
|
|
Hybrid example: `order { rrf(nearest($d.embedding, $q), bm25($d.body, $q_text)) desc } limit 20`.
|
|
|
|
### Joins / set operations
|
|
|
|
- Joins are implicit: MATCH bindings + traversals are implemented as scans + CSR/CSC lookups.
|
|
- `not { … }` lowers to an `AntiJoin` over the inner pipeline.
|
|
|
|
### Scoped reads
|
|
|
|
- `query(target, source, name, params)` — at any branch or snapshot.
|
|
- `run_query_at(version, …)` — direct historical query at a manifest version.
|
|
|
|
### Concurrency
|
|
|
|
- Snapshot isolation per query: all reads inside a query use the same `Snapshot`.
|
|
- Readers and writers on different branches don't block each other.
|
|
|
|
## Mutation execution (`exec/mutation.rs`)
|
|
|
|
Resolves expression values to literals, converts to typed Arrow arrays (`literal_to_typed_array(lit, DataType, num_rows)`), then writes via Lance's two-phase distributed-write API at end-of-query:
|
|
|
|
- `insert` (no `@key`, edges) → accumulate into `MutationStaging.pending` (Append mode); finalize calls `stage_append` once per touched table.
|
|
- `insert` (`@key` node) → accumulate into `pending` (Merge mode); finalize calls `stage_merge_insert` once per touched table.
|
|
- `update` → scan committed via Lance + pending via DataFusion `MemTable` (read-your-writes), apply assignments, accumulate into `pending` (Merge mode).
|
|
- `delete` → still inline-commits via `delete_where` (Lance v6.0.1 has no public two-phase delete; `DeleteBuilder::execute_uncommitted` first ships in v7.0.0-beta.10 — tracked as MR-A in [docs/dev/lance.md](lance.md)); recorded into `MutationStaging.inline_committed`.
|
|
|
|
**D₂ parse-time rule.** A single mutation query is either insert/update-only or delete-only. Mixed → reject before any I/O. The check fires in `enforce_no_mixed_destructive_constructive(&ir)` inside `execute_named_mutation`.
|
|
|
|
Multi-statement mutations are atomic at the publisher commit boundary: every insert/update batch lives in memory until end-of-query, then exactly one `stage_*` + `commit_staged` runs per touched table, then `ManifestBatchPublisher::publish` commits the manifest atomically with per-table `expected_table_versions` CAS.
|
|
|
|
### Mutation flow — sequence
|
|
|
|
```mermaid
|
|
sequenceDiagram
|
|
autonumber
|
|
participant client as Client
|
|
participant og as Omnigraph::mutate_as<br/>(mutation.rs)
|
|
participant cmp as omnigraph-compiler
|
|
participant stg as MutationStaging<br/>(exec/staging.rs)
|
|
participant ts as table_store
|
|
participant lance as Lance dataset
|
|
participant pub as ManifestBatchPublisher
|
|
|
|
client->>og: mutate_as(branch, source, name, params, actor_id)
|
|
og->>cmp: parse + typecheck + lower_mutation_query
|
|
cmp-->>og: MutationIR
|
|
og->>og: enforce_no_mixed_destructive_constructive (D₂)
|
|
loop for each mutation op
|
|
og->>og: resolve literals + build batch
|
|
alt insert / update (accumulate)
|
|
og->>ts: open dataset @ pre-write version (first touch)
|
|
og->>stg: ensure_path + append_batch (PendingMode)
|
|
opt update — scan committed + pending
|
|
og->>ts: scan_with_pending (Lance + DataFusion MemTable union)
|
|
ts-->>og: matched batches
|
|
end
|
|
else delete (inline-commit, D₂ keeps separate)
|
|
og->>ts: delete_where (advances Lance HEAD)
|
|
og->>stg: record_inline (SubTableUpdate)
|
|
end
|
|
end
|
|
og->>stg: finalize(db, branch)
|
|
loop per pending table
|
|
stg->>ts: stage_append OR stage_merge_insert (one per table)
|
|
ts-->>stg: StagedWrite (transaction + fragments)
|
|
stg->>ts: commit_staged (advances Lance HEAD)
|
|
ts-->>stg: new Dataset
|
|
end
|
|
stg-->>og: (updates: Vec<SubTableUpdate>, expected_versions)
|
|
og->>pub: commit_updates_on_branch_with_expected
|
|
pub->>pub: publisher CAS (cross-table OCC on __manifest)
|
|
pub-->>og: new manifest version
|
|
og-->>client: MutationResult
|
|
```
|
|
|
|
**Code paths:**
|
|
|
|
- Entry: `Omnigraph::mutate_as` at `crates/omnigraph/src/exec/mutation.rs`
|
|
- Per-mutation orchestration: `mutate_with_current_actor` at `crates/omnigraph/src/exec/mutation.rs`
|
|
- D₂ check: `enforce_no_mixed_destructive_constructive` (in the same file)
|
|
- Per-op execution: `execute_insert`, `execute_update`, `execute_delete_node`, `execute_delete_edge`
|
|
- Pending-aware reads: `TableStore::scan_with_pending` / `count_rows_with_pending` at `crates/omnigraph/src/table_store.rs`
|
|
- Edge cardinality with pending: `validate_edge_cardinality_with_pending` at `crates/omnigraph/src/exec/mutation.rs`
|
|
- Per-query accumulator: `crates/omnigraph/src/exec/staging.rs` (`MutationStaging`, `PendingTable`, `PendingMode`, `finalize`)
|
|
- End-of-query Lance commit: `TableStore::stage_append`, `stage_merge_insert`, `commit_staged` at `crates/omnigraph/src/table_store.rs`
|
|
- Manifest commit primitive: `commit_updates_on_branch_with_expected` at `crates/omnigraph/src/db/omnigraph/table_ops.rs`
|
|
|
|
Atomicity guarantee for multi-statement mutations: a mid-query failure leaves Lance HEAD untouched on staged tables (no inline commit happened during op execution), so the next mutation proceeds normally with no `ExpectedVersionMismatch`. The publisher CAS at the very end either succeeds (manifest advances atomically across all touched sub-tables) or fails with a typed `ManifestConflictDetails::ExpectedVersionMismatch` (no partial publish). See [docs/dev/invariants.md](invariants.md) and [docs/dev/writes.md](writes.md).
|
|
|
|
## Bulk loader (`loader/mod.rs`)
|
|
|
|
- **JSONL only** in v1, with two record shapes:
|
|
- Node: `{"type":"NodeType", "data":{…}}`
|
|
- Edge: `{"edge":"EdgeType", "from":"src_id", "to":"dst_id", "data":{…}}`
|
|
- Lines starting with `//` are treated as comments.
|
|
- Schema validation on every row (typecheck, required props, blob base64 decoding).
|
|
- Edge endpoint resolution by node `@key`.
|
|
|
|
## Load modes (`LoadMode`)
|
|
|
|
| Mode | Semantics | Path (post-MR-794) |
|
|
|---|---|---|
|
|
| `Overwrite` | Replace all data in the target tables on the branch | Inline-commit per type, then publisher CAS at end-of-load. Truncate-then-append doesn't fit the staged shape; documented residual. |
|
|
| `Append` | Strict insert; duplicates error | In-memory `MutationStaging` accumulator; one `stage_append` + `commit_staged` per touched table at end-of-load; publisher CAS. |
|
|
| `Merge` | Upsert by `id` (`merge_insert`) | Same accumulator; one `stage_merge_insert` per touched table at end-of-load (Merge mode dedupes by `id`, last-write-wins); publisher CAS. |
|
|
|
|
For Append/Merge, a mid-load failure (RI / cardinality violation, validation error) leaves Lance HEAD untouched on the staged tables — the next load on the same tables proceeds normally with no `ExpectedVersionMismatch`. For Overwrite, a mid-load failure can still leave Lance HEAD on a partially-truncated table; the next overwrite replaces it.
|
|
|
|
## `load` vs `ingest`
|
|
|
|
- `load(branch, data, mode)` — direct load to a branch (single publisher commit per call).
|
|
- `ingest(branch, from, data, mode)` — branch-creating wrapper: if `branch` doesn't exist, fork it from `from` (default `main`) via `branch_create_from`, then call `load(branch, data, mode)`.
|
|
- Returns `IngestResult { branch, base_branch, branch_created, mode, tables[] }`.
|
|
- `ingest_as(actor_id)` records the actor on the resulting commit.
|
|
|
|
## Embeddings during load
|
|
|
|
If a node type has `@embed` properties, the loader calls the engine embedding client (Gemini, RETRIEVAL_DOCUMENT) per row to populate the vector column. See [embeddings.md](../user/embeddings.md).
|