Skip to content

fix: make the equijoin-filter fold sound about term equality - #1728

Merged
aaj3f merged 13 commits into
mainfrom
fix/filter-fold-term-equality-soundness
Aug 28, 2026
Merged

fix: make the equijoin-filter fold sound about term equality#1728
aaj3f merged 13 commits into
mainfrom
fix/filter-fold-term-equality-soundness

Conversation

@aaj3f

@aaj3f aaj3f commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #1721
Fixes #1723
Fixes #1738

Two soundness defects in the equijoin-filter fold, with one root cause worth stating plainly: the fold's correctness rests on "the join implements RDF term equality," and it does not. #1721 is where a bad statistic lets a query into that rewrite; #1723 is where the code walks in deliberately. They're together here because the fix for either one is unconvincing without the description of the join that the other forces you to write down. #1738 — the same bad statistic read by the scan-narrowing lane, where it invents rows rather than declining an optimization — is folded in as well, because the fix below made its correct fix cheap and the class deserves to close whole.

What the join actually guarantees

filter_fold turns ?a :p ?x . ?b :p ?y . FILTER(?x = ?y) into ?a :p ?x . ?b :p ?x, which is a large win on the analytic shapes (BSBM BI-2 goes ~28s → ~0.03s) and is exactly right when the objects are nodes. The module doc justified the rewrite against two notions of equality — SPARQL = is value equality, sameTerm is term equality — and concluded that = needs a node guard and sameTerm needs none.

But the join is a third thing. It unifies on the encoded term, which sits between the two and coincides with neither on literals:

  • It is looser than term equality on purpose. A bare numeric object carries no datatype constraint into the probe, so 1, "1"^^xsd:long and 1.0 all unify. That's a product decision, pinned by it_literal_identity.rs::bare_numeric_literals_stay_lenient_across_subtypes, and the index's normalized numeric key is built around it. (One caveat worth naming rather than letting a reader trip over: that leniency currently holds on the novelty lane and not once the ledger is indexed, because a constant object is encoded to its own datatype for the bound-object seek — the pinning test only ever exercised novelty. That's Numeric subtype leniency holds on novelty but not once indexed — the same constant-object query answers differently across an index publish #1737. It doesn't change anything here; the fold has to be gated under either behaviour.)
  • It is looser again where the encoding flattens: every string-dictionary datatype that is not xsd:string or rdf:langString reaches the join key as xsd:string, so "abc"^^ex:custom unifies with "abc".
  • And it isn't value equality either — "1"^^xsd:string and 1 never unify, where = raises a type error rather than the join's silent non-match.

So sameTerm, which is the strictest equality SPARQL offers and specifically the function you reach for when you do not want value equality, was being answered with something looser than =. On a thirteen-object matrix under one predicate it returned 35 rows where the unfolded answer is 21, equating 1 with 1.0 and "abc"^^xsd:string with "abc"^^ex:custom. No unusual ledger state needed — it fired on any ledger with stats present. (#1723 quotes 19 → 47 for the same matrix; those were measured before #1676 landed and tightened the string half of the join. The gap is smaller now, and still a wrong answer.)

Both functions now require both variables to be provably node-valued, where all three notions coincide, and the module doc says what the join guarantees instead of assuming it. sameTerm still folds on a ref-only predicate, which is where the optimization was earning its keep anyway. The = answer does not move: 31 rows on the same matrix, before and after, which the tests pin as a companion guard.

The other shape of fix — make the join key carry datatype and language so the premise becomes true — is mostly already landed for strings (#1676 did the plain/lang-tagged work, and #1736 is closing the rest), and it still would not get us there. The numeric leniency is deliberate, and it lives in the normalized numeric key the index is built on. That is not a thing to undo for a fold, and it is why the join stays looser than sameTerm even with the string half fully fixed: 21 rows against the join's 31 on this matrix.

Why a no-op delete could reach the unsound rewrite

StatsView::property_ref_only is the sole soundness input to the guard = was already using, and it was read off the PropertyStatEntry.datatypes counts. On the query path that breakdown is the base index merged with novelty as a blind ±1 delta log with no probe of the base (assemble_fast_stats_inner), and merge_property_datatypes drops any tag whose merged count reaches zero, with no clamp at the base value.

So a retraction of a fact the ledger never held still charges its -1. Under a predicate carrying both refs and literals — precisely the population this guard exists for — that can drive the last literal tag to zero, the predicate reads as all-ref, and the fold gets a licence it must not have. Three mundane transaction shapes reach it: a DELETE DATA of triples not in the graph (a no-op by SPARQL 1.1 Update), the JSON-LD equivalent, and replaying a delete that already ran — an at-least-once delivery retry, a replayed migration, an idempotent-by-intent client. The results change silently, with no error and no warning, and stay changed until the next index publish reconciles the tags.

PropertyStatEntry now carries observed_datatypes — the datatype tags of the current state as of the publish, unioned at query time with the tags novelty asserted — and that is what the flag reads. An assertion can still add a tag and revoke the licence while no retraction can take one away, so the set is monotone under retraction within the novelty window, and each publish re-derives it from the exact breakdown so a tag whose data is genuinely gone ages out. Everything else about the merge is unchanged: the datatypes counts stay exactly as they were, unclamped, because several estimators sum them and because reconciling them properly means probing the base index per novelty flake — quadratic in the novelty window, which is the same conclusion #1391 reaches about the same lane.

Two things I deliberately did not do. I didn't try to make the estimate lane exact, for the reason above. And I didn't reuse the class_coverage_trustworthy pattern next door (stats_cache.rs:134), which refuses the whole view whenever novelty is non-empty: that would disable the fold on every live ledger and give up the BI-2 class of win in exactly the deployments that need it. The monotone rule keeps the optimization and removes the hazard.

One incidental change on the same path. The novelty walk kept three maps under the same (namespace_code, name) key — the running count, the per-datatype deltas, and the asserted tags — and building that key allocated its name, once per map, for every flake in the window. They are one PropertyStatDelta behind one borrowed key now, so the per-flake allocation is gone entirely — what is left is per distinct predicate, including the one owned (u16, String) the wire format wants, built once at the end in finalize_stats. This isn't the query hot path — it runs on a stats-cache rebuild, once per overlay epoch — but the walk is O(novelty window), which makes it the one place in stats assembly where per-flake constant factors show up at all.

The same tag set, read at the wrong t — and why the index now remembers its history

A published index describes current state as of the publish, and a read at to_t <= indexed_t is served from it verbatim (runtime_stats.rs:212) — novelty only ever holds flakes after it. For the counts that is the ordinary estimate drift the planner already tolerates. For the observed-tag set it isn't, because that set is read as a licence, and the direction it is wrong in is the unsafe one.

No spurious retraction is needed for this one. Delete every literal under a mixed predicate, publish, and the index's tag set is honestly all-ref — for the published t. A query at an earlier t still sees those literals, and would read that same set as its licence to fold a FILTER(?x = ?y) that equates "abc" with "abc"^^ex:custom. Nothing about the ledger is unusual: the delete is real, the stats are honest, and the fold is simply reading a fact about a different t.

Clearing the set for every property on any read below the index t would close that hole, and it is also a wholesale loss: the fold is worth ~28s → ~0.03s on BI-2, and the clearing takes it away from every predicate on every historical read — including pure-ref predicates that never carried a literal and cannot be affected by the hazard. Nothing on hand could narrow it, because the set is re-derived from current-state counts at each publish: after the publish there is no record that distinguishes "always ref-only" from "literals deleted before the publish". The information has to be persisted, so now it is.

The stats section rides inside the FIR6 root behind a u32 length prefix, and every root decoder advances by the prefix rather than by its own consumed count (index_root.rs:1052, core's mirror in db.rs) — which means a strictly appended tail is invisible to old readers and detectable by new ones, with no version bump and no migration. The tail (encode_historical_tail in stats_wire.rs, which documents the layout and the evolution rules) carries, per property — aggregate and graph-scoped — historical_datatypes: every datatype tag the property has carried at any t since IndexStats::historical_since_t, made monotone across publishes by construction: each build's persisted set is the prior persisted set unioned with every tag its walk observes. A full rebuild and an import replay the entire commit chain, so they claim coverage from genesis (historical_since_t = Some(0)). An incremental publish seeds from the base root's persisted sets plus the base's exact current-state tags, and its novelty walk observes every record in the window — a tag visible at any t in (base_t, index_t] was either visible at base_t or asserted inside the window — so the base's boundary carries forward, and a base that predates the tail adopts soundly at its own t. Adoption trusts exactly the artifact the current-state licence already trusts at every publish (the build-side counts being exact for the published state), so it introduces no new assumption.

cached_stats_view_for_db then substitutes instead of clearing: on a read below the index t, observed_datatypes := historical_datatypes when db.t >= historical_since_t, cleared only below the boundary or on a root that predates the tail. The soundness argument is two sentences: for any db.t at or above the boundary, the historical set contains every tag visible at db.t; and an over-approximation can only decline a rewrite, never license one. So a never-literal predicate keeps the fold at historical ts, a mixed-history predicate correctly loses it, and the conservative fallback is confined to old roots and pre-adoption ts — a region that is fixed at adoption and disappears entirely at the ledger's next full rebuild, instead of being a permanent tax on time-travel analytics.

Two sets rather than one, deliberately. Making the existing field itself monotone-forever would have been the smaller change, but it trades away the per-publish self-cleaning that current-state reads rely on: a predicate that once carried a stray literal would decline the fold permanently, even after the data is long gone and reindexed. observed_datatypes keeps its exact current-state semantics — current reads lose nothing, ever — and the historical set is consulted only on the reads where the current-state set was never a fact about the queried graph to begin with.

One repair the seeding pass absorbed on the way: the incremental path's base carry-forward was all-or-nothing — if the HLL sketch blob loaded, base-stats seeding was skipped entirely, so a key the blob happened to lack fell out of the stats. It seeds per key now (incremental.rs), blob entry winning, base-root stats as the per-key fallback.

The graph-scoped twin: scan narrowing (#1738)

The same statistic has a second consumer, and it is the worse one: infer_exact_datatype_sid_from_stats (fluree-db-query/src/binary_scan.rs) read the graph-scoped datatype counts as a set and narrowed a bound-object scan to one exact datatype when that set was (effectively) a singleton. Those counts are merged by the same blind ±1 with the same drop-at-zero (increment_count, runtime_stats.rs), so the same no-op delete zeroes a real tag, leaves a singleton, and the scan returns rows that do not match the query — #1721 declined an optimization, this lane invents rows. It reproduces exactly as filed: ex:p carrying "25"^^xsd:int and "25"^^xsd:long, published index, novelty non-empty on both measurements, ?s ex:p 25 — a no-op delete typed xsd:int moves the answer, and the control typed xsd:short, a tag the predicate never carried, does not.

The issue deferred this as roughly the same size again on a lane this PR didn't touch. The wire work changed that arithmetic: the historical tail and the producer plumbing had to visit every one of those construction sites anyway, so the mirror is folded in rather than deferred. GraphPropertyStatEntry now carries observed_datatypes and historical_datatypes with the same semantics as the aggregate — same producers, same tail, same substitution below the index t — the novelty merge tracks per-(graph, predicate) asserted tags next to the aggregate's (update_graph_property_datatypes), and the narrowing reads the set instead of the counts, with empty meaning unknown and declining. That also closes the time-travel variant of the narrowing, which the aggregate-only treatment never covered: the graph-scoped counts were being read as current-state facts at historical ts too.

Keeping a fail-closed field filled

An empty observed-tag set means "unknown", not "no datatypes", so a producer that forgets to fill one costs an optimization rather than breaking a query. That's the right failure mode, and it's also the reason these fields want more test attention than their size suggests: a miss is invisible. observed_datatypes is still not on the wire — every decoder re-derives it from the exact breakdown it just read — so its producer pins remain the only thing that would catch a miss. The historical sets and their boundary, by contrast, now genuinely are on the wire, which is what finally lets a round-trip assert something the re-derivation can't fake: the tail tests plant a historical tag no current count mentions and watch it survive encode → decode in every decoder.

Three structural things follow. The import's copy of the per-graph → ledger-wide property roll-up stays gone: it and the index pipelines share aggregate_property_entries_by_sid, which now also unions the per-graph historical sets into the aggregate — one producer, one test, for both fields. The two binary-index decode entry points (decode_stats, decode_stats_with_len) had byte-identical bodies, which would have meant maintaining the tail parse twice; decode_stats is now a thin wrapper over the other, so the format has exactly two live decoders — this crate's and fluree-db-core's reader-only mirror (the one the memory backend reaches) — and each still gets its own named re-derivation assertion. And the build-side walk tracks the historical tags as a 256-bit set on IdPropertyHll (one shift-or per record, no allocation), because the rebuild path feeds every record of every commit through it.

Tests

The mechanism is pinned at the source in runtime_stats.rs: a spurious retraction must not take a literal tag away, a novelty assertion must still be able to add one, and a predicate novelty introduces outright with only ref objects must still qualify. stats_view.rs pins that the flag reads the tag set and not the counts, including the fail-closed empty case. stats_cache.rs pins the licence around the boundary: a read at the index's own t keeps the fold, a historical read keeps it for a never-literal predicate and loses it for a deleted-literal one, a read below the adoption boundary fails closed, and an old root without the tail keeps today's conservative behavior everywhere below the index t.

The wire contract has its own suite in stats_wire.rs: the tail round-trips through both decoders (with a historical tag no count mentions, so re-derivation can't fake it); a blob encoded without the tail decodes conservatively; a new blob is byte-for-byte the old encoding plus a strict suffix — which is the structural fact that makes an old reader's parse of a new blob exactly its parse of the old one; and an unknown future tail tag reads as absent with the remainder consumed.

End to end, it_issue_1721_repro.rs pins all three no-op shapes, the published-index variant, and the time-travel read — which now also asserts the published root carries historical_since_t = Some(0) and remembers the deleted literals' tags in ex:p's historical set while its current-state set is honestly all-ref. The monotone-across-publishes claim gets the test that isolates it: three publishes, where the third window contains no trace of the deleted literals, so only the base root's persisted sets can carry their tags forward — remove the incremental union and it goes red, which doubles as proof the publish routed through the incremental pipeline rather than a silent rebuild. it_issue_1738_scan_narrowing.rs pins the narrowing hazard on both query surfaces with the control that makes the mechanism conclusive, and binary_scan.rs unit-pins the consumer: the observed set vetoes narrowing when the counts dropped a tag, and an empty set fails closed. it_issue_1723_sameterm_fold.rs pins sameTerm and = against their unfolded answers on the thirteen-object matrix, and pins what the fold rewrites into by naming the pairs the join equates rather than counting them. Each new guard was reverted in place and its specific assertion watched go red before restoring.

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

The encoded-object read lane flattens non-xsd:string string-dictionary datatypes onto xsd:string (fluree-db-query/src/object_binding.rs:105-112, and is_string_term_constraint at binding.rs:264-272 has the mirror-image gap). That's visible with no fold involved at all: on an indexed ledger, FILTER(?x = ?y) reports "abc"^^xsd:string equal to "abc"^^ex:custom, and a self-join on that predicate loses the custom-typed literal's own identity row. It's independent of both issues here and predates them — it's the residue of #1676, which fixed the plain/lang-tagged half of the same problem. It is why both the published-index and the time-travel tests compare measurements taken within one lane rather than across a lane switch. That one is #1729, and #1736 is the fix; once it lands, both of those tests can be tightened to compare across the lane switch instead, which is the stronger assertion and is only blocked by the flattening.

aaj3f added 4 commits August 27, 2026 15:14
… shrink

`StatsView::property_ref_only` is the sole soundness input to the
equijoin-filter fold: a `true` licenses rewriting `FILTER(?x = ?y)` into a
join, which is only equivalent when the objects are nodes. It was read off
`PropertyStatEntry.datatypes`, and on the query path that breakdown is the base
index merged with novelty as a blind ±1 delta log with no probe of the base. A
retraction of a fact the ledger never held still charges its `-1`, and
`merge_property_datatypes` drops any tag whose merged count reaches zero — so a
predicate carrying both refs and literals could lose its last literal tag to a
delete that removed nothing, read as all-ref, and hand the fold a licence it
must not have. A no-op `DELETE DATA`, the JSON-LD equivalent, or a replayed
delete was enough.

`PropertyStatEntry` now carries `observed_datatypes`, the datatype tags the
property has ever been seen with, and that is what the flag reads. The novelty
merge builds it as the base index's tags unioned with the tags novelty
ASSERTED, so an assertion can still add a tag and revoke the licence while no
retraction can take one away. The `datatypes` counts are untouched — several
estimators sum them, and reconciling them would mean probing the base index
per novelty flake, which is quadratic in the novelty window.

The flag is therefore allowed to be conservative and never optimistic: after
legitimately deleting every literal under a predicate the fold stays declined
until the next index publish reissues the base tag set without that tag. Gating
on `novelty.is_empty()`, as `class_coverage_trustworthy` does, would instead
have disabled the fold on every live ledger.
`filter_fold` skipped its node-valued check entirely for
`FILTER(sameTerm(?x, ?y))`, on the premise — stated in the module doc — that
`sameTerm` is term equality and so needs no guard. That is true of SPARQL and
false of the join the fold rewrites into.

The join unifies on the encoded term, which is neither value equality nor term
equality for literals. It is looser than term equality on purpose: a bare
numeric object carries no datatype constraint into the probe, so `1`,
`"1"^^xsd:long` and `1.0` all unify, which
`it_literal_identity.rs::bare_numeric_literals_stay_lenient_across_subtypes`
pins as a product decision and the index's normalized numeric key is built
around. It is looser again where the encoding flattens string-dictionary
datatypes onto `xsd:string`, so `"abc"^^ex:custom` unifies with `"abc"`. And it
is not value equality either — `"1"^^xsd:string` and `1` never unify, where `=`
raises a type error rather than silently not matching.

So `sameTerm`, the strictest equality SPARQL offers, was being answered with
something looser than `=`. On a thirteen-object matrix under one predicate it
returned 35 rows where the unfolded answer is 21, equating `1` with `1.0` and
`"abc"^^xsd:string` with `"abc"^^ex:custom`. Both functions now require both
variables to be provably node-valued, where all three notions coincide, and the
module doc states what the join actually guarantees instead of assuming it.
`sameTerm` still folds on a ref-only predicate, which is where the optimization
was earning its keep.
The ref-only flag now reads a field every producer of `PropertyStatEntry` has
to fill, and an unfilled one reads as "unknown". That fails closed — a missed
producer costs the equijoin-filter fold silently instead of breaking a query —
so it wants a test of its own. This walks the chain a deployment actually uses,
indexer aggregate through stats-wire encode and decode into the view, and
asserts an all-ref predicate still reads ref-only with a literal-valued sibling
for contrast.
aaj3f added 4 commits August 28, 2026 00:01
The walk over the novelty window kept three maps under the same
`(namespace_code, name)` key — the running count, the per-datatype ±1
deltas, and the tags novelty asserted. Building that key owned its name,
so every flake in the window paid three `String` allocations to reach
three entries that always move together.

Fold them into one `PropertyStatDelta` value behind one key, and borrow
the name from the flake (or from the index entry that seeded it) rather
than copying it: the walk now allocates nothing per flake, and the owned
`(u16, String)` the wire format wants is built once per distinct
predicate in `finalize_stats`.

This is not the query hot path — it runs on a stats-cache rebuild, once
per overlay epoch — but the loop is O(novelty window), so it is the one
place in stats assembly where per-flake constant factors show up at all.
The import carried its own copy of the per-graph -> ledger-wide property
roll-up, byte for byte the same accumulation the incremental and rebuild
pipelines run through `aggregate_property_entries_from_graphs`, differing
only in how a `p_id` becomes a SID (the import already has the table; the
index pipelines go through the IRI and the prefix trie).

Split that difference out into a resolver closure so both go through one
`aggregate_property_entries_by_sid`. The duplicate mattered more than
duplicates usually do because it was a producer of
`PropertyStatEntry::observed_datatypes`, which is fail-closed: an unfilled
one reads as "unknown" and silently declines the equijoin-filter fold
instead of failing anything, so a copy that drifted would cost the
optimization with nothing going red. One producer, and a unit test that
pins the tags surviving the roll-up.
…rivation

`observed_datatypes` is not on the wire. Every decoder re-derives it from
the datatype breakdown it just read, which means a round-trip test can
only ever observe the one decoder it happens to route through — and a
decoder that stopped re-deriving would hand back an empty set, which reads
as "unknown", which declines the equijoin-filter fold silently.

So assert it directly, once per entry point: `decode_stats` and
`decode_stats_with_len` here, plus `fluree-db-core`'s reader-only mirror of
the same format (the memory backend reaches that one where the binary path
reaches these two). Zeroing any of the three now fails a named assertion.

Also narrows `ref_only_survives_a_published_index_round_trip`'s doc comment
to what that test actually covers, and points at the unit pins for the rest.
The observed-datatype set says which tags a predicate carries, and the
ref-only flag turns "only node tags" into a licence for the equijoin-filter
fold. On a current-state read that holds. Below the published index `t` it
does not, and no spurious retraction is needed to break it: a read at
`to_t <= indexed_t` is served from the base index verbatim, and the base
index is current state *as of the publish*. Delete every literal under a
mixed predicate, publish, and its tag set is honestly all-ref — for the
published `t`. A query at an earlier `t` still sees those literals and
would read that same set as its licence.

Clear `observed_datatypes` when the read is below the index `t`, in the
one builder that knows both numbers. Empty already means "unknown", so
this needs no new state and no new flag: the flag falls back to
not-provably-ref-only and the fold declines for the historical read, while
the counts — which are estimates either way — are left alone. Current-state
reads at `to_t == indexed_t` are unaffected.

The end-to-end test measures folded against unfolded at the same `t`
rather than against a written-out expectation, because the historical read
lane flattens datatypes on its own (#1729) and that moves the answer too;
comparing two query shapes inside one lane isolates the fold.
… rows

`join.len() > eq.len()` said "the join is looser than `=` on this data",
which was true only because the indexed-read datatype flattening (#1729)
was manufacturing four `"abc"^^ex:custom` pairings. Fixing that flattening
takes the join to exactly the `=` answer on this matrix, so the assertion
would have gone red for a fix rather than a regression.

Assert the two things that stay true instead. The join equates numeric
subtypes — four distinct RDF terms sharing one normalized numeric key —
which is the looseness that outlives any encoding change and the reason
`sameTerm` needs the node guard while `=` does not. And the join accepts
everything `=` accepts here, so the fold can only ever add rows to a `=`
answer, never drop one, which is what makes the node-valued check the whole
of the soundness argument.

Comments record why each number is what it is, including that this ledger
is novelty-only and the numeric leniency is currently lane-dependent once
indexed (#1737).

@bplatz bplatz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Reproduced both defects on pristine main (b892ac9f9) with my own fixtures and confirmed both are closed here:

#1721  no-op delete (3 triples before and after, nothing removed)
       main:   3 rows -> 4 rows, inventing [ex:s2 ~ ex:s1]
       branch: 3 rows -> 3 rows

#1723  sameTerm foldable / unfolded / `=` foldable
       main:   35 / 21 / 31
       branch: 21 / 21 / 31

One framing note worth using: on main sameTerm returned more rows than = (35 vs 31). The strictest equality SPARQL offers was the loosest thing in the engine — sharper than 35-vs-21 and it lands the root cause in one line.

Also checked #1729/#1736/#1737/#1738 are all real open issues, and that #1721's own title is scoped to filter_fold, so the Fixes line auto-closing it is correct with #1738 carrying the class remainder.

Two inline items. The first is the one I'd want explored before final merge — not because the fix is wrong, but because the cost is wider than the hazard and it may or may not be reducible.

// `t` at which it demonstrably carried literals. Empty means
// "unknown", so clearing the set declines the fold for the historical
// read and leaves the counts alone.
if db.t < db.snapshot.t {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clears observed_datatypes for every property, so every read below the index t loses the fold for every predicate — including pure-ref predicates that never carried a literal and cannot be affected by the hazard. Given the fold is worth ~28s -> 0.03s on BI-2, that is a wholesale fast-path loss for time-travel analytics, and the description ("the fold declines for the historical read") reads as scoped to the hazardous predicate rather than global.

Worth a pass for a sound narrowing before merge. I couldn't find one that works with what's on hand — the base set is re-derived from current-state counts, so it can't distinguish "always ref-only" from "literals deleted before the publish" — so this may well be the only correct answer today. Two directions if you want to test that:

  • Persist the observed-tag set on the wire and make it monotone across publishes, not just within a novelty window. Then a historical read has a set that is sound for every t, and the clearing goes away entirely. Bigger change, and it makes the field mean what its doc already claims (see the other comment).
  • Narrow the clearing to predicates that could have lost a tag — needs some record that a retraction happened under that predicate before the publish, which I don't think exists today.

If neither is cheap, saying so explicitly plus stating the cost at full size is fine — just worth having looked rather than accepting the blast radius by default.

Comment thread fluree-db-core/src/index_stats.rs Outdated
/// Graph-scoped property stats (authoritative for range narrowing) live under
/// `IndexStats.graphs[*].properties[*].datatypes`.
pub datatypes: Vec<(u8, u64)>,
/// The datatype tags this property has ever been observed carrying, sorted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"has ever been observed carrying" overstates the field. It is re-derived from the current-state datatypes counts by tags_of at all three decoders, so every index publish resets it — the accurate scope is the second half of this doc, monotone under retraction within the novelty window.

Worth tightening because the "ever" reading is exactly what would make a later reader conclude the time-travel clearing is unnecessary, or reuse the field somewhere the per-publish reset breaks the assumption. The name has the same pull.

aaj3f added 3 commits August 28, 2026 11:14
…cross publishes

The stats section rides inside the FIR6 root behind a u32 length prefix,
and every root decoder advances by the prefix rather than the decoder's
consumed count — so an appended tail section is invisible to old readers
and detectable by new ones. The tail carries, per property (aggregate and
graph-scoped), the set of datatype tags the property has carried at any t
since IndexStats::historical_since_t, accumulated monotonically: each
build unions the prior persisted set with every tag its walk observes.

Full rebuild and import replay the whole commit chain, so they claim
coverage from genesis. An incremental publish seeds from the base root's
persisted sets plus the base's exact current-state tags, which lets a
base that predates the tail adopt at its own t; the novelty window is
covered by the walk. The observed_datatypes field keeps its current-state
per-publish semantics untouched — current reads lose nothing.
…oth hazards end to end

The tail round-trips through both live decoders; a blob without it decodes
conservatively; a new blob is byte-for-byte the old encoding plus a strict
suffix (which is what makes an old reader's parse of a new blob exactly its
parse of the old one); an unknown future tail tag reads as absent. The
monotone-across-publishes claim gets the test that isolates it: three
publishes, where the third window carries no trace of the deleted literals
and only the base root's persisted sets can carry their tags — it goes red
if the incremental union is removed, which doubles as proof the publish
routed through the incremental pipeline. #1738's no-op-delete scan
narrowing is pinned with the control that makes the mechanism conclusive,
on both query surfaces.
@aaj3f

aaj3f commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @bplatz — you were right to push on this, and the wire direction you sketched is what landed. The blast radius is gone rather than documented.

Feasibility turned on one structural fact, and it's the first thing I checked: the stats section rides inside the FIR6 root behind a u32 length prefix, and both root decoders slice exactly stats_len bytes and discard the stats decoder's own consumed count — index_root.rs:1052 and core's mirror at db.rs:982. So a strictly appended tail is invisible to old readers and detectable by new ones (pos < len), with no version bump and no migration. That's pinned structurally: a test asserts a new blob is byte-for-byte the old encoding plus a suffix, and a cross-version test decodes an old-format blob conservatively.

One deliberate divergence from the one-field version of your suggestion, and I'd defend it: making observed_datatypes itself monotone-forever silently trades away its per-publish self-cleaning — a predicate that once carried a stray literal would decline the fold permanently on current-state reads. So there are two sets. observed_datatypes keeps exact current-state semantics (current reads lose nothing, ever), and a new historical_datatypes — monotone since historical_since_t, travelling on the wire — is what stats_cache substitutes below the index t. The global clearing you flagged now survives only below the adoption boundary and on old-format roots, a region that ages out at the next rebuild instead of being permanent. This also settles your second inline from the other direction: the docs and code now agree because the field that genuinely means "ever (since the boundary)" exists and is named that, while observed_datatypes is documented as current-state-per-publish.

Soundness in two sentences: for every t in [historical_since_t, index_t], a property's historical set contains every tag visible at t — rebuild and import observe every record ever (→ Some(0)), and an incremental's window tag was either visible at base_t (seeded from the base's state ∪ persisted sets, which is what makes adoption at base_t sound for old-format bases) or asserted in the window (walked). And an over-approximating tag set can only decline a rewrite, never license one, so substitution is sound and extra tags are harmless.

#1738 folded in. The tail and its producers had to visit all six of that issue's construction sites anyway, so the marginal cost collapsed: GraphPropertyStatEntry carries both sets, the novelty merge tracks per-(g, p) asserted tags, and infer_exact_datatype_sid_from_stats narrows off the set instead of the ±1-drifted counts — which also closes the time-travel variant of the narrowing that the old aggregate-only clearing never covered. Fixes #1738 is in the body, so #1721's class closes rather than the fold's half of it.

Non-vacuity ran for real on all of it — the boundary substitution, the tail write, the #1738 e2e with its control, and a three-publish monotone test that only goes red when the incremental union is removed and thereby proves the publish actually routed incrementally rather than via a silent rebuild. The red test check on the pre-push run was the inherited it_ledger_lifecycle failure your 6b4c211d4 has since fixed on main; the fresh run should be fully green.

@aaj3f
aaj3f merged commit bbda727 into main Aug 28, 2026
14 checks passed
@aaj3f
aaj3f deleted the fix/filter-fold-term-equality-soundness branch August 28, 2026 16:14
aaj3f added a commit that referenced this pull request Aug 28, 2026
…ures

The merge with main was textually clean but the resolver tests construct
IndexStats and PropertyStatEntry literals that main's #1728 added fields
to. Values follow main's own test conventions: historical_since_t: None
(no adoption boundary), observed_datatypes mirroring the entry's own
tags, historical_datatypes empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:query Query execution, planning, fast paths, overlay, result formatting bug Something isn't working as expected

Projects

None yet

2 participants