8000 feat: OpenOptions::allow_overrides gate (#869) by adityamukho · Pull Request #873 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- **`OpenOptions::allow_overrides`** (#869, part of #862): the embedder lists, as `Overridable` variants, the query keywords that may override the database's settings. The default, `Overridable::DEFAULTS` (`MaxDerivedFacts`, `MaxResults`), allows every keyword that could do so before, so no query changes. A query using a keyword the list leaves out is refused with the new **API-025** before it runs, from `execute`, `query`, `prepare` and `WriteTransaction::execute`; the list replaces the defaults rather than adding to them, and is not stored in the file. `BrowserDb` takes no options and uses the defaults. `Overridable` is `#[non_exhaustive]`.

### Changed

- **Ordinary average** (#777): `(avg ?v)` no longer raises PRS-027. Grouped and ungrouped queries use the same numeric population and arithmetic as window `avg`: skip nil and nonnumeric values, sum exactly, round the total once to Float, then divide by the numeric count. Integer totals are not saturated. Results are Float, or nil for an existing group without numbers; no matching rows yields no rows. The total can overflow before division, and division rounds separately. Existing window behavior is unchanged. Retained exact-sum allocations for ordinary `sum` and `avg` are now charged to the query memory limit.
Expand All @@ -30,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Documentation

- **Configuration reference** (#869): [docs/CONFIGURATION.md](docs/CONFIGURATION.md) lists every `OpenOptions` setting, where else it can be changed, and how `allow_overrides` gates query text.
- **Heavy WAL entry replay test** (#865): a nightly acceptance test writes one WAL entry of 1,000,000 facts, reopens without a checkpoint and checks every replayed fact through entity, attribute and attribute-value lookups, then checkpoints, verifies and reopens. It passes; no production change.
- **Failure-path correctness map** (#843): [X08's correctness-target map](docs/benchmarks/843-failure-correctness/README.md) ties invalid bounds, windows and binds, callback errors, exhaustion, corrupt pages, failed-handle retry, v1–v6 refusal and heavy single-entry WAL replay to 138 named tests at a pinned v3 revision, with their error and data invariants, and limits the C08 repair and C30 rollback claims to their fixtures. Gaps found: heavy WAL replay through `Minigraf::open` (#865) and the recursion round bound (#861). X08 stays excluded. No production, test, public API or nightly-route change.

Expand Down
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ cargo run < demos/demo_negation.txt

5. **`src/repl.rs`** — Interactive REPL; TTY-aware (suppresses prompts/banner for piped input)

6. **`src/db.rs`** — Public API: `Minigraf::open/execute/query/fact_log/prepare/begin_write/checkpoint/save/verify/rebuild_indexes`, `WriteTransaction`, `IntegrityReport`, `OpenOptions::page_cache_size`, `OpenOptions::read_only` (#429: shared lock, nothing written, writes are `API-014`), `OpenOptions::query_memory_limit` (#549: `QRY-012`, off by default), `OpenOptions::transaction_chunk_facts` (#550)
6. **`src/db.rs`** — Public API: `Minigraf::open/execute/query/fact_log/prepare/begin_write/checkpoint/save/verify/rebuild_indexes`, `WriteTransaction`, `IntegrityReport`, `OpenOptions::page_cache_size`, `OpenOptions::read_only` (#429: shared lock, nothing written, writes are `API-014`), `OpenOptions::query_memory_limit` (#549: `QRY-012`, off by default), `OpenOptions::transaction_chunk_facts` (#550), `OpenOptions::allow_overrides` (#869: which query keywords may override settings, `API-025`; see `docs/CONFIGURATION.md`)
- `src/overrides.rs`: `Overridable` (`DEFAULTS`, `keyword`) and `check_overrides`, called after parsing at every query entry point (`BrowserDb` uses `DEFAULTS`)
- `src/cursor.rs`: `Cursor` / `Batch` — owned, `Send` cursor returned by `Minigraf::query` and `PreparedQuery::query` (#432); pulls engine queries one internal batch at a time (`close`/drop stops the work; a read error after some rows comes on the next call), other queries computed at open; holds a `Snapshot` (one state of the store, its committed generation pinned, #543) until dropped
- `src/fact_log.rs`: `FactLog` / `FactFilter` / `FactRecord` / `FactOrder` — every fact version streamed by `Minigraf::fact_log` (#430): tx order (multi-pass, bounded window) or storage order, key-level filters; an open log pins the committed generation (`storage::pins`), so checkpoints run but do not reuse its pages (`API-013` deprecated, #543)
- `src/log_writer.rs`: `LogWriter` — builds a new file from `FactRecord`s with their tx and valid-time kept (#431): tx-ordered appends with holes (`API-015`/`API-016`), batched copy-on-write checkpoints at tx boundaries, no WAL, built at `<path>.partial` and renamed by `finish` (`STG-043` if the target exists)
Expand Down Expand Up @@ -244,7 +245,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**2062 tests** (2041 active, 21 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
**2071 tests** (2050 active, 21 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

The #760 deterministic storage suite adds three native unit tests in a separate `--cfg minigraf_shuttle` build, outside the default count. Run only `graph::storage::deterministic::` with that cfg; ordinary unit tests do not enter its scheduler. `deterministic-schedules.yml` runs relevant v3 PRs on Linux/macOS/Windows plus Linux MSRV, and larger nightly/manual searches with replay artifacts. The identical main workflow activates the v3 schedule after both PRs merge. See `docs/testing/deterministic-schedules.md` for scope, controls and findings.
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ File-backed databases limit string values to 4,068 bytes (`WAL-003`). In-memory

**Durability tuning:** every write is `fsync`'d immediately by default (`SyncMode::Full`). Bulk loaders/migrations that can safely re-run from a checkpoint watermark on failure can trade that for throughput with `OpenOptions::new().synchronous(SyncMode::Normal)` — `checkpoint()` still fsyncs unconditionally in both modes. See the [Performance Tuning](https://project-minigraf.github.io/minigraf-docs/latest/performance-tuning/#configuration-knobs) for the full tradeoff and the write-batching pattern that pairs with it.

**Configuration:** [docs/CONFIGURATION.md](docs/CONFIGURATION.md) lists every `OpenOptions` setting, which ones query text can override, and how `OpenOptions::allow_overrides` lets the embedder refuse those keywords (`API-025`, #869).

**Running Minigraf where data loss matters:** [docs/DURABILITY.md](docs/DURABILITY.md) covers what survives a process crash and a power loss, recovery on open, supported filesystems and containers, backups and restores, `verify()` and `rebuild_indexes()`, upgrading from v2.x, and which errors mean retry, reopen or restore.

## Contributing
Expand Down
82 changes: 82 additions & 0 deletions docs/CONFIGURATION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Configuration

Minigraf needs no configuration: `Minigraf::open("app.graph")` uses safe
defaults. `OpenOptions` changes them for one handle. Nothing here is stored
in the `.graph` file; each open sets it.

This page lists every `OpenOptions` setting, where else it can be changed,
and how the embedder controls which settings query text may override
(`allow_overrides`, #862).

## Settings and where they can be changed

"Query text" means keywords in the query vector, such as
`:max-derived-facts 1000`. Query text is gated by
[`allow_overrides`](#allow_overrides). Settings changed from application
code are not gated.

| `OpenOptions` field | Default | Also changed by | Semantics of an override |
|---|---|---|---|
| `max_derived_facts` | 1,000,000 | query text: `:max-derived-facts N` | Replaces the database value for that query, up or down |
| `max_results` (deprecated) | no limit of its own | query text: `:max-results N` | Alone, replaces the database value; with `:max-derived-facts`, the lower applies |
| `query_memory_limit` | no limit | — (query keyword and `PreparedQuery` setter planned, #870) | — |
| round limit | 1000, fixed | — (`OpenOptions::max_iterations` and `:max-iterations` planned, #861) | — |
| `transaction_chunk_facts` | 65,536 | — (per-transaction `TxOptions` planned, #871) | — |
| `synchronous` | `SyncMode::Full` | — (per-transaction `TxOptions` planned, #871) | — |
| `wal_checkpoint_threshold` | 1000 | open only | Database-wide checkpoint policy |
| `page_cache_size` | 256 pages | open only | Shared cache |
| `allow_unlocked` | `false` | open only | Decided when the file is opened |
| `read_only` | `false` | open only | Decided when the file is opened |
| `allow_overrides` | `Overridable::DEFAULTS` | open only | Policy for query text |

`BrowserDb` takes no options: it always runs with the defaults, including
`Overridable::DEFAULTS`.

## `allow_overrides`

`OpenOptions::allow_overrides` lists the query keywords that may override the
database's settings, as `Overridable` variants:

| Variant | Keyword | In `DEFAULTS` |
|---|---|---|
| `Overridable::MaxDerivedFacts` | `:max-derived-facts` | yes |
| `Overridable::MaxResults` | `:max-results` | yes |

The default, `Overridable::DEFAULTS`, allows every keyword that could
override a setting before the list existed, so existing queries keep
working.

A query that uses a keyword the list leaves out is refused with
[`API-025`](ERROR_REFERENCE.md#api-025-query-option-override-not-allowed)
before it runs. The check depends only on whether the keyword is present,
never on its value, and the override is never ignored or clamped. It applies
to `Minigraf::execute`, `Minigraf::query`, `Minigraf::prepare` (a refused
query yields no `PreparedQuery`) and `WriteTransaction::execute` (a refused
query leaves the transaction usable).

The list **replaces** the defaults; it is not merged with them. Duplicates
have no effect.

```rust
use minigraf::{OpenOptions, Overridable};

// Query text may not override any limit, e.g. when it comes from users or an LLM.
let db = OpenOptions::new()
.allow_overrides([])
.path("app.graph")
.open()?;

// Only :max-results; :max-derived-facts is refused with API-025.
let db = OpenOptions::new()
.allow_overrides([Overridable::MaxResults])
.open_memory()?;

// The defaults plus another variant (for keywords added later).
let opts = OpenOptions::new().allow_overrides(
Overridable::DEFAULTS.iter().copied().chain([/* Overridable::… */]),
);
```

`Overridable` is `#[non_exhaustive]`: later releases add variants for new
keywords, such as `:query-memory-limit` (#870, not in `DEFAULTS`) and
`:max-iterations` (#861, in `DEFAULTS`).
13 changes: 13 additions & 0 deletions docs/ERROR_REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ with no `CodedError` anywhere in its chain.
| API-022 | Missing bind value for slot | Database API |
| API-023 | Bind slot not permitted in attribute position | Database API |
| API-024 | Bind slot type mismatch | Database API |
| API-025 | Query option override not allowed | Database API |
| INT-000 | Unclassified internal error | Internal |
| INT-001 | WriteTransaction already in progress on this thread | Internal |
| INT-002 | Invalid entity (API layer) | Internal |
Expand Down Expand Up @@ -3651,6 +3652,18 @@ ValueSet::new(vec![inner]) // API-021

**Scenario**: `:as-of $tx` is bound to `Val(Integer(5))` instead of `TxCount(5)`.

### API-025 Query option override not allowed

**Error text**: `query option {} is not allowed on this database; OpenOptions::allow_overrides does not list Overridable::{}`

**Cause**: The query used a keyword that overrides one of the database's settings, such as `:max-derived-facts` or `:max-results`, and the embedder's `OpenOptions::allow_overrides` list leaves that keyword out. The query is refused before it runs, by `execute`, `query`, `prepare` and `WriteTransaction::execute`; a refused statement in a `WriteTransaction` leaves the transaction usable. When a query uses several refused keywords, the message names the first in the order `:max-derived-facts`, `:max-results`. The list is not stored in the file: each open sets it, and the default (`Overridable::DEFAULTS`) allows both keywords.

**Resolution**:
- Remove the keyword from the query and rely on the database's own setting.
- Or, if the embedder means to let queries override it, add the `Overridable` variant to `OpenOptions::allow_overrides`. The list replaces the defaults; chain `Overridable::DEFAULTS` to add to them.

**Scenario**: An application opens its database with `OpenOptions::new().allow_overrides([])` so that query text from its users cannot raise `max_derived_facts`; a user query ending in `:max-derived-facts 100000000` is refused with API-025.

---

## INT — Internal Errors
Expand Down
11 changes: 6 additions & 5 deletions docs/TEST_COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

| | Default features | With `--features test-hooks` |
|---|---|---|
| **Active** | 2,041 | 2,076 |
| **Active** | 2,050 | 2,085 |
| **Ignored** | 21 | 24 |
| **Total** | 2,062 | 2,100 |
| **Total** | 2,071 | 2,109 |

Counts above are the current Cargo test inventory (active versus ignored),
including #777, #789, #781, #794, #795, #837 and #865 (Linux). Verification results below identify which suites ran.
including #777, #789, #781, #794, #795, #837, #865 and #869 (Linux). Verification results below identify which suites ran.

**#838 manual migration verification**: the [manual sweep](benchmarks/838-migration/README.md)
checks every fixture value and cursor snapshot before accepting timings, including
Expand Down Expand Up @@ -641,14 +641,15 @@ These are not `#[test]`s; run them with `cargo kani` or `.github/workflows/kani.
- The forms next to them still run: an option followed by a keyword, `(- ?n 1)`, a zero-argument rule.
- Thirteen new `tests/grammar/invalid/syntax/` fixtures.
- **Grammar page** (`tests/grammar_doc.rs`, 10 tests; #643): `docs/GRAMMAR.md` matches the EBNF rendered from `tests/grammar/grammar.pest`. Each rendering rule is checked on a small grammar: sequence, choice and grouping, names and builtins, `SOI`/`EOI` dropped, lookaheads as comments, bounded repetition, quoting, token marks, section titles, line breaking, the whitespace note and the comment note it joins.
- **`OpenOptions::allow_overrides`** (#869; `tests/allow_overrides_test.rs`, 6 tests; `src/overrides.rs`, 2 unit tests; 1 doc test on `OpenOptions::allow_overrides`): the defaults allow `:max-derived-facts` and `:max-results`; an empty list refuses each with `API-025` from `execute`, `query`, `prepare` and `WriteTransaction::execute`, and queries without them still run; a list replaces the defaults; a refused statement leaves the write transaction usable; `DEFAULTS` read back, chained and with duplicates; the list holds on a file database and is not persisted; `:max-derived-facts` is reported first when both keywords are refused. API-025's registration and documentation are checked by the registry tests.

## Browser and WASI

None of these are counted in the summary. The wasm32 targets run in CI (`.github/workflows/wasm.yml`).

- **In a browser** (`wasm-pack test --headless`, Firefox and Chrome): 61 `wasm_bindgen_test` tests. The final #641 snapshot passed in headless Firefox on the bench; Chrome remains a CI run.
- **In a browser** (`wasm-pack test --headless`, Firefox and Chrome): 62 `wasm_bindgen_test` tests. The final #641 snapshot passed in headless Firefox on the bench; Chrome remains a CI run.
- Three committed-memory-backend input-order fixtures: scalar Sort omission, streaming counts with Boolean/float representatives and mixed numeric group order, and partition windows preserving float edges and numeric group classes (#641).
- Twenty-five for `BrowserDb`, including IndexedDB persistence, the cursor, and the flush returning every dirty page and failing on one it cannot read (#470).
- Twenty-six for `BrowserDb`, including IndexedDB persistence, the default `allow_overrides` list (#869), the cursor, and the flush returning every dirty page and failing on one it cannot read (#470).
- Eleven for `BrowserBufferBackend` (which used `#[test]` and never ran before).
- Two checking that IndexedDB page keys accept only whole numbers from 0 to 2^53.
- Fifteen for `BrowserDb` user functions (#180), below.
Expand Down
Loading
Loading
0