fix(query): STR() on a double returns the canonical XSD lexical form - #1750
Conversation
STR() routed doubles through Rust's Display, so `STR(?d)` returned "1" for the same term the serializer rendered "1.0E0" two fields over in the same result set — and "inf" for a value no format spells that way. #1445 fixed the six RDF output sites by adding `canonical_xsd_double`; expression evaluation was never on that list. Route `into_string_value`'s Double, Float and TypedLiteral{Double} arms through the serializer's own routine rather than deriving a second one: `fluree-db-query` picks up a dependency on `fluree-graph-ir`, where the helper already lives. Floats need an f32 mantissa (widening 33.33f32 to f64 spells it 3.3329998016357422E1), so `canonical_xsd_float` joins it there, sharing the one formatter; GROUP_CONCAT's numeric coercion takes the same form. The xsd:string() cast deliberately does NOT follow. SPARQL §17.5 defers casting to XPath, whose double->string rule is plain decimal notation in [1e-6, 1e6), and W3C cast-string requires xsd:string("1E0"^^xsd:double) to be "1". It kept falling through to into_string_value, so it now has explicit arms — including the XSD spellings for NaN/INF, which were leaking Rust's "inf" under every reading of the spec.
bplatz
left a comment
There was a problem hiding this comment.
Approving — the double side is solid. I reproduced the gates locally (graph-ir + fluree-db-query --lib green, W3C 36/0, clippy/fmt clean) and the non-vacuity claim: both sparql_str_* tests go red on revert. Keeping the cast separate is the right call and well evidenced.
Please look at the inline points before merging, though — the float sibling isn't actually closed. STR() still disagrees with the serializer for any stored xsd:float that isn't f32-exact, and the new float test can't catch it. Some of the rest are probably follow-up issues rather than changes here; fine either way as long as they get filed.
| ))), | ||
| ComparableValue::Float(f) => Some(ComparableValue::String(Arc::from( | ||
| super::cast::format_f32(f), | ||
| fluree_graph_ir::canonical_xsd_float(f), |
There was a problem hiding this comment.
STR() on a stored xsd:float still diverges from the serializer. Floats are stored as FlakeValue::Double(f64) and ingest never narrows, so "3.14159265358979"^^xsd:float serializes as 3.14159265358979E0 while STR() returns 3.1415927E0 — coerce_numeric_operand's d as f32 truncates first. GROUP_CONCAT keeps the f64 and agrees with the serializer, so one term now has three renderings.
Simplest close: don't downcast on this path. canonical_xsd_double on the stored f64 matches the serializer exactly, and the widening artifact the new doc warns about can't arise here — the f64 came from parsing the lexical, not from widening an f32. The conflation underneath (serializers key on the Double variant, not the declared datatype) looks like follow-up issue material.
There was a problem hiding this comment.
Right on the defect — and one refinement on the mechanism, because it ended up shaping the fix. For a stored float the truncation isn't in coerce_numeric_operand (that path only handles the string-backed cast/STRDT literals); it's at binding→ComparableValue conversion itself — lit_to_comparable (eval.rs) and the #1470 EncodedLit re-tag. Which means "don't downcast on this path" can't be done inside into_string_value: by the time the Float arm runs the f64 is gone, and widening it back is exactly the artifact case. And the general arm genuinely can't call canonical_xsd_double either — your "the f64 came from parsing the lexical" guarantee holds for a stored term, but a float computed inside the expression reaches the same arm as a widened f32, where 33.33f32 * 1 would spell itself 3.3330001831054688E1.
So the shape that landed: eval_str reads the stored f64 straight off the binding for the bare-variable shape (stored_float_f64 in eval.rs, covering both the Lit and EncodedLit lanes — same pattern as eval_lang's existing binding fast path), and renders canonical_xsd_double; computed floats keep canonical_xsd_float. Verified live: stored "3.14159265358979"^^xsd:float — serializer 3.14159265358979E0, STR() was 3.1415927E0, now equal, on both lanes.
One wrinkle your GROUP_CONCAT sub-claim surfaced when I ran it: it agrees with the serializer on the plain lane (GROUP BY ?s → agg_group_concat's Double arm → 3.14159265358979E0), but GROUP_CONCAT(?f) … GROUP BY ?f gives a third spelling, 3.1415927 — the group-key lane round-trips the aggregate input through expression eval → to_binding → format_f32. That one, plus STR(COALESCE(?f)) (the inner expression evaluates to an f32 before STR() can see the binding) and the serializer-precision question itself, are the umbrella you called: filed as #1776.
|
|
||
| let cases = [ | ||
| ("ex:f1", "1E0", "1.0E0"), | ||
| ("ex:f2", "33.33", "3.333E1"), |
There was a problem hiding this comment.
All three cases are f32-exact, so str_lex == serialized can't fail here. 3.14159265358979 or 1.23456789012345E-5 breaks it (see the note on eval/value.rs). Worth one f32-inexact case whichever way the fix goes.
There was a problem hiding this comment.
Confirmed exactly as you read it — with the fix reverted, the original three rows all stay green (they pass under either implementation) and only an inexact row discriminates. Added both of your candidates (3.14159265358979, 1.23456789012345E-5); red without the fix (left: "3.1415927E0" / right: "3.14159265358979E0"), green with it. Also added the same inexact value on the EncodedLit lane (sparql_encoded_xsd_float_str_matches_serializer, reindexed ledger, next to the #1470 re-tag tests) since the memory-ledger tests never touch that lane — and proved it actually fires by disabling only the encoded arm of stored_float_f64: the encoded test goes red with the same truncation while the Lit-lane test stays green.
| // (`format_f32`) rather than STR()'s canonical form — `STR()` of | ||
| // the result still agrees with how that result serializes, which | ||
| // is the invariant #1695 is about. | ||
| ComparableValue::Float(f) => Ok(Binding::lit( |
There was a problem hiding this comment.
This builds a term's lexical form rather than a cast result, so canonical_xsd_float is arguably what belongs here: BIND(?f + 0 AS ?y) over a stored 1.0E0 float yields "1"^^xsd:float, so the same value spells two ways depending on provenance. STR() does agree with the serializer per-term, but on a non-canonical spelling — the class of thing #1445/#1695 exist to remove. Fine to keep deliberately; the comment should then say that.
There was a problem hiding this comment.
Reproduced as described: stored "1.0E0"^^xsd:float serializes 1.0E0, BIND(?f + 0 AS ?y) yields "1"^^xsd:float. Two spellings by provenance, exactly.
I did try the switch before deciding, and it breaks something concrete: a minted float binding is a string-backed literal, and xsd:string(?y) reads that lexical verbatim (the TypedLiteral{String} fall-through in cast_to_string). Today that's "1" — which is what XPath requires for the cast, verified at head — so minting "1.0E0" instead makes the rebound cast wrong, and fixing that would mean teaching cast_to_string to parse float-typed strings back out, i.e. the split just moves. So: each minted term is internally consistent (serializer, STR(), and xsd:string() of it all return the one string), the split is between two terms rather than two renderings of one term — but you're right that the value-level split is the #1445/#1695 class. Kept deliberately, and the comment now says exactly that, with the rebound-cast reasoning inline. Cross-provenance canonicalization rides with the serializer-precision decision in #1776 — if we ever narrow the serializer to f32 by datatype, this arm collapses into it for free.
| /// take their XSD spellings — `f64::to_string()` alone yields `"inf"`, which | ||
| /// is not a valid lexical form under either the cast rules or the canonical | ||
| /// ones (#1695). | ||
| fn format_f64(d: f64) -> String { |
There was a problem hiding this comment.
On the range gap you flagged in the description: it's two-sided. xsd:string(1.0E-7) returns 0.0000001 where XPath wants 1.0E-7, same rule as the >= 1e6 side. Worth filing — mention both bounds.
There was a problem hiding this comment.
Verified both bounds by execution at head: xsd:string(1.0E-7) → 0.0000001, xsd:string(1.0E30) → the 31-digit integer. Two-sided, as you said.
Folded rather than filed, though — the "implementing XPath's algorithm" cost I punted on in the description had quietly collapsed once canonical_xsd_double/canonical_xsd_float existed: outside [1e-6, 1e6) XPath prescribes the canonical lexical representation, which is precisely those writers, so the whole fix is a range check in format_f32/format_f64. Unit pins on the bounds (1e-6 inclusive stays decimal, 1e6 exclusive goes scientific, zero stays "0"), integration cast columns pin 1.0E-7/1.0E6/1.0E30, W3C still 36/0 — nothing in the corpus reaches either side. PR body updated to match.
| // and pick up STR()'s canonical form, failing W3C `cast-string`. | ||
| match &v { | ||
| ComparableValue::Double(d) | ||
| | ComparableValue::TypedLiteral { |
There was a problem hiding this comment.
Nit: this arm ignores dtc, so a float arriving as TypedLiteral { val: Double, .. } takes the f64 branch while Float takes f32. Doesn't reproduce today (floats reach the cast as Float), but it's the same variant-vs-datatype conflation as the STR() issue.
There was a problem hiding this comment.
Confirmed — and it's unreachable by construction, not just unreproduced: I went through every TypedLiteral producer, and none wraps a Double (STRDT/STRLANG/casts wrap strings, TryFrom<FlakeValue> maps Double to the bare variant, unwrap_typed_literal extracts it back out, the temporal family is the only non-string wrap). Execution agrees — both float shapes take other arms (stored → Float → 3.1415927, STRDT-minted → string-backed → verbatim). Left the arm as belt-and-braces and added the guarding comment naming the variant-vs-datatype conflation and pointing at #1776, so if a producer ever appears the arm's obligation to split on the datatype is written down where it'll be found.
| /// Callers must exclude NaN/±INF first — their `{:e}` forms carry no | ||
| /// exponent, and their XSD spellings (`NaN`, `INF`, `-INF`) differ from | ||
| /// Rust's. | ||
| fn finite_canonical<F: fmt::LowerExp>(d: F) -> StackBuf { |
There was a problem hiding this comment.
Nit: debug_assert!(d.is_finite()) went away with the generic, so the contract is doc-only now. Both public wrappers guard, so nothing is broken — a sealed trait, or an assert at the two call sites, restores the machine check.
There was a problem hiding this comment.
Took the sealed-trait shape — it turned out to pay for itself twice: XsdFloat (private sealed module, f32/f64 only) hands finite_canonical its debug_assert! back, and it's the same trait the NaN/INF dedupe from your other note hangs off, so it isn't speculative machinery. Call-site asserts would've been vacuous here anyway — each wrapper branches on the specials one line above the call.
| /// double-precision widening artifacts (`33.33f32` stays `3.333E1`, not | ||
| /// `3.333000183105469E1`). | ||
| #[must_use] | ||
| pub fn canonical_xsd_float(f: f32) -> String { |
There was a problem hiding this comment.
Nit: the NaN/INF spelling is now written four times (here, canonical_xsd_double, format_f32, format_f64). One helper would do.
There was a problem hiding this comment.
Done — XsdFloat::nonfinite_xsd(), a default method on the sealed trait returning Option<&'static str>, so the spelling exists exactly once and costs nothing. Wired through all four formatters here plus format_f32/format_f64 in eval/cast.rs — your four sites turned out to be six once I counted the push/write variants, all on the one helper now.
The generic rewrite of finite_canonical dropped the concrete version's debug_assert!(is_finite) — the finite-input contract was doc-only. A small sealed trait (f32/f64) restores the machine check and, while it is there, becomes the single home of the NaN/INF/-INF spelling that was written out in all four public formatters. Zero-cost: static strings, default method, no allocation.
…st range gap was two-sided A stored xsd:float is a full-precision FlakeValue::Double; the serializer prints that f64, but expression eval narrows the binding to an f32 (lit_to_comparable / the EncodedLit re-tag), so STR() on an f32-inexact value spelled the truncation: 3.1415927E0 against the serializer's 3.14159265358979E0 for the same term. The Float ComparableValue cannot recover the f64, so eval_str reads it off the binding itself (stored_float_f64, both the Lit and EncodedLit lanes) and renders canonical_xsd_double — exactly the serializer's rendering, and no widening artifact can arise there because the f64 was parsed from the lexical. A float computed inside an expression is a genuine f32 and keeps canonical_xsd_float. The existing float test rows were all f32-exact and could not catch this; the new rows are inexact on purpose, with the EncodedLit lane pinned on a reindexed ledger and the JSON-LD (str ?f) twin alongside. The xsd:string() cast gap flagged in the PR description turned out to be two-sided and cheap to close now that the canonical writers exist: XPath casts through xs:decimal only inside [1e-6, 1e6) and prescribes the canonical lexical form outside it, on both sides — xsd:string(1.0E-7) is 1.0E-7, not 0.0000001, just as xsd:string(1.0E30) is 1.0E30, not a 31-digit integer. format_f32/format_f64 now apply the range; the cast-column pins state it. The variant-vs-datatype conflation underneath (serializers keying floats on the Double variant, the group-key GROUP_CONCAT lane, STR through COALESCE/IF) is #1776; the unreachable TypedLiteral{Double} cast arm carries a guarding comment pointing there, and the stored-vs-computed provenance split at to_binding is documented as deliberate. Follow-up: #1776.
Fixes #1695.
STR()on anxsd:doublewas routing through Rust'sDisplay, soSTR(?d)came back"1"for the same term the serializer printed as"1.0E0"two fields over in the same result set — and"inf"for a value that no RDF format spells that way. #1445 fixed this for the six RDF output sites by addingcanonical_xsd_double; expression evaluation was simply never on that list, so we've had two renderings of one concept since then.The mechanism is the one the issue traced:
ComparableValue::into_string_value(fluree-db-query/src/eval/value.rs) formatted theDoublearm withd.to_string(). Rather than derive a second formatter next to it,fluree-db-querypicks up a dependency onfluree-graph-irand calls the serializer's owncanonical_xsd_double— the same functionexport.rs,format/sparql.rs,format/sparql_xml.rsandformat/delimited.rsalready call. TheTypedLiteral { val: FlakeValue::Double, .. }arm the issue flagged takes it too.Two siblings turned out to be the same defect and are folded in:
STR()on anxsd:float. There are two layers here. The visible one is the same as the double's: the serializer gave1.0E0whereSTR()gave1. The subtle one is that a storedxsd:floatis carried as a full-precisionFlakeValue::Double(ingest never narrows), and expression evaluation narrows the binding to an f32 on the way intoComparableValue(lit_to_comparable, and theEncodedLitre-tag from query: xsd:float first-class on the EncodedLit (binary-store) path #1470) — which is right for arithmetic anddatatype(), and irreversible for lexical purposes. For an f32-inexact value like"3.14159265358979"^^xsd:floatthe serializer prints the stored f64 (3.14159265358979E0), so no rendering of the f32 can ever agree with it.STR()therefore reads the stored f64 straight off the binding (stored_float_f64ineval.rs, covering both theLitand the late-materializedEncodedLitlanes) and renderscanonical_xsd_double— exactly the serializer's string, and no widening artifact can arise there because that f64 was parsed from the lexical, never widened from an f32. A float computed inside an expression is a genuine f32 and renderscanonical_xsd_float, which joins its sibling influree-graph-ir/src/xsd_double.rs— both share the onefinite_canonicalwriter behind a small sealedXsdFloattrait that keeps the finite-input contract machine-checked (debug_assert!) and theNaN/INF/-INFspellings written exactly once, for the cast renderers too.GROUP_CONCAT's numeric coercion (aggregate.rs), which stringified doubles the same way —GROUP_CONCAT(?d)produced1|1000000where the serializer renders those same two terms1.0E0and1.0E6.One sibling I deliberately did not fold in, and I think this is the more interesting part of the PR. My first pass also routed the
xsd:string()cast through the canonical form, on the theory that it was the same seam. The W3C suite said otherwise, and it's right: SPARQL §17.5 defers casting to XPath, whose float/double→string rule casts throughxs:decimal— plain decimal notation — for absolute values in[1e-6, 1e6), anddata-sparql11/cast/cast-string.srxrequiresxsd:string("1E0"^^xsd:double)to be"1", not"1.0E0". So the cast andSTR()are answering genuinely different questions and need to stay apart. Sincecast_to_stringwas reaching the canonical form only by falling through tointo_string_value, it now has explicitDouble/Floatarms of its own (eval/cast.rs) — which also picks up the one part of the cast that was broken under every reading of the spec: it was emitting Rust's"inf"/"-inf"rather thanINF/-INF.But the XPath range rule is exactly that — a range, and it is two-sided: outside
[1e-6, 1e6)XPath prescribes the canonical lexical representation, which with the canonical writers in hand is a two-line check rather than "implementing XPath's algorithm". Soformat_f32/format_f64apply it:xsd:string(1.0E-7)is1.0E-7(not0.0000001), just asxsd:string(1.0E30)is1.0E30(not a 31-digit integer). Nothing in the W3C corpus moves in either direction — the suite is 36/0 on both sides of the change; the cast columns in the tests pin both bounds.The invariant that came out of all this, and the one the tests actually assert, is
STR(?x)equals what we serialize for?x— checked per-row against the serialized value of the same binding rather than against a hardcoded string, so the two paths can't drift apart again without a red test.What stays open, deliberately, in #1776: the conflation underneath all three symptoms — float renderers keying on the
Doublevariant rather than the declared datatype. Concretely: whether the serializer should print anxsd:floatterm at f32 precision at all (a user-visible output decision, same shape as #1445's); the group-keyGROUP_CONCATlane, whose aggregate input round-trips throughto_bindingand mints a third spelling;STR(COALESCE(?f)), which evaluates the inner expression to an f32 beforeSTR()can see the binding; and the stored-vs-computed provenance split atto_binding(a computed float mints its XPath-cast lexical — each such term is internally consistent across serializer/STR/cast, and switching it to the canonical spelling would breakxsd:string()on the rebound term, so it stays, documented at the site). The unreachableTypedLiteral { val: Double, .. }cast arm carries a guarding comment pointing at the same issue.Follow-up: #1776.
Tests
sparql_str_double_matches_serializer_canonical_formsits right beside the existing serializer pinsparql_double_canonical_lexical_form_across_formats, so the two can't drift. It covers the issue's repro (1E0), an integral value, both exponent directions, a negative,NaN/INF/-INF— and pins the cast column alongside on both sides of the XPath range (1.0E-7,1.0E6,1.0E30), precisely so the deliberate relationship between the two functions is stated rather than left for someone to "fix" later.sparql_str_float_matches_serializer_canonical_formincludes f32-INEXACT rows (3.14159265358979,1.23456789012345E-5) — the discriminating cases: an f32-exact row passes under any implementation, an inexact one only ifSTR()reads the stored f64.sparql_encoded_xsd_float_str_matches_serializerpins the same agreement on the late-materializedEncodedLitlane (reindexed ledger), next to the query: xsd:float first-class on the EncodedLit (binary-store) path #1470 re-tag tests it extends.sparql_group_concat_double_canonical_formfor the aggregate sibling.jsonld_bind_str_double_canonical_formandjsonld_bind_str_float_canonical_formare the JSON-LD twins per the parity rule —(str ?d)gives the same canonical string while the value itself stays a native JSON number.into_string_valueandcanonical_xsd_floatinfluree-graph-ir, plusformat_f32/format_f64range pins (format_f64_xpath_range_is_two_sided) including the inclusive/exclusive bounds and zero.Non-vacuity checked by revert, one lever at a time: with the
eval_strfast path disabled, the inexact float rows and both float twins go red (left: "3.1415927E0"/right: "3.14159265358979E0") while the f32-exact rows stay green; with only theEncodedLitarm ofstored_float_f64disabled, the encoded-lane test goes red with the same truncation while theLit-lane test stays green — proving that lane actually fires; with the cast range check disabled, the unit and integration cast pins go red (left: "0.0000001"). All green on restore.Gates
-p fluree-db-query(1488 lib, all green),-p fluree-graph-ir(51),-p fluree-db-apigrp_query_sparql(372) andgrp_query(425), the W3C suite (36 passed; 0 failed), workspace check (minus the excluded search crates), clippy-D warningsclean on all three touched crates,cargo fmt --alllast andtestsuite-sparql's own fmt clean. No register movement in either direction —testsuite-sparql/tests/registers/mod.rsis untouched, which is the honest answer here: the cast register never moved because I corrected the scope rather than registering a regression.