8000 fix(query): expression = / != equate a keyword with a ref to its entity (#744) by adityamukho · Pull Request #748 · 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **An expression's `=` and `!=` equate a keyword with a ref to the entity it names** (#744). Patterns, joins, `not` bodies and `or` branches treat the keyword `:bob` as equal to a ref to the entity `:bob` names, but `[(= ?g :bob)]` compared the two values as they are, so with `?g` bound to that ref it kept no row and `[(!= ?g :bob)]` kept the row: the same question answered differently by how the query was written, with no error. `=` and `!=` now compare as joins do, in queries, rule bodies, `not` clauses and binding expressions, against a keyword constant, a keyword value or a bound `Val(Ref)`. A keyword still equals only the same keyword, and `<`, `>`, `<=` and `>=` on a keyword or a ref are still a type mismatch (no row).
- **`checkpoint`, `verify` and `rebuild_indexes` inside a `WriteTransaction` return `INT-001` instead of deadlocking** (#734). Called on a thread that holds a `WriteTransaction`, they waited on the write lock that the transaction already held, so the thread hung. They now return `INT-001`, as `execute`, `query`, `fact_log` and `begin_write` already did, and the transaction stays usable. `prepare` and `PreparedQuery::execute`/`query` make the same check; they could not deadlock, but they ran inside the transaction where `execute`/`query` refused. `checkpoint` deadlocked in every release since `WriteTransaction` was added; the fix for it and for the prepared calls ships in v2.x too.
- **A float constant in a pattern matches equal floats only** (#706). A float written in a pattern's value position, or bound to a `$slot` of a prepared query, matched every stored float within an absolute `f64::EPSILON` (about `2.2e-16`), so `[?e :m/x 0.0]` returned an entity whose `:m/x` is `1e-17`, while `[?e :m/x ?v] [(= ?v 0.0)]` did not. Above 1.0 the epsilon is smaller than the gap between adjacent floats, so only floats smaller than about `2.2e-16` in magnitude were affected: every one of them matched `0.0`, and `1e-20` matched `5e-20`. Float constants now compare by IEEE `==`, as joins, predicates and the indexes do: `0.0` matches `-0.0` and nothing else changes. The fix covers patterns in queries, rule bodies and `not` / `or` bodies, on the executor and the streaming engine. Nothing stored was wrong. The same fix ships in v2.0.10.
- **WAL entries of more than 1,000,000 facts replay** (#686). A WAL entry of more than 1,000,000 facts was taken as damage: replay stopped there, and the next open cut the WAL off at that entry, so after a crash, kill or close without a checkpoint that transaction and every later one in the WAL were lost, though each had committed. The writer had no such limit. Since #550 no v3 transaction writes such an entry, but a WAL left by v2.x can hold one. Replay now takes an entry's fact count as damage only when the bytes after it cannot hold that many facts. Affected every release since v0.5.0. Also fixed in v2.0.10.
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**1792 tests** (1779 passing, 13 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
**1796 tests** (1783 passing, 13 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.

**Testing conventions** — see the Testing Conventions section below before writing any tests.
Expand Down
7 changes: 4 additions & 3 deletions docs/TEST_COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

| | Default features | With `--features test-hooks` |
|---|---|---|
| **Passing** | 1,779 | 1,797 |
| **Passing** | 1,783 | 1,801 |
| **Ignored** | 13 | 14 |
| **Total** | 1,792 | 1,811 |
| **Total** | 1,796 | 1,815 |

**Verified**: 2026-10-24 with `cargo test --quiet`.

Expand Down Expand Up @@ -370,11 +370,12 @@ These tests compare the database against an independent answer rather than a sin
- the planner builds a keyless join on the side with the smaller product estimate (#575's shape, a selective outer side, a tie, reordering off).
- `meter.rs` (4 unit tests) checks charge, release, drop, peak and the allocator form.
- `Aggregate` charges its sorted map of groups until every row is built, and lets its hash table and group rows go first, with `sum-distinct` members charged once (`aggregate_finish_charges_its_sorted_groups`, `aggregate_finish_keeps_sum_distinct_members_charged_once`).
- **Join rules** (`tests/join_rules_test.rs`, 11 tests; #545)
- **Join rules** (`tests/join_rules_test.rs`, 15 tests; #545, #744)
- A join on a float value pairs `0.0` with `-0.0`, through the hash key and as a shared variable. A unit test checks that `-0.0` and `0.0` hash alike.
- A float constant in a pattern, bound to a `$slot`, in a rule body or in an `=` predicate matches equal floats only (`1e-17` does not match `0.0`; `0.0` matches `-0.0`; #706), in memory, on file before and after a checkpoint and after reopen.
- A variable bound to a keyword by one pattern and to the ref of the same entity by another returns the kind from the first pattern in `:where` text order: when the planner runs the patterns in another order, through the attribute position, before and after a rule call, and with a repeated pattern.
- An expression binding unifies with a pattern binding the same variable; the first binder in the text, pattern or expression, gives the value (#596).
- An expression's `=` and `!=` equate a keyword with a ref to the entity it names, as the pattern constant and the join do: either operand order, a keyword constant or value, a bound `Val(Ref)`, a binding expression, a rule body, a `not` clause, an `or` branch and a keyword whose name is a UUID, in memory, after a checkpoint and after reopen; a keyword still equals the same keyword only, and set elements still compare as they are (#744).
- A pseudo-attribute pattern never gives a variable its value, even when it runs first (#606; the plan-independence matrix checks it with reordering off).
- **Pseudo-attribute sources** (`tests/pseudo_source_test.rs`, 12 tests; #594)
- A per-fact pseudo-attribute reads the nearest earlier pattern in the text with the same entity term (#594's example in both text orders). Two pseudo-attributes share a source. `[:alice ?a _]` gives one row per fact. Constant and bound value terms check.
Expand Down
17 changes: 14 additions & 3 deletions src/query/datalog/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,11 +639,22 @@ fn to_float_pair(l: &Value, r: &Value) -> Result<(f64, f64), ()> {
Ok((lf, rf))
}

/// Whether `l` and `r` are equal as a join compares them: structurally,
/// except that a keyword equals a ref to the entity it names (#744).
fn expr_eq(l: &Value, r: &Value) -> bool {
match (l, r) {
(Value::Keyword(k), Value::Ref(u)) | (Value::Ref(u), Value::Keyword(k)) => {
edn_to_entity_id(&EdnValue::Keyword(k.clone())).is_ok_and(|e| e == *u)
}
_ => l == r,
}
}

fn eval_binop(op: &BinOp, l: Value, r: Value) -> Result<Value, ()> {
match op {
// Structural equality — works for all Value variants; no type mismatch error.
BinOp::Eq => return Ok(Value::Boolean(l == r)),
BinOp::Neq => return Ok(Value::Boolean(l != r)),
// Equality — works for all Value variants; no type mismatch error.
BinOp::Eq => return Ok(Value::Boolean(expr_eq(&l, &r))),
BinOp::Neq => return Ok(Value::Boolean(!expr_eq(&l, &r))),
_ => {}
}

Expand Down
Loading
Loading
0