FFFF fix(query): float constants match equal floats only (#706) by adityamukho · Pull Request #710 · 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
8000
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **An unknown aggregate or window function is an error whatever the data** (#414). Aggregate and window function names in `:find` are now resolved before the query runs: an unknown aggregate fails with the new `QRY-010` and an unknown window function with the new `QRY-011`, even on an empty database or when no row matches. Before, such a query returned no results until a row matched and then failed with `INT-029`/`INT-030`, which stay registered for internal inconsistencies only.
- **One current valid-time window per fact** (#435). At any transaction time, each `(entity, attribute, value)` now has exactly one current valid-time window: the one from its latest `transact`. A later assertion of the same fact replaces the earlier window, which stays visible through `:as-of` of the earlier transaction. Before, every window ever asserted stayed live until a `retract`, so closing an open window with `(transact {:valid-from vf :valid-to D} ...)` left the open one live and `:valid-at` after `D` still found the fact. Closing, extending or reopening a window is now a plain `transact` with the new bounds; `retract` still withdraws the fact. **This changes query results for existing data:** a fact asserted in several transactions with different windows is now valid only in its latest window at the current transaction time. A fact that was true over two separate periods can no longer have both windows live at once; model each period as its own entity, or read earlier periods with `:as-of`. Asserting one fact with two different windows in one `transact` is rejected with the new `API-011`, and nothing is written. Data written before this check with two windows in one transaction keeps both.
- **The `uuid` dependency no longer enables `v4`** (#439). The library only makes v5 UUIDs, so the `v4` feature and, on browser wasm, `js` are gone from its `uuid` dependency, and with them `getrandom`. A crate that called `Uuid::new_v4` only because Cargo feature unification turned `v4` on through minigraf must now enable `uuid/v4` itself. The release binary shrinks by 48 bytes.
- **Join answers no longer depend on the query plan** (#545). Two cases changed. A join on a float value now pairs `0.0` with `-0.0`, as a shared variable compared after the join key already did; before, `Value`'s hash told them apart, so the join key never paired them. NaN still equals nothing. Float constants in a pattern still match within `f64::EPSILON`. And a variable bound to a keyword by one pattern and to the ref of the entity the keyword names by another, which already joined, now returns the kind from the first of those patterns in the `:where` text. Before, it returned the kind from the pattern the planner ran first, so the answer could change with the selectivity of the other patterns, and differed under the `wasm` feature, which does not reorder. For example, with `[:alice :friend :bob]` stored as a keyword, `[:find ?b :where [?a :friend ?b] [?b :name "Bob"]]` returned the ref of `:bob`, because the planner runs the pattern with two constants first, and now returns the keyword `:bob`. Expression bindings follow the next entry (#596). The rule covers the top-level `:where` of queries with and without rules; inside `or`, `not` and rule bodies the kinds are unchanged.
- **Join answers no longer depend on the query plan** (#545). Two cases changed. A join on a float value now pairs `0.0` with `-0.0`, as a shared variable compared after the join key already did; before, `Value`'s hash told them apart, so the join key never paired them. NaN still equals nothing. (Float constants in a pattern matched within `f64::EPSILON` until #706; see Fixed.) And a variable bound to a keyword by one pattern and to the ref of the entity the keyword names by another, which already joined, now returns the kind from the first of those patterns in the `:where` text. Before, it returned the kind from the pattern the planner ran first, so the answer could change with the selectivity of the other patterns, and differed under the `wasm` feature, which does not reorder. For example, with `[:alice :friend :bob]` stored as a keyword, `[:find ?b :where [?a :friend ?b] [?b :name "Bob"]]` returned the ref of `:bob`, because the planner runs the pattern with two constants first, and now returns the keyword `:bob`. Expression bindings follow the next entry (#596). The rule covers the top-level `:where` of queries with and without rules; inside `or`, `not` and rule bodies the kinds are unchanged.
- **An expression binding a variable that another clause binds keeps only the rows where the two agree** (#596). `[(f …) ?x]` used to overwrite `?x` wherever the planner ran it, so when a pattern also bound `?x` the answer depended on whether that pattern ran first, and differed under the `wasm` feature, which does not reorder. Now the expression unifies: the row is kept when its result equals `?x`'s value, with the join equality of #545 (floats by IEEE `==`, a keyword equal to a ref to the entity it names, an integer never equal to a float, NaN equal to nothing). For example, with `[:a :age 30] [:b :age 40] [:p1 :score 41]`, `[:find ?a ?x :where [?e :age ?a] [:p1 :score ?x] [(+ ?a 1) ?x]]` returned `[30 31] [40 41]` natively and now returns `[40 41]`, as it did under `wasm`. `[(+ ?v 1) ?v]` never matches, and two expressions binding one variable must agree. When the two values differ only as keyword and ref, the variable returns the value of the first clause in the `:where` text that binds it, expressions included. This applies to `or` branches, `not` bodies and rule bodies too, on the executor and on the streaming engine.
- **A value a `not` / `not-join` body takes from the outer row compares as a join compares it** (#546). The executor had two paths for a negation body: the exclusion-set path compared the outer row's values as joins do (floats by IEEE `==`, a keyword equal to a ref to the entity it names), while the per-row path, taken when the body held an expression or a pseudo-attribute, in a query that invokes a rule, in an `or` branch and in a rule body, substituted each outer value into the body as a literal, so a float matched within `f64::EPSILON`. The answer therefore changed with the shape of the body: with `[:e1 :x 1e-20] [:g :y 2e-20]`, `[:find ?e :where [?e :x ?f] (not-join [?f] [?g :y ?f])]` returned `:e1` and `[:find ?e :where [?e :x ?f] (not-join [?f] [?g :y ?f] [(> ?f 0.0)])]` returned nothing. Every path now matches the body from the outer values, compared by join equality, so both return `:e1`; `0.0` and `-0.0` are equal and NaN equals nothing. A literal constant in the body still matches floats within `f64::EPSILON`.
- **An `or` / `or-join` gives the same answer on every executor path** (#546). The executor evaluated `or` branches on two paths, and their answers differed. The joined path, taken unless a branch held a `not`, a pseudo-attribute reading an outer pattern's metadata or an expression reading an outer variable, ran each branch alone and joined its rows to the outer rows. The per-row path started each branch from the outer row. Three cases changed, all to the per-row answer. (1) The joined path compared values exactly, so a keyword never met a ref to the entity it names: with `[:alice :friend :bob] [:bob :name "B"]`, `[:find ?e ?n :where [?e :friend ?f] (or [?f :name ?n] [?f :alias ?n])]` returned nothing and now returns `:alice "B"`; floats compare by IEEE `==`. (2) It joined every branch's rows on the outer variables any branch names, so `(or [?e :a ?x] [?f :b ?x])` matched nothing; each branch now meets the outer row on the variables it names. (3) It dropped the rows of an `or-join` branch that does not bind every join variable; such a branch constrains nothing about that variable, so `(or-join [?e] [?x :a 1] [?e :b 2])` keeps every outer row when some entity has `:a 1`. Rows an `or` returns are still distinct. Also, with no `:valid-at`, `:db/valid-at` in a branch read the wall clock instead of the query's "now"; it now reads the query's.
Expand Down Expand Up @@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **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.
- **A recursive rule query no longer joins a rule's new facts with every magic-set fact** (#694). On the engine (#548), each round after the first runs a recursive rule from the facts the last round derived, then joins the rest of its body. The rest kept the order the planner chose for the whole body, so the next clause could share no variable with the new facts: after the magic-set rewrite of `(reach :n0 ?to)`, the body `(magic ?from) [?from :next ?mid] (reach ?mid ?to)` joined each new `reach` fact with every `magic` fact, then probed `:next` once per pair. The rest of the body is now planned with the new facts' variables bound, each next pattern sharing a variable with those bound before it when one can. Answers are unchanged. Nightly `recursion` benchmarks (bench profile): `fanout/w10_d3` 4.94 s → 22.6 ms (it was about 94 ms on the executor before #548), `chain/depth_100` 699 ms → 41.5 ms, `chain/depth_10` 1.15 ms → 0.68 ms.
- **`BrowserDb` errors carry their error code** (#673). The browser API (`@minigraf/browser`) rejected with the error's top-level message only, such as `Unknown command: nosuchcommand`, while every other API reports `[PRS-001] Unknown command: nosuchcommand`. Every `BrowserDb` rejection now goes through `MinigrafError`, so it starts with its code. Host errors that have no code yet (no IndexedDB, an import blob whose length is not a multiple of the page size) are unchanged.
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**1771 tests** (1757 passing, 14 ignored; unit + integration + doc).
**1772 tests** (1758 passing, 14 ignored; unit + integration + doc).
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
6 changes: 3 additions & 3 deletions docs/TEST_COVERAGE.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/query/datalog/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl PatternMatcher {

EdnValue::Float(f) => {
if let Value::Float(ff) = fact_value {
(f - ff).abs() < f64::EPSILON
f == ff
} else {
false
}
Expand Down
4 changes: 2 additions & 2 deletions src/query/engine/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub(crate) enum ValueConst {
Keyword(String, EntityId),
/// A UUID: a ref to it, or a keyword that names it.
Uuid(EntityId),
/// A float: floats within `f64::EPSILON`.
/// A float: floats equal by IEEE `==`, so `0.0` matches `-0.0` (#706).
Float(f64),
}

Expand All @@ -73,7 +73,7 @@ impl ValueConst {
u == fu
}
(ValueConst::Uuid(u), Value::Keyword(k)) => names(k, u),
(ValueConst::Float(f), Value::Float(ff)) => (f - ff).abs() < f64::EPSILON,
(ValueConst::Float(f), Value::Float(ff)) => f == ff,
_ => false,
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/query/engine/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl KeyTest {
m
}
},
(KeyTest::Float(f), KeyValue::Float(ff)) => (*f - ff).abs() < f64::EPSILON,
(KeyTest::Float(f), KeyValue::Float(ff)) => f == ff,
_ => false,
})
}
Expand Down Expand Up @@ -376,7 +376,7 @@ fn cell_matches(c: &ValueConst, cell: &Cell, ctx: &mut Ctx<'_>) -> Result<bool>
ctx.ids.uuid(ctx.keys, *e)? == *u
}
(ValueConst::Uuid(u), Cell::Iid(i)) => names(&ctx.ids.name(ctx.keys, *i)?, u),
(ValueConst::Float(f), Cell::Val(Scalar::Float(ff))) => (f - ff).abs() < f64::EPSILON,
(ValueConst::Float(f), Cell::Val(Scalar::Float(ff))) => f == ff,
_ => false,
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/query/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ fn queries(tx: u64) -> Vec<String> {
"[?e :person/name \"no such name\"]",
// Cross-kind constants (#544 PR 2): a keyword matches the keyword and
// a ref to the entity it names; a UUID matches the ref and keywords
// that name it; floats match within an epsilon.
// that name it; floats match by IEEE `==` (#706).
"[?e :person/status :s1]",
"[?e :person/friend :p2]",
"[?e :person/boss :p2]",
Expand Down
6 changes: 3 additions & 3 deletions src/storage/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,9 @@ pub fn attribute_value_prefix(a: u32, v: &KeyValue) -> Vec<u8> {
k
}

/// A kind of value whose matches a constant cannot name by one key: floats
/// match within an epsilon, keywords match a UUID constant by the entity
/// they name (#544).
/// A kind of value whose matches a constant cannot name by one key: a float
/// matches `0.0` and `-0.0` alike (#706), keywords match a UUID constant by
/// the entity they name (#544).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ValueKind {
Float,
Expand Down
77 changes: 72 additions & 5 deletions tests/join_rules_test.rs
10BC8
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! keyword and a ref returns the kind from the first pattern, in `:where`
//! text order, that binds it.

use minigraf::{Minigraf, QueryResult, Value};
use minigraf::{BindValue, Minigraf, QueryResult, Value};

fn rows(r: QueryResult) -> Vec<Vec<Value>> {
match r {
Expand Down Expand Up @@ -43,12 +43,79 @@ fn float_join_pairs_zero_with_negative_zero() {
assert_eq!(r.len(), 1, "a's 0.0 and -0.0 agree");
}

/// `1e-17`, `0.0` and `-0.0`: the first is within `f64::EPSILON` of the
/// others but equal to neither (#706).
const SMALL_FLOATS: &str =
"(transact [[:tiny :m/x 0.00000000000000001] [:zero :m/x 0.0] [:neg :m/x -0.0]])";

/// The keywords naming the entities in `r`'s first column, sorted.
fn names_of(r: &[Vec<Value>]) -> Vec<&'static str> {
let mut n: Vec<&str> = r
.iter()
.map(|row| {
[":tiny", ":zero", ":neg"]
.into_iter()
.find(|k| is_ref_to(&row[0], k))
.unwrap_or("?")
})
.collect();
n.sort();
n
}

/// A float constant written in a pattern, bound to a `$slot`, written in a
/// rule body, or compared by a predicate matches equal floats only (#706).
fn assert_float_constants_exact(db: &Minigraf) {
db.execute("(rule [(zeroish ?e) [?e :m/x 0.0]])").unwrap();
db.execute("(rule [(xval ?e ?v) [?e :m/x ?v]])").unwrap();
for q in [
"[:find ?e :where [?e :m/x 0.0]]",
"[:find ?e :where [?e :m/x -0.0]]",
"[:find ?e :where [?e :m/x ?v] [(= ?v 0.0)]]",
"[:find ?e :where (zeroish ?e)]",
"[:find ?e :where (xval ?e 0.0)]",
] {
assert_eq!(names_of(&query(db, q)), [":neg", ":zero"], "{q}");
}
let r = query(db, "[:find ?e :where (xval ?e 0.00000000000000001)]");
assert_eq!(names_of(&r), [":tiny"], "a rule call's float argument");
let r = query(db, "[:find ?e :where [?e :m/x 0.00000000000000001]]");
assert_eq!(names_of(&r), [":tiny"], "1e-17 matches itself only");
let r = query(db, "[:find ?e :where [?e :m/x 0.00000000000000002]]");
assert!(r.is_empty(), "2e-17 matches nothing");

let p = db
.prepare("(query [:find ?e :where [?e :m/x $v]])")
.unwrap();
for (v, want) in [(0.0, &[":neg", ":zero"][..]), (1e-17, &[":tiny"][..])] {
let r = rows(
p.execute(&[("v", BindValue::Val(Value::Float(v)))])
.unwrap(),
);
assert_eq!(names_of(&r), want, "bound {v}");
}
}

#[test]
fn float_constant_keeps_epsilon_match() {
fn float_constant_matches_equal_floats_only() {
let db = Minigraf::in_memory().unwrap();
db.execute("(transact [[:a :x 1.0]])").unwrap();
let r = query(&db, "[:find ?e :where [?e :x 1.0000000000000001]]");
assert_eq!(r.len(), 1, "a float constant matches within epsilon");
db.execute(SMALL_FLOATS).unwrap();
assert_float_constants_exact(&db);
}

#[cfg(not(target_arch = "wasm32"))]
#[cfg(not(target_arch = "wasm32"))]
#[test]
fn float_constant_matches_equal_floats_only_after_checkpoint() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("floats.graph");
let db = Minigraf::open(&path).unwrap();
db.execute(SMALL_FLOATS).unwrap();
assert_float_constants_exact(&db);
db.checkpoint().unwrap();
assert_float_constants_exact(&db);
drop(db);
assert_float_constants_exact(&Minigraf::open(&path).unwrap());
}

fn friends_db() -> Minigraf {
Expand Down
12 changes: 7 additions & 5 deletions tests/negation_equality_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,16 @@ fn imported_keyword_equals_ref_to_its_entity() {
}

#[test]
fn body_float_constant_keeps_epsilon_match() {
fn body_float_constant_matches_equal_floats_only() {
let db = Minigraf::in_memory().unwrap();
db.execute("(transact [[:a :x 1.0] [:b :x 2.0] [:a :y 1.0]])")
db.execute("(transact [[:a :x 1.0] [:b :x 2.0] [:a :y 0.0] [:b :y 0.00000000000000001]])")
.unwrap();
// A literal in the body still matches within `f64::EPSILON`.
// A literal in the body matches equal floats only (#706): `1e-17` is
// within `f64::EPSILON` of `0.0` but not equal to it.
for q in [
"[:find ?e :where [?e :x _] (not [?e :y 1.0000000000000001])]",
"[:find ?e :where [?e :x ?v] (not [?e :y 1.0000000000000001] [(> ?v 0.0)])]",
"[:find ?e :where [?e :x _] (not [?e :y 0.0])]",
"[:find ?e :where [?e :x ?v] (not [?e :y 0.0] [(> ?v 0.0)])]",
"[:find ?e :where [?e :x _] (not [?e :y -0.0])]",
] {
assert_eq!(entities(&db, q, &[":a", ":b"]), vec![":b"], "{q}");
}
Expand Down
Loading
0