Repository navigation
Conversation
…pace `parse_xsd_double` and `parse_xsd_float` accept exactly the XSD 1.1 lexical forms: numerals, `INF`, `+INF`, `-INF` and `NaN`. A numeral too large in magnitude for the datatype maps to `INF` or `-INF`, as XSD 1.1 specifies. Rust's spellings (`inf`, `Infinity`, `nan`) are not lexical forms of either datatype. Every place that turns a double or float literal into a value uses it: JSON-LD coercion, SPARQL query literals and casts, SPARQL UPDATE, Turtle, and string-backed aggregate inputs. JSON-LD and SPARQL UPDATE refuse the other spellings as ill-typed literals, the rule other built-in datatypes follow (#1988), and Turtle keeps them as ill-typed literals. SPARQL UPDATE words the refusal as JSON-LD does (`Parse error: ...`). R2RML renders non-finite float cells with the XSD spellings. Cypher's `toFloat` is not an XSD cast. It gets its own function and reads strings as Cypher does, `Infinity`, `-Infinity` and `NaN` included.
`ObjKey::encode_f64` is total. `-INF` sorts below every finite value, `INF` above, and NaN, canonicalized to one quiet NaN, above `INF`. The keys extend the existing order inside `NUM_F64`, so the index format and its version are unchanged and existing index files need no migration. `FlakeValue`'s `Ord` and the index keys share this one total order, so novelty and the index sort doubles alike. The overlay, scan and bound lookup paths encode the special values, and bulk import stores them as numbers, as transactions do. Numeric range walks (the COUNT, top-k and range-semijoin fast paths) stop at `INF`'s key, since no comparison selects NaN. `ObjKeyError` loses its `NaN` and `Infinite` variants.
Storage order is total, but SPARQL comparison is XPath's (SPARQL 1.1 §17.3, F&O 3.1 §4.3): `=`, `<`, `>`, `<=` and `>=` are false when an operand is NaN, and `!=` is true. The comparison is false, not a type error. `INF` is above and `-INF` below every other number. `sameTerm(NaN, NaN)` is true (§17.4.1.8). `SUM` and `AVG` with a NaN member are NaN, and `INF + -INF` is NaN (§18.5.1.3, §18.5.1.4), on the generic and the fused R2RML aggregate lanes alike. SHACL `sh:hasValue` and `sh:in` treat NaN as one term, and no range facet holds for NaN. GROUP BY, COUNT(DISTINCT), hash joins and the reasoner's derived-fact set key a double by its value, as `FlakeValue`'s equality and hash do: every NaN is one key, whatever its sign or payload, and -0.0 is 0.0.
A failure to resolve a commit's content into index records is now `IndexerError::Unindexable`. The same build would stop the same way, so the background indexer halts the ledger: waiters resolve as failed, index status reports `halted: true` with the error, and an incremental build does not fall back to a full rebuild for it. An implicit trigger (a commit, a push, a published commit, a catch-up sweep) starts no build for a halted ledger; its waiter resolves as failed with the same error. An explicit request (`IndexerHandle::trigger_explicit`, used by the admin index API), a reindex that succeeds, or a restart clears the halt. `IndexStatusSnapshot` and `IndexStatusResult` gain `halted`.
…lane - `it_xsd_double_special_values` (grp_index): values written through JSON-LD, SPARQL UPDATE, Turtle and bulk import read back from novelty, novelty over an index, an incremental build and a full rebuild, including after retraction; the SPARQL value semantics on both query surfaces and every lane; GROUP BY, COUNT(DISTINCT) and hash joins over NaNs of either sign and over -0.0; ordering and DISTINCT among integers beyond i64; one transaction holding every numeric type; copies through INSERT ... USING a named graph and retraction through DELETE ... WHERE (with WITH); non-XSD spellings per surface. - `it_xsd_double_fast_paths` (own binary): the COUNT-with-comparison, MIN/MAX and top-k fast paths agree with the general pipeline, with routing pinned. - `it_query_cypher`: `toFloat` reads `Infinity`, `-Infinity` and `NaN`.
Bulk import in earlier versions stored xsd:double and xsd:float INF and -INF in the index as the text `inf` and `-inf`. Arithmetic, SUM, AVG and comparisons inside expressions read that text, under those two datatypes, as INF and -INF, as they did before, so such an index reads as it did without a rebuild. Only reads take these two spellings; the write surfaces keep to the XSD lexical space. Tests: an index that v4.2.3's bulk import wrote (fluree-db-api/tests/fixtures/bulk-import-4.2.3, with its source Turtle), read on both query surfaces against v4.2.3's own answers; the same text written as Turtle, read from novelty and once indexed; unit tests at both read sites. docs/concepts/datatypes.md describes the exception.
A halted ledger records the nameservice `index_t` its failed build started from. The periodic re-sweep lifts the halt when the ledger's `index_t` has moved past it, as it does when another process indexes the ledger; implicit triggers then build it again. Tests: the re-sweep lifts a halt once a newer index is published, and not before; a reindex that succeeds lifts a halt (through a hidden test hook, `IndexerHandle::halt_for_test`).
`i64_fits_f64` compares the unsigned magnitude, so `Long(i64::MIN)` compares with any double by value, like every other integer beyond 2^53.
`FlakeValue` equality is by value across numeric types, so `Double(2^63)` equals `BigInt(2^63)`. Its hash took the integer path for doubles up to `i64::MAX as f64`, which rounds up to 2^63, and so hashed 2^63 as `i64::MAX`. The integer path now stops below 2^63, and 2^63 hashes as the exact decimal the `BigInt` arm uses. The test checks that equal values hash equally across `Long`, `Double`, `BigInt` and `Decimal`.
`LiteralValue`'s `Ord` compares doubles with `f64::total_cmp`. That order is total over every double, NaN of either sign included, and it is `Equal` exactly when the bits are, as `PartialEq` compares them, so sorting a graph's triples is well defined.
`IndexerHandle::cancel`, which every drop path calls, now also clears a halt and its error: it ends the ledger's indexing state, so a ledger created again under the same id starts without one and its commits index as usual. Maintenance holds (a reindex, an index sweep) cancel pending work but keep the halt, since the ledger goes on. The test halts a ledger, drops it, creates it again under the same id, commits, and expects the index to advance.
bplatz
left a comment
There was a problem hiding this comment.
Approving — the core fix is solid: finite keys are unchanged (no migration), NaN/-0.0 canonicalization is consistent across key, Ord, Hash, group/join keys, range walks stop at INF, and the NaN comparison split never leaks the storage order into SPARQL comparison. Please review the inline comments before merging; a few items couldn't be anchored to the diff, so they're here:
Must-fix (not in the diff)
- Cross-ledger shapes with
INF/-INFnow fail to translate — regression.fluree-db-api/src/cross_ledger/shapes_materializer.rs:297rendersFlakeValue::Double(f) => f.to_string(), which gives Rust'sinf/-inf. The receiving side goes throughvalue_convert::parse_xsd_lexical, which this PR switched to the strictparse_xsd_double, so a model ledger shape withsh:maxInclusive "INF"^^xsd:double(or-INFinsh:in) now errors withinvalid …#double lexical \inf`. It round-tripped before. One-line fix:fluree_graph_ir::canonical_xsd_double(*f)`. - Issue linking. The body should say
Fixes #2043, and the two Follow-ups (xsd:float single-precision rounding; MEDIAN/VARIANCE/STDDEV vs NaN) need issues filed andFollow-up: #Nlines (see CLAUDE.md /docs/contributing/issue-linking.md).
Should-fix
- #2043 asks for export coverage ("Export drops the triple"). The fix for that comes from removing the NULL sentinel in
dict_overlay/binary_scan, but no test exports NaN/INFfrom novelty and from the index. - The indexer halt (commits 4, 8, 12) is independent of the double fix — #2043 itself calls retry-forever "a separate problem". I'd be fine splitting it into its own PR; if it stays, see the inline comments on its test coverage and visibility.
Pre-existing, worth follow-up issues (not blockers)
graph_commit_builder.rs:112andvalidate.rs:769/778writenullforINF/NaN (commit detail flakes; SHACLsh:value).- Double division by zero errors;
op:numeric-dividesaysINF/-INF/NaN (andop:numeric-modNaN). xsd:boolean(NaN)is unbound; XPath casting givesfalse.- JSON-LD filter s-expression atoms still use
parse::<f64>()(parse/filter_sexpr.rs:230,465), so(> ?v inf)/(= ?v nan)are doubles — contradicts "one parser on every surface". coerce.rs:233:*d <= i64::MAX as f64admits 2^63 and saturates toi64::MAX(the same off-by-one commit 10 fixed inHash).- Stale comment at
binary_scan.rs:~1705saying NaN falls back to discriminant order inOrd.
| let (key, max) = match (otype, threshold) { | ||
| (OType::XSD_INTEGER, FlakeValue::Long(n)) => (ObjKey::encode_i64(*n).as_u64(), u64::MAX), | ||
| (OType::XSD_DOUBLE, FlakeValue::Long(n)) => ( | ||
| ObjKey::encode_f64(*n as f64).as_u64(), |
There was a problem hiding this comment.
*n as f64 rounds a Long threshold above 2^53, so this lane doesn't compare exactly the way the generic lane does (numeric_cmp goes through BigDecimal). Reproduced: stored "9007199254740992"^^xsd:double, SELECT (COUNT(?s) AS ?c) WHERE { ?s ex:v ?v FILTER(?v >= 9007199254740993) } → 1 with fast paths on, 0 with them off.
Pre-existing, but this function was rewritten here and the PR body says "Fluree compares numbers exactly". Fix: compute the boundary key from the exact comparison (e.g. nudge to the adjacent f64 when n as f64 rounds across n), or decline the fast path when n.unsigned_abs() > 2^53. fast_star_const_order_topk.rs:342 has the same *n as f64.
| /// Add one row's floating value to the accumulator. NaN and the infinities are | ||
| /// summed and counted like any other double, as the standard aggregate pipeline | ||
| /// does: SUM is `op:numeric-add` (SPARQL 1.1 §18.5.1.3), so a NaN member makes | ||
| /// the sum NaN. A physical exact or text column under a double datatype |
There was a problem hiding this comment.
"converts, as the generic path does" — the Column::String arm below (line 834) still uses t.trim().parse::<f64>(), which accepts Infinity, nan, +inf and whitespace. The generic lane now coerces strictly (coerce_value → parse_xsd_double), so a text cell Infinity gives SUM = INF here and unbound on the generic lane — a new disagreement between the two lanes (both accepted it before). Suggest using the same parser (plus legacy_import_infinity if you want parity with the index lane), without the trim. Separately (pre-existing), an unparseable cell is silently skipped here, whereas accumulate_exact_row escalates.
| return Ok(result); | ||
| } | ||
| // A full rebuild resolves the same commit and fails the same way. | ||
| Err(e) if !e.is_retryable() => return Err(e), |
There was a problem hiding this comment.
Nothing exercises a real Unindexable end to end. The orchestrator tests inject the error into on_build_error, and it_index_halt uses halt_for_test. If this arm, or the Resolve(Unindexable) → IndexerError::Unindexable mappings in build/incremental.rs (~730) and build/rebuild.rs (~365), regressed to a retryable error, every test would still pass and the ledger would go back to full-rebuilding forever. Could you add one test that commits genuinely unindexable data and asserts halted on both the incremental and the rebuild path, then mutation-prove it? Candidates: a vector dimension mismatch under one predicate (I couldn't find a write-side dims check — unverified), or a temporal/geo string that won't encode.
| return; | ||
| } | ||
| if !error.is_retryable() { | ||
| warn!( |
There was a problem hiding this comment.
A halt is easy to miss in operation. This one warn! is the only signal; later implicit triggers log at debug!, and halted only reaches the Rust IndexStatusResult — the server has no index-status route, the CLI doesn't show it, and Python's index_status exposes error but not halted. What an operator sees is novelty growing until reindex_max_bytes, then 503 NoveltyAtMax on every write. Suggest error! here, and ideally naming the halt in the backpressure error (or exposing it on a status endpoint). Server operators' only levers today are /reindex (which fails again on genuinely bad data) or a restart.
| /// implicit trigger (a commit, a push, a published commit, a catch-up | ||
| /// sweep) starts one. An explicit request | ||
| /// ([`IndexerHandle::trigger_explicit`]), a reindex that succeeds, a newer | ||
| /// index that any process publishes for the ledger, dropping the ledger |
There was a problem hiding this comment.
"a newer index that any process publishes for the ledger … clears it" isn't unconditional: clear_superseded_halts only runs inside the catch-up re-sweep (line 2743), which is skipped when catchup_sweeps_enabled = false (the Raft delegated worker, fluree-db-server/src/state.rs:409-425) or catchup_interval is zero. Either call it on those paths too or qualify the docs here, in admin.rs:298-303 and in the PR body.
Related edge case (suspected): a drop by another process doesn't clear this process's halt, since only a local cancel does; a ledger recreated under the same id elsewhere stays halted here until its index_t passes the old one.
| .map_err(|_| LowerError::invalid_decimal(value, datatype.span))?; | ||
| Ok(FlakeValue::Double(d)) | ||
| fluree_db_core::coerce::coerce_string_value(value, dt_iri.as_str()) | ||
| .map_err(|_| LowerError::invalid_decimal(value, datatype.span)) |
There was a problem hiding this comment.
Nit: a bad xsd:double/xsd:float literal now reports Invalid decimal literal 'inf'. Transactions say Cannot parse 'inf' as xsd:double. LowerError::invalid_literal(value, "xsd:double", …) would be accurate.
| @@ -12,6 +12,10 @@ | |||
| //! through these helpers so Fluree emits one consistent, spec-aligned form. | |||
| //! JSON-LD (and other JSON-native typed output) is deliberately excluded: | |||
| //! there a double is a native JSON number, never a lexical string. | |||
There was a problem hiding this comment.
Nit: stale — "there a double is a native JSON number, never a lexical string" contradicts the new docs: JSON-LD results carry the special values as the strings "INF"/"-INF"/"NaN".
| fully_indexed(fluree, ledger_id).await | ||
| } | ||
|
|
||
| async fn incrementally_indexed(fluree: &Fluree, ledger_id: &str) -> LedgerState { |
There was a problem hiding this comment.
The "incremental build" lane isn't pinned to the incremental path. build_and_publish_index goes through build_index_for_record, which falls back to a full rebuild on any retryable incremental failure (lib.rs:242-247), so this lane could silently become a second full-rebuild lane and still pass. Could you assert that incremental actually ran (tracing span or result stats)?
| } | ||
| } | ||
| let fired = proceeded.iter().filter(|s| *s == case.site).count(); | ||
| if !disabled && lane == "indexed" && fired != 2 { |
There was a problem hiding this comment.
Nit: routing is pinned on the indexed lane and with fast paths off, but not on "novelty over an index". If the overlay lane is expected to take (or decline) the fast path, pin that too, so it can't pass by silently taking the generic lane.
|
|
||
| /// Bulk import in earlier versions stored `xsd:double` and `xsd:float` `INF` | ||
| /// and `-INF` in the index as the text `inf` and `-inf`. The fixture is such | ||
| /// an index, written by v4.2.3's bulk import from `source.ttl` beside it. It |
There was a problem hiding this comment.
Nit: please record the exact v4.2.3 command(s) that produced tests/fixtures/bulk-import-4.2.3, so the fixture can be regenerated or extended.
Summary
xsd:doubleandxsd:floatinclude the special valuesINF,-INFandNaN(XSD 1.1 Part 2 §3.3.4, §3.3.5). With this change every double has an index key, the special values read back on every lane with SPARQL's value semantics, andxsd:double/xsd:floatlexical forms are parsed as XSD defines them.What changes
ObjKey::encode_f64gives every double a key. The keys extend the existing order insideNUM_F64:-INFbelow every finite value,INFabove, and one canonical NaN aboveINF. The index format and its version are unchanged, and existing index files need no migration.FlakeValue'sOrdand the index keys share that total order, so novelty and the index sort doubles alike. The overlay, scan and bound-lookup paths encode the special values, and bulk import stores them as numbers, as transactions do.FlakeValue::cmp, ORDER BY) is total, with NaN afterINFand all NaNs equal. SPARQL comparison follows XPath, under which NaN is unordered. Numeric range walks (the COUNT, top-k and range-semijoin fast paths) stop atINF's key, so no comparison selects a NaN.fluree_graph_ir::parse_xsd_double/parse_xsd_float) defines the lexical space: numerals,INF,+INF,-INFandNaN, with numerals beyond the range mapping toINF/-INFas XSD 1.1 specifies. Every write surface, query literals, casts and string-backed aggregate inputs use it. Rust's spellings (inf,Infinity,nan) are not values: JSON-LD and SPARQL UPDATE refuse them as ill-typed literals and Turtle keeps them as ill-typed literals, the rule other built-in datatypes follow (fix: keep typed literal values through the index (reindex affected ledgers) #1988). SPARQL UPDATE words that refusal as JSON-LD does (Parse error: Cannot parse 'inf' as xsd:double: …). R2RML renders non-finite float cells with the XSD spellings. Cypher'stoFloatis not an XSD cast and keeps reading strings as Cypher does,Infinity,-InfinityandNaNincluded.IndexerError::Unindexable. The same build would stop the same way, so the background indexer halts the ledger: waiters resolve as failed, index status reportshalted: truewith the error, and an incremental build does not fall back to a full rebuild for it. Implicit triggers (a commit, a push, a published commit, a catch-up sweep) start no build for a halted ledger. An explicit index request (IndexerHandle::trigger_explicit, used by the admin index API), a reindex that succeeds, a newer index that any process publishes for the ledger (the periodic re-sweep checks), or a restart lifts the halt. Dropping a ledger clears its halt, so a ledger created again under the same id starts without one.Semantics
Per the specs, except where marked:
=,<,>,<=and>=are false when an operand is NaN, and!=is true (SPARQL 1.1 §17.3; XPath F&O 3.1 §4.3). The comparison is false, not a type error, so!(?v < 0)holds for NaN.INFis above and-INFbelow every finite number of any numeric type (F&O 3.1 §4.3.2). Fluree compares numbers exactly, so an integer or decimal beyond the double range is still finite.sameTerm(NaN, NaN)is true (§17.4.1.8).SUMandAVGwith a NaN member are NaN, andINF + -INFis NaN (§18.5.1.3, §18.5.1.4).COUNTcounts NaN (§18.5.1.2).DISTINCT,GROUP BY,COUNT(DISTINCT)and hash joins treat every NaN as one value, whatever its sign or payload (§18.5, §18.5.1), and-0.0as0, on every read lane.sh:hasValueandsh:intreat NaN as one term; no range facet holds for NaN.INFin ascending order; SPARQL leaves its position open (§15.1). MIN and MAX follow ORDER BY (§18.5.1.5, §18.5.1.6), so MAX is NaN when a NaN is present and MIN is the smallest other value.docs/concepts/datatypes.mdgains a section with the same list.Compatibility
main;main;xsd:double/xsd:floatis read as a number only when it is an XSD lexical form. Such strings come from SPARQL UPDATExsd:floatliterals written before fix: keep typed literal values through the index (reindex affected ledgers) #1988; a non-XSD spelling among them now reads as the ill-typed literal it was stored as.xsd:double/xsd:floatvalues that a previous bulk import stored in the index as text (inf,-inf). Arithmetic,SUM,AVGand comparisons inside expressions read that text asINFand-INF; toisNumeric,ORDER BY,MIN/MAXand range filters it stays text, as before. Only reads take those two spellings; the write surfaces keep to the XSD lexical space. The special-value semantics above apply to every index alike.docs/concepts/datatypes.mddescribes the exception.ObjKey::encode_f64is infallible, andObjKeyErrorloses itsNaN/Infinitevariants.IndexerErrorgainsUnindexable;IndexStatusSnapshotandIndexStatusResultgainhalted;IndexerHandlegainstrigger_explicit,clear_haltand a hidden test hook,halt_for_test.FunctiongainsCypherToFloat.Performance
Measured against
mainat6b9d5b619(this branch as it stood on that base) on a numeric-heavy ledger: 1.5M triples (500k subjects, each with twoxsd:doublevalues and onexsd:integer), both CLIs built with thedev-fastprofile, runs interleaved, medians of 5 runs (15 for the last three reads, 7 for the novelty insert):mainreindex)1e5 < ?v < 1e10COUNTwith?v > 0over index + noveltyORDER BY ?vover 550k values, index + noveltySUM/AVGover index + noveltyEvery other measured read (fast-path COUNT, top-k, MIN/MAX, GROUP BY, bound lookup,
SUM/AVGover the index alone) is within ±2.5% at its final run count; two 5-run cells that read +7% and +8% were re-measured at 15 runs and did not reproduce. Query results are byte-identical between the two builds.Tests
it_xsd_double_special_values(grp_index): values written through JSON-LD, SPARQL UPDATE, Turtle and bulk import read back on every lane, including after retraction; one transaction holding every numeric type; the semantics table on SPARQL and its JSON-LD twin, on four read lanes (novelty, novelty over an index, incremental build, full rebuild); ordering, comparison,COUNT(DISTINCT), MIN/MAX andDATATYPEamong integers beyondi64;GROUP BY,COUNT(DISTINCT)and hash joins over NaNs of either sign and over-0.0, on both surfaces and every lane; values copied out of a named graph byINSERT … USINGand retracted throughDELETE … WHERE, includingWITHa named graph; non-XSD spellings per surface; casts.it_xsd_double_fast_paths(own binary): the COUNT-with-comparison, MIN/MAX and top-k fast paths with the kill switch on and off, routing pinned.it_query_cypher:toFloatreadsInfinity,-InfinityandNaN.FlakeValue, the reasoner's derived facts), SHACL value constraints, R2RML rendering, the fused aggregate and Cypher'stoFloat.values_a_previous_bulk_import_stored_as_text_read_as_before(grp_index): reads an index that v4.2.3's bulk import wrote (tests/fixtures/bulk-import-4.2.3, with its source Turtle) on both surfaces and expects v4.2.3's own answers.inf_text_reads_as_infinity_in_arithmetic_and_aggregates: the same text written as Turtle, from novelty and once indexed.it_index_halt(grp_index): a reindex that succeeds lifts a halt; a ledger dropped while halted and created again under the same id indexes its next commit. Orchestrator unit tests: implicit triggers leave a halted ledger alone, an explicit trigger builds it, a newer index lifts the halt, and cancelling a ledger clears its halt while a maintenance hold keeps it.Long,Double,BigIntandDecimal;Long(i64::MIN)compares with doubles; graph-ir literal order is total and agrees with equality.Commits
feat(graph-ir): one parser for thexsd:double/xsd:floatlexical space, used by every surface.fix(index): every double has an index key; one total storage order.fix(query): SPARQL comparison, sameTerm and aggregates for the special values; group and join keys by value.feat(indexer): a ledger whose committed data cannot be indexed is reported as halted; implicit triggers leave it halted.test(api): integration tests.docs(datatypes): the special values and their query semantics.fix(query):INFand-INFthat an earlier bulk import stored as text read as before.feat(indexer): a newer index lifts a halt.fix(core):Long(i64::MIN)compares with a double by magnitude.fix(core):Double(2^63)hashes like the integer it equals.fix(graph-ir): double literals order by IEEE total order.fix(indexer): dropping a ledger clears its halt.Follow-ups
xsd:floatvalues are stored as doubles without single-precision rounding, so"3.4e39"^^xsd:floatkeeps its double value instead of mapping toINF. Rounding every float literal changes stored values and equality, so it is left for a separate change.MEDIAN,VARIANCEandSTDDEVleave NaN out, whileSUMandAVGpropagate it. Making them consistent also needsMEDIAN's sort on the total order; left for a separate change.