fix(query): keep every string-dictionary datatype's own term identity

[aaj3f]

Fixes #1729

EncodedLit identifies a literal by (o_kind, o_key, dt_id, lang_id), and for anything in the string dictionary o_key is just the interned lexical form — so the datatype is the whole of the term's identity. Only xsd:string, rdf:langString and @fulltext have a reserved DatatypeDictId; every other string datatype is numbered per ledger. The decode arm at object_binding.rs:105 handed all of those xsd:string's id, which is not a near-miss but an outright merge: on an indexed ledger "abc", "abc"^^xsd:anyURI, "abc"^^xsd:token and "abc"^^ex:custom became one term.

Both lanes were wrong, in opposite directions

I put the four literals under one predicate and ran the same queries against novelty-only state and then against a published index. The indexed lane lost the datatype outright — DATATYPE(?o) reported xsd:string for all four, FILTER(?x = ?y) returned the full 4×4 cross product, and the self-join ?a ex:p ?o . ?b ex:p ?o returned (s1,s1) (s2,s1) (s3,s1) (s4,s1): each non-xsd:string literal's own identity row replaced by a pairing with the plain string. That last one is the shape that worries me most, because it is a wrong answer that still looks like a plausible answer.

The novelty lane was wrong too, which I didn't expect going in. There the bindings are Binding::Lit and carry their real datatype, so DATATYPE() and = were right — but is_string_term_constraint (binding.rs:264) recognised only xsd:string and a language tag, so a binding of any other string datatype carried no constraint into a join or OPTIONAL probe and matched the whole string family. Same self-join, 13 rows instead of 4. It is the same defect from the opposite side: one relation says these literals are distinct, the other says nothing at all about them.

Which is why both sites move together here. Fixing either alone leaves the two disagreeing, and I did revert each one independently to confirm each is load-bearing — with only the mirror fixed the indexed phase fails, with only the decode lane fixed the novelty phase fails.

The change

The decode arm now declines the datatypes it cannot name. That is the rule the temporal subtypes immediately below it already follow ("the other temporal subtypes stay materialized until their datatype ids are represented in EncodedLit"), and the row-at-a-time callers — binary_scan.rs, the join probe — already have a materialize-and-decode fallback for exactly this case, so those literals arrive as Binding::Lit carrying their exact datatype Sid. xsd:string, rdf:langString and @fulltext keep their encoded form untouched.

The cyclic-BGP operator needed more than the decline. Its EncObj::encode derived join identity from the materializer, so a declined datatype didn't fall back to a per-row decode there — it bailed the whole fast path (unsupported-object-binding), on the first such row in the scan loop: one xsd:anyURI value anywhere in an edge's predicate would have turned the operator off for that query. A cliff, and a silent one, since the fallback tree returns the same rows, slower. The cyclic data plane never needed the semantic datatype, so EncIdentity::Lit now keys on (o_type, o_key) instead of (o_kind, dt_id, lang_id, o_key). For everything the fast path carried before that is the same partition — o_type ↔ (o_kind, dt_id, lang_id) is a bijection across the encoded arms (integer/long, double/float and date/time/dateTime all have distinct reserved ids, JSON/vector/NumBig are one o_type each, and a langString's lang_id is the OType payload), and encoded_lit_identity_fields_are_injective_over_o_types walks all 65,536 o_types so a future arm that breaks the bijection fails a test instead of quietly splitting terms the rest of the engine unifies. For the datatypes the materializer declines, (o_type, o_key) is precisely the term identity this PR is about: o_key is the interned lexical form and o_type names the datatype — OType::customer_datatype(id) embeds the per-ledger id EncodedLit can't carry. The rows that survive the join decode at emit (EncObj::to_binding, through a novelty-aware graph view) into the same materialized Lit the fallback scan produces. That puts the string decode on output rows rather than scanned rows, which is the right side of a cyclic join to pay on — the operator exists because intermediates outnumber outputs.

The per-ledger id is safe as a join key there for a reason worth stating rather than assuming: the cyclic fast path never opens multi-ledger. open_fast_path (cyclic_bgp.rs) gates on allow_cursor_fast_path (fast_path_common.rs), which rejects ctx.is_multi_ledger(), and every relation in one operator instance is scanned from the single (ctx.binary_store, ctx.binary_g_id) pair — one datatype dictionary, one string dictionary. What leaves the operator is a materialized Lit with the real datatype Sid, so nothing keyed per-ledger escapes into a downstream join.

is_string_term_constraint becomes is_string_dict_term and takes the binding rather than the constraint. That reframing is most of the point: the question is "is this term interned in the string dictionary", and it has one answer per binding shape. An encoded literal is in the lane exactly when its object kind is LEX_ID, which is the dictionary's own kind and admits no argument. A decoded one is in it when its value is a string and its datatype is one the dictionary holds — the value alone isn't enough, because a cast or STRDT can build a string-backed literal for a datatype stored somewhere else entirely (xsd:float(?o) is carried as a String + xsd:float), and handing a constraint to a probe that has nothing to do with the string dictionary moves queries this PR has no business moving — ?x ex:p ?o . BIND(xsd:float(?o) AS ?f) ?y ex:p ?f is the one that showed it.

The datatype half is is_string_dict_datatype, in fluree-db-core/src/datatypes.rs next to dt_compatible because it's the same kind of shared datatype-matching rule and doesn't belong to the query crate. Membership is read off the OType each recognized datatype resolves to rather than a hand-written list of names, so it can't drift from the storage layout; an IRI Fluree doesn't recognize is a customer datatype, and those always route to the string dictionary, so it answers true. Both probe sites in join.rs and optional.rs read the one relation, and the encoded arm names its datatype through reserved_datatype_sid, which DATATYPE(?v) was already using to answer the identical question about the identical field. That arm's per-ledger dt_sids() fallback is unreachable by the same argument — only the three reserved ids can appear on an encoded string-dict binding — and now carries a debug_assert saying so, loud the day a widening of the encoded set forgets that probe site.

This follows #1676 (aaaa7d429) deliberately rather than inventing a second shape — same probe-substitution mechanism, same dt_compatible matching on the scan side, same test file. The one thing it adds is the decode-lane half, which #1676 didn't need for its two datatypes because both of theirs have reserved ids.

What it costs, and what I chose not to do

On the row-at-a-time paths those datatypes lose late materialization: a linear scan over a predicate full of xsd:anyURI decodes a string per row instead of carrying a u32, and the join probe pays the same on its driving rows. I'd rather pay that than keep the wrong answer, and the datatypes that carry the bulk of string data in practice — xsd:string and rdf:langString — are untouched. The cyclic-BGP fast path pays only per surviving row, per the keying above; an earlier revision of this branch let it decline outright, which was a cliff this fix had no need to ship.

The alternative I still chose not to take is teaching EncodedLit itself to carry a per-ledger datatype id, which would need a store handle threaded into late_materialized_object_binding plus a matching extension to encoded_equivalent so decoded and encoded forms still normalize to one key. That's a real option if the linear-scan cost ever shows up in a profile, but it's a much larger change to make a correctness fix.

Numerics are untouched and deliberately so. The join is lenient across numeric subtypes by product decision (it_literal_identity.rs:189), rooted in the index's normalized numeric key, and numerics sit outside this lane on both sides of the relation — never LEX_ID when encoded, and excluded by datatype when decoded, which is what the string-backed-cast clause above is for. The cyclic keying doesn't touch this either: 1^^xsd:integer and 1^^xsd:long carried different dt_ids before and carry different o_types now — same split — and the leniency itself lives in scan-side dt_compatible, not in the join identity. I re-measured the numeric join, the numeric = and the bare-constant match on both lanes before and after, and they are byte-identical.

Tests

string_dict_datatypes_keep_their_identity_in_novelty_and_index pins both consequences from the issue — the = equality and the self-join identity row — plus sameTerm, DATATYPE(), COUNT(DISTINCT), the OPTIONAL probe and the constant-object form, with the JSON-LD twin for the value-object and self-join shapes. It pins xsd:anyURI and xsd:token alongside ex:custom, so this can't quietly be a custom-datatype-only fix, and it runs in three lanes: novelty-only, index-only, and novelty layered over a published index. That third one is where a decoded Lit from novelty and an EncodedLit from the index meet at the same join and DISTINCT key, and it's the lane I'd expect a future regression to come back through.

The cyclic operator gets its own binary, it_cyclic_bgp_string_dict.rs: a shortcut triangle whose literal edge mixes xsd:string, xsd:anyURI and a customer datatype must keep the fast path ON — a positive engagement marker (cyclic_enumerate span present, no cyclic bgp fast path bail event), byte-equality with the fallback tree including the same-lexical-form/different-datatype pairs that must not join, and a novelty tail whose xsd:anyURI lexical form exists only above the string-dict watermark, so the emit decode has to route through DictNovelty. Reverting the encode arm fails it at the engagement assertion — it cannot pass by silently declining.

At the source, object_binding.rs pins which datatypes stay encoded, which are declined, and — the one I care about — that the decode lane and the probe mirror agree on the lane's membership, since two relations over one concept drifting apart is exactly how we got here. It also pins that a string-backed xsd:float stays out. datatypes.rs pins the datatype half in both directions, including that an unrecognized IRI reads as a customer datatype. In cyclic_bgp.rs, object_only_cycle_vars_accept_non_reserved_string_dict_objects pins the identity algebra (same key unifies, same lexical form under a different datatype splits, emit demands the store), and the exhaustive o_type walk above pins the bijection the keying stands on.

The #1729 assertions all fail against the un-fixed code — I reverted each half in place and watched the specific phase go red before restoring it. The xsd:float and datatypes.rs cases are guards on behaviour that was already correct, so they pass either way; they are there so a future widening of the relation has to be deliberate.

Interaction with #1728

None left to coordinate. fix/filter-fold-term-equality-soundness already retargeted the assertion this change would once have tripped: it_issue_1723_sameterm_fold.rs now makes named-pair assertions on the numeric half of "the join is looser than =" instead of a row-count comparison, and its comment cites #1729 and #1737 directly — a join.len() > eq.len() assertion "would go red the day that is fixed — for a fix, not a regression." The two branches land cleanly in either order, and #1728's published-index test can tighten across the lane switch once both are in, since the indexed and novelty lanes now agree on string-dictionary datatypes.

On the red test check

it_ledger_lifecycle::ledger_exists_on_file_storage fails here, and it fails on main at the merge base (fe3c198c8) with the same panic at the same line — the CI run for that commit is red for exactly this one test and nothing else. This branch's run is 11412/11413, main's is 11406/11407; same single failure, six more tests. Not introduced here, and I haven't touched that lane.

One thing found on the way that isn't fixed here

The numeric lane has its own indexed-vs-novelty divergence that predates this and is untouched by it. ?s ex:age 25 against 25, "25"^^xsd:long and 25.0 returns all three on a novelty-only ledger and only the xsd:integer row once the ledger is indexed — the bound-object seek encodes 25 as XSD_INTEGER and seeks that one (o_type, o_key). It's the same divergence class as this issue, on the lane the fix is deliberately staying out of, and bare_numeric_literals_stay_lenient_across_subtypes only ever exercised novelty so nothing caught it. Measured identical before and after this change. Filed as #1737 rather than growing this one — it needs a product call, since the two candidate resolutions (make the indexed seek visit the other subtypes, or make novelty strict) pull in opposite directions, and the leniency is currently load-bearing for the reasoning that closed #1723.

Follow-up: #1737

[bplatz]

Approving the correctness work — it's right, and both halves are verified load-bearing. But I'd like the cyclic-BGP cost addressed before this merges, not deferred; details in the first inline comment. It looks smaller than the PR's stated alternative and it's on a fast path, so I'd rather not ship the cliff and revisit it from a profile.

Verified. Both halves confirmed at the source on main — the else arm handing DatatypeDictId::STRING to every non-langString/non-fulltext StringDict type, and is_string_term_constraint matching only XSD::string plus a lang tag. Reverting each half independently, keeping the other:

MUTANT A (decode lane reverted):  string_dict_datatypes_keep_their_identity... FAILED
MUTANT B (probe mirror reverted): string_dict_datatypes_keep_their_identity... FAILED

Each is load-bearing exactly as claimed. Baselines green: it_literal_identity 4/4, core datatypes 2/2, grp_query 422/422.

I also checked the thing the removed xsd_string_sid() OnceLock comment was proud of — whether reserved_datatype_sid(dt) reintroduces the per-driving-row allocation. It doesn't; it reads WELL_KNOWN_DATATYPES and clones, so it's a refcount bump either way. And is_string_dict_datatype is a namespace-code match plus a name lookup, same order as the old two-field check.

The "Interaction with #1728" section is stale. I looked at origin/fix/filter-fold-term-equality-soundness: the assert!(join.len() > folded.len(), ...) line is already gone, replaced with named-pair assertions on the numeric half, and its comment now cites #1729 and #1737 directly — "a join.len() > eq.len() assertion would go red the day that is fixed — for a fix, not a regression." The coordination hazard has already been resolved the way this PR recommends, so the section as written could send whoever sequences the merges to redo it.

No action on the red test check — your diagnosis is right, and I reached the same conclusion independently while reviewing #1730: ledger_exists_on_file_storage fails on main too. It's now red on main and on three PRs, so it probably wants its own issue rather than a re-run from each branch.

Minor: #1737 is filed and referenced in prose but without a Follow-up: #1737 marker — same convention item as #1718/#1719.

[bplatz]

This return None is consumed differently by different callers, and one of them is worse than the cost paragraph describes.

binary_scan.rs:1508 and join.rs:3169 fall back to materialize-and-decode — that's the "decodes a string per row instead of carrying a u32" cost, correctly described. But cyclic_bgp.rs:330 propagates it: EncObj::encode → encode_object_for_edge → log_fast_path_bail("unsupported-object-binding") at :739/:832 → Ok(None) → return Ok(false) at :1368. The cyclic-BGP fast path declines entirely — and it bails on the first such row in the scan loop, so one xsd:anyURI value anywhere in the predicate switches the fast path off for that whole edge. That's a cliff, not a gradient.

I think it can be preserved without the large EncodedLit change, because the cyclic data plane never needs the semantic datatype:

1. EncObj already carries o_type, and its Hash/Eq/Ord use only id.
2. o_type discriminates exactly what dt_id can't — OType::customer_datatype(DatatypeDictId) embeds the per-ledger id, which is why your own test writes OType::customer_datatype(DatatypeDictId::RESERVED_COUNT).
3. Re-keying EncIdentity::Lit on o_type rather than (dt_id, lang_id) looks behaviour-preserving — I checked each encoded arm and the mapping is injective: INTEGER/LONG/DOUBLE/FLOAT distinct, DATE/TIME/DATE_TIME distinct, Json/Vector/NumBig one o_type each (0x8008/0x8009/0x800B), and langString's lang_id is the OType payload. So nothing that unifies today would split. Notably the numeric leniency you're protecting doesn't live here at all — 1^^xsd:integer and 1^^xsd:long already carry different dt_ids — it comes from scan-side dt_compatible.

Relation build and join then keep every string-dict row with no store handle and no decode. The cost moves to to_binding(), which does need the exact datatype Sid — three call sites, the live one being assignment_to_columns(&self, assignment, cols), which has no ctx today. So it wants the store threaded into that one method and its callers. That puts the decode on rows that survive the join rather than rows scanned, which is the right place for it given cyclic BGP exists because intermediates outnumber outputs.

Two caveats, and the first is why I'm raising it rather than asserting it: this is design analysis from reading, not something I built or measured. And multi-ledger is what I'd check first — customer datatype ids are per-ledger, so across two ledgers the same datatype IRI could carry different o_types and this keying would wrongly split it. Today's dt_id keying has the same exposure, but your decline sidesteps it completely by materializing to real Sids, so the narrow fix is genuinely weaker there. If that turns out to be load-bearing, the decline is the right answer and the cost paragraph just needs widening to say "the cyclic fast path declines" instead of "decodes a string per row".

(Separately: I initially thought fast_post_order_limit was affected too. It isn't — its header says strings/refs/arena already bail to the generic top-k, so widening the decline set doesn't reach it.)

[bplatz]

Unreachable by the PR's own argument — the comment four lines up says only the three reserved ids can appear here, since late_materialized_object_binding keeps every other string datatype materialized.

A debug_assert would make a future widening of the encoded set loud instead of silently falling back to a per-ledger dt_sids() lookup that nothing tests.

[aaj3f]

Thanks @bplatz — you were right that the cliff shouldn't ship, right about the design that removes it, and right to hang it all on the multi-ledger question. That caveat discharges cleanly: the cyclic-BGP fast path cannot run multi-ledger, by exactly the gate you'd expect — open_fast_path (cyclic_bgp.rs:1049-1053) requires allow_cursor_fast_path(ctx), which is !ctx.is_multi_ledger() && ctx.from_t.is_none() && ctx.allow_unfiltered() (fast_path_common.rs:3632), with is_multi_ledger true exactly when the active graph scope spans more than one distinct ledger_id (context.rs:973). And there's a second, independent layer: every relation row in one operator instance comes from the single (ctx.binary_store, ctx.binary_g_id) pair — one datatype dictionary, one string dictionary — and what leaves the operator is a materialized Binding::Lit carrying the real datatype Sid, identical to the fallback's output. So per-ledger ids neither cross ledgers inside the join nor escape it.

With that settled, your sketch is implemented essentially as proposed. EncIdentity::Lit is re-keyed to (o_type, o_key, num_big_p_id); EncObj::encode keeps every StringDict o_type; decode moved to to_binding behind a novelty-aware view built at open_fast_path (watermark routing included), so the string decode lands on rows that survive the join — your "intermediates outnumber outputs" placement. I re-derived the injectivity myself rather than inheriting your checking, and it agrees — INTEGER/LONG, DOUBLE/FLOAT, the three temporals, Json/Vector/Decimal all map to distinct reserved DatatypeDictIds, and langString's lang_id is the OType payload. It's now pinned exhaustively over all 65,536 o_types (encoded_lit_identity_fields_are_injective_over_o_types panics if two ever share a triple), so the property is a gate rather than a review artifact. Your numeric-leniency point checked out too — it's scan-side dt_compatible, and integer/long split identically before and after.

The one place the sketch needed extending: assignment_to_columns had to become fallible, since the emit decode can error — the square-wedge caller restores its state before propagating.

The new it_cyclic_bgp_string_dict binary pins the whole story: a stays-ON marker (enumerate span present, no bail event) with an xsd:string/xsd:anyURI/customer-datatype mix, byte-equality with the fallback, the same-lexical-form/different-datatype pairs proven non-joining, and a novelty tail forcing the decode through DictNovelty. Non-vacuity ran for real: reverting the encode arm fails the test at "cyclic fast path never enumerated — it declined instead."

Also folded in: the join.rs:1013 debug_assert as you suggested; the stale "#1728 interaction" section rewritten after re-verifying at the source (the > assert is indeed gone, replaced by the named-pair numeric assertions citing #1729/#1737 — exactly as you found); the Follow-up: #1737 marker; and the cost section now names what was previously a cliff honestly. it_literal_identity 4/4 unchanged, grp_query 422/422 matching your baseline.