fix(query): encode JSON-LD constant references at lowering so joins reach the batched lanes by bplatz · Pull Request #1856 · fluree/db

bplatz · 2026-09-16T12:18:35Z

Why

The same two-pattern join, on an indexed ledger with no policy:

Surface	right predicate encoded	batched probe lane used
JSON-LD	no	no
SPARQL	yes	yes

JSON-LD lowering left every constant subject and predicate as Ref::Iri for "deferred encoding" (lower_ref_term), and the only pass that encoded them later, in the runner, runs under RDFS/OWL reasoning alone. SPARQL's lower_iri_ref encodes at parse. The nested-loop join's batched subject, object, and exists probe lanes require the right predicate as Ref::Sid (is_batched_eligible), so JSON-LD joins always took the per-row scan path. Both the lowering and the lane requirement date to the v4 baseline: JSON-LD joins have never used the batched lanes, on any ledger, with or without policy.

Explain has its own IRI-to-SID normalization, which is why explained JSON-LD plans always looked encoded while execution was not.

Found while testing #1855: the per-predicate policy lanes there could only be reached from SPARQL.

What changes

One lowering rule. IriEncoder gains encode_ref / encode_term: a SID when the prefix is registered, the IRI otherwise for the scan to encode per graph. JSON-LD's lower_ref_term, SPARQL's lower_iri / lower_iri_ref and its two stable-blank-node sites, and Cypher's iri_ref / iri_term all call it; the six hand-rolled copies of the match are gone. Cypher keeps its one documented addition — a bare name with no @vocab lands under namespace 0, which encode_iri_strict rejects by design — layered over the shared rule rather than beside it. Cross-ledger execution already re-encodes pattern SIDs per graph through reencode_sid, exactly as it does for SPARQL today. Every api consumer that matches on the IRI form already handles the SID form.

History gate on the probe lanes. Encoding exposed a latent gap in the lanes themselves. None of the probe-lane planners (subject_probe_lane_plan, object_probe_lane_plan, star_probe_lane_plan), nor the join's early overlay-free admission in compute_batched_overlay_mode, checked for a history range. SPARQL has no history-range form, so the lanes had never met one. Once JSON-LD reached them, two existing tests (OPTIONAL and FILTER NOT EXISTS under from/to) regressed to current-state answers with no op.

The three planners already shared an identical admission preamble (policy → overlay-free → eager → single-graph) and the join hand-rolled a prefix of it, so rather than add a fifth copy of a new step to each, the preamble is now one probe_lane_admission(ctx) that all four call, with the history gate in it ahead of the overlay-free return. The gate itself is ExecutionContext::is_history_range(), which also replaces the from_t.is_some() / is_none() checks the other fast paths had hand-rolled (allow_cursor_fast_path, fast_path_eligible_no_policy, cursor_fast_path_for_predicate, fast_union_star_count_all, range_semijoin, fast_whole_graph_agg). from_t and the plan's TemporalMode::History are set from the same history_time_range() match and nothing else sets from_t, so the cyclic BGP operator's mode field — whose only use was a second check of the same fact — is dropped.

The annotation-edge probe was checked as the other Ref::Iri-declines-while-Ref::Sid-proceeds site: it is independently gated on history and every cross-ledger case in its own build_batch (optional.rs), so encoding does not newly reach it.

Tests

New: the same join through JSON-LD and SPARQL must both report use_batched on the join's routing event. Fails with the previous lowering.
New: a JSON-LD join over a two-ledger dataset whose ledgers assign the join predicate different namespace codes (the fixture asserts the divergence) must return the same rows as the SPARQL spelling. With reencode_sid made a no-op it returns no rows.
The two parse unit tests that pinned the deferred IRI form now expect the SID for a registered prefix and the IRI for an unregistered one.
The two history tests above are the pins for the lane gate: with the history check removed from probe_lane_admission, grp_query_history is 62 passed / 2 failed, exactly those two.

Suites run: query, sparql, and cypher crate unit tests; api grp_query, grp_query_history, grp_query_sparql, grp_policy, grp_query_reason, grp_misc; clippy on the query, sparql, cypher, and api crates; fmt.

Relationship to #1855

Independent, branched from main. Merging both means JSON-LD joins under a property-only policy reach the per-predicate probe lanes too.

Follow-up

Follow-up: #1892 — the object-position twin, lower_term, still defers constant IRIs to Term::Iri where SPARQL and Cypher encode. No consumer has been shown to lose a lane on it, so it is filed as a parity audit rather than widened into this PR.

…each the batched lanes JSON-LD lowering left every constant subject and predicate as Ref::Iri ("deferred encoding"), and the only pass that encoded them later ran under RDFS/OWL reasoning alone. SPARQL lowering encodes at parse. The nested-loop join's batched subject, object, and exists probe lanes require the right predicate as Ref::Sid (is_batched_eligible), so the same join took the batched lane from SPARQL and the per-row scan path from JSON-LD. Both the lowering and the lane's requirement date to the v4 baseline: JSON-LD joins have never used those lanes. lower_ref_term now encodes the way SPARQL's lower_iri_ref does: a SID when the prefix is registered, the IRI otherwise for the scan to encode per graph. Cross-ledger execution already re-encodes pattern SIDs per graph (reencode_sid), exactly as it does for SPARQL today. Explain had its own IRI-to-SID normalization, which is why explained plans looked encoded while execution was not. Encoding exposed a latent gap in the lanes themselves: none of the probe lane planners, nor the join's early overlay-free admission, checked for a history range. SPARQL has no history-range form, so the lanes had never met one; two existing history tests (OPTIONAL and FILTER NOT EXISTS under from/to) regressed to current-state answers once JSON-LD reached the lanes. The planners and the join now decline under from_t, ahead of the overlay-free return, and those two tests pin it. Regression test: the same two-pattern join through JSON-LD and SPARQL must both report use_batched on the join's routing event; it fails with the previous lowering. The two parse unit tests that pinned the deferred form now expect the SID for a registered prefix and the IRI for an unregistered one.

aaj3f

This is a good catch @bplatz, and the fix seems the obvious one to me (compared to, say, teaching the fast-path about Ref::Iri etc). Approving w/ some small notes below:

The diagnosis is the good part: two front ends producing different IR for the same query, with explain carrying its own IRI-to-SID normalization so that every explained JSON-LD plan looked encoded while execution never saw a SID — that's the combination that keeps a fast path silently unreachable since the v4 baseline. The fix is the right one too, and for a reason beyond "it works": it removes the divergence instead of compensating for it, and I checked that the new lower_ref_term is the literal same expression as SPARQL's lower_iri_ref (fluree-db-sparql/src/lower/term.rs:301-308) rather than merely the same idea. The history gate is what earns the approval, though. I mutated it out and ran grp_query_history: 62 pass, and exactly the two tests you name go red — history_optional_emits_retracts_inside_optional_block and history_filter_not_exists_inner_inherits_history — so the pins are load-bearing and not decorative. I also traced that ctx.from_t.is_some() is an exact proxy for history mode in both directions (dataset_query.rs:745-748 → view/dataset.rs:89-91 → temporal_mode.rs:100), and walked every Ref::Iri consumer in the query crate for the blast radius; all symmetric except the annotation-edge probe, which turns out to be independently gated at optional.rs:2038-2043. My two notes are both optional and neither changes the verdict: a JSON-LD cross-ledger join test would pin the other path encoding newly reached, and I'd add a sentence to the body recording that the annotation-edge probe was checked.

Adherence to repo commitments:

Patterns/abstractions: ✔ Converges JSON-LD onto SPARQL's existing lowering rather than adding a parallel encode step — which matters, since at least four operators already carry their own Ref::Iri encode and the alternative design would have multiplied them. Honors the shared-IR model directly.
Performance (speed first, memory second): ✔ No performance-degradation risk, and the reason is structural: the gate can only decline lanes this same PR made reachable (JSON-LD never reached them before; SPARQL has no history-range form), so the set removed is exactly the set added. New per-query cost is one encode_iri_strict per constant at lowering and an Option::is_some() on a context field at lane-plan time — no per-row work, no allocation, no lock.
Testing: ✔ The new pin is wired into grp_query.rs:8-9, so it isn't dead under autotests = false; it runs (1 passed, 5.52 s); the two rewritten parse assertions pass inside 1578 green query-crate lib tests; and the history gate's pins provably go red under mutation. testsuite-sparql green, so the W3C suite hasn't moved.
Conventions: ✔ Self-describing subject, a body that reasons from mechanism to consequence and separates what changed from what it exposed, clippy and fmt green in CI, no stray unwrap/anyhow.

Verified locally at branch HEAD f81a877e4: gate mutation → grp_query_history 62 passed / 2 FAILED (the two named tests), restored byte-identical → 64 passed / 0 failed; grp_query -- jsonld_join_batched 1 passed; fluree-db-query --lib 1578 passed; gh pr checks 1856 all green including testsuite-sparql. (--all-features wouldn't build on my machine — a local cxx C++-headers issue, not this branch.)

Approving so you can merge whenever you're ready — the cross-ledger JSON-LD test is the one note I'd actually like to see land with it, since it's the only newly-reached path without a pin of its own.

aaj3f · 2026-09-16T15:53:47Z

+        // nested-loop join's batched probe lanes above all — test for the SID
+        // form, so leaving every constant as an IRI silently kept JSON-LD
+        // joins off those lanes. Cross-ledger execution re-encodes pattern
+        // SIDs per graph (`reencode_sid`), exactly as it does for SPARQL.


Optional. This is more of a question than a suggestion, and it's about the body rather than the code. The line "Cross-ledger execution already re-encodes pattern SIDs per graph (reencode_sid), exactly as it does for SPARQL today, so JSON-LD is no worse than SPARQL on datasets" is accurate — I read property_path.rs:1489-1491 and context.rs:472-480 and it holds — but "no worse than SPARQL" is doing quiet work: it's also a behaviour change for JSON-LD, from "encode fresh against each target graph" to "re-encode the primary graph's SID, falling back to the raw SID when undecodable."

I don't think that fallback is reachable in a way that matters, because encode_iri (unlike encode_iri_strict) falls back to a full-IRI SID rather than returning None, so reencode_sid essentially always succeeds and the .or_else(|| Some(s.clone())) is close to dead. So this isn't a correctness worry.

It is a coverage one, though. You gave the history path two pins because encoding newly reached it; the cross-ledger dataset path is the other thing encoding newly reached, and it's pinned only by "SPARQL has always done this." A single JSON-LD-over-a-two-ledger-dataset join, asserting the same rows as the SPARQL spelling, would close that gap and would have caught it if the re-encode had gone the other way.

Minor and non-blocking — but if you agree it's right, I'd rather see it folded in now than lost in the backlog.

Addressed in 87517ed

aaj3f · 2026-09-16T15:53:48Z

    ctx.allow_unfiltered()
 }

+/// True under a history-range query. The probe lanes read current leaflet


Optional — and this one is genuinely just a note for the body. Claude and I chased "what else did encoding unlock?", since that's the question your own history discovery teaches a reader to ask, and the answer turns out to be reassuring but non-obvious enough to be worth a sentence.

There are exactly three places in the query crate where the Ref::Iri arm declines while Ref::Sid proceeds: optional.rs:1937, annotation_edge_probe.rs:167, and r2rml/sql_lane/lower.rs:421. The r2rml one is a false alarm — parse_triple at lower.rs:3591-3597 decodes a Ref::Sid back to an IRI before building SubjRef, so both forms land in the same arm. The other two are the annotation-edge probe, and EdgePos::from_ref's comment ("None … for cross-ledger Iri refs, which this single-ledger fast path cannot probe") reads exactly like the history gap did — a guard that only held because JSON-LD always produced Iri.

It isn't one. The operator's own build_batch declines on self.planning.is_history() || ctx.dataset.is_some() || ctx.is_multi_ledger() at optional.rs:2038-2043, so history and every cross-ledger case are caught independently of the ref form, and the Ref::Iri arm is belt-and-braces.

I'd only suggest a line in the body saying the annotation-edge probe was checked and is independently gated. Right now a careful reader gets told about one newly-reached lane family and has to go find out for themselves whether there's another — and the reason there isn't a problem lives three files away from anything in this diff.

Commenting here because optional.rs and annotation_edge_probe.rs are not in this diff.

Addressed in 87517ed

aaj3f · 2026-09-16T15:53:48Z

+    let view = fluree.db(LEDGER).await.expect("db");
+    let (store, guard) = init_test_tracing();
+
+    let jsonld = json!({


Praise. Asserting on the join's use_batched routing event rather than the answer is the right instinct, and the comment says why in one line: "Pinned by the join's routing event, not the answer — the per-row path computes the same rows." Running JSON-LD and SPARQL through the same loop with the same assertion is what makes it a parity test instead of a JSON-LD test, and it's the shape I'd want copied the next time a fast path is surface-specific.

I confirmed it actually runs, too — it's wired into grp_query.rs:8-9, which matters here since fluree-db-api sets autotests = false and an unwired it_*.rs would compile nowhere.

aaj3f · 2026-09-16T15:53:48Z

+/// retract event in the window with its `t` and `op`, which only the scan's
+/// history mode produces. Checked before the overlay-free return, which
+/// would otherwise admit the lane on a clean graph.
+pub(crate) fn probe_lane_history_declines(ctx: &ExecutionContext<'_>) -> bool {


Praise. "Checked before the overlay-free return, which would otherwise admit the lane on a clean graph" is the whole finding compressed into one clause, and it sits right next to the policy gate that was placed for the same reason. Someone adding a fourth gate here in a year will get the rule for free.

For the record, this gate is load-bearing and I checked it rather than assuming: neutering it (plus the join's) and running grp_query_history gives 62 passed / 2 FAILED, and the two failures are exactly the tests you name — history_optional_emits_retracts_inside_optional_block and history_filter_not_exists_inner_inherits_history. Restored, it's 64/0.

aaj3f · 2026-09-16T15:53:48Z

 ) -> Result<Ref> {
    match term {
        UnresolvedTerm::Var(name) => {
            let var_id = vars.get_or_insert(name);


Praise. The comment explains not just what the change does but what the old behaviour silently cost — "leaving every constant as an IRI silently kept JSON-LD joins off those lanes." That's the sentence that would have prevented the bug from lasting since the v4 baseline, and it's now in the file where the next person will read it.

I also read this against SPARQL's lower_iri_ref (fluree-db-sparql/src/lower/term.rs:301-308) rather than taking the parity claim on the description: it's the same encode_iri_strict, the same SID-else-IRI shape. Literally parity, not approximately.

zonotope

Nothing blocking, but I think the code could be cleaned up a bit.

zonotope · 2026-09-16T16:43:21Z

+        UnresolvedTerm::Iri(iri) => Ok(match encoder.encode_iri_strict(iri) {
+            Some(sid) => Ref::Sid(sid),
+            None => Ref::Iri(iri.clone()),
+        }),


I think this match should be a shared helper. The justification for this change is, according to the doc comment above, to encode the way sparql lowering does. Duplicating the logic for what to do with the result of encode_iri_strict means that there's the potential for skew later on if we decide to change the encoding functionality at one call site or find a bug that needs fixing later.

This goes beyond just sparql as there are many call sites that duplicate the same logic across our query back ends:

in fluree-db-sparql/src/lower/term.rs at 80, 142, 293, 302

in fluree-db-cypher/src/lower/context.rs at 244 and 261

The cypher calling also has a subtle difference in how it treats namespace 0 even though its docs claim "parity with the sparql lowering", so we might already have the drift I'm worried about.

I think we could add lower_ref and lower_term methods on IriEncoder since all of the affected crates already import that one.

Addressed in 87517ed

IriEncoder::encode_ref / encode_term; all seven sites call it. The Cypher namespace-0 arm isn't drift — bare Cypher names deliberately land under ns 0, which encode_iri_strict rejects by design — so it's now layered over the shared rule rather than beside it.

zonotope · 2026-09-16T17:47:52Z

+        // A history range needs every event with its `t` and `op`; the
+        // batched lanes emit current facts. Before the overlay-free return for
+        // the same reason as the policy gate below.
+        if self.mode.is_history() || crate::fast_path_common::probe_lane_history_declines(ctx) {


both probe_lane_history_declines and mode.is_history are testing the same thing, so this check is redundant

probe_lane_history_declines just checks from_t, and both from_t and mode.is_history come from the same history_time_range match in dataset_query.rs 745 and 924. That's also the only place from_t is set as far as I can tell. I think the mode.is_history check here as well as the one at cyclic_bgp.rs:1105 can go away.

Addressed in 87517ed

zonotope · 2026-09-16T17:50:14Z

+/// would otherwise admit the lane on a clean graph.
+pub(crate) fn probe_lane_history_declines(ctx: &ExecutionContext<'_>) -> bool {
+    ctx.from_t.is_some()
+}


There are from_t checks scattered across the codebase that are checking similar concepts (here as well as all the calls to this new function, fast_union_star_count_all.rs:110, range_semijoin.rs:609,
fast_whole_graph_agg.rs:539). I think this points to a context level predicate where the logic could live in one place replacing all of the hand rolled calls and clearly stating the intent.

ExecutionContext::is_history_range() has a general enough name to fit everywhere while still being descriptive around the intent, and it would eliminate the repetition.

Addressed in 87517ed

zonotope · 2026-09-16T17:51:44Z

    store: &Arc<BinaryIndexStore>,
    pred_sid: &Sid,
 ) -> Result<ProbeLanePlan> {
+    if probe_lane_history_declines(ctx) {


The only caller of this function you're modifying is join.rs:1768, which sits behind the probe_lane_history_declines return at join.rs:1743. I think we can just get rid of it.

Addressed in 87517ed

Resolved one level up: the three planners shared an identical admission preamble and the join hand-rolled a prefix of it, so probe_lane_admission is now the single copy all four call. No per-planner gate remains to be redundant.

…ane admission Review follow-ups on #1856. `IriEncoder::encode_ref` / `encode_term` carry the one rule every surface lowers constants by (SID when the prefix is registered, else the IRI for the scan to encode per graph). JSON-LD, SPARQL's four sites, and Cypher's two now call it; Cypher's documented namespace-0 bare-name addition is layered over the shared rule instead of duplicating it. `ExecutionContext::is_history_range()` names the fact the fast paths had each spelled as `from_t.is_some()` / `is_none()`; `from_t` and the plan's `TemporalMode::History` come from the same `history_time_range()` match, so the cyclic BGP operator's `mode` field, whose only use was a second check of the same fact, is dropped. The three probe-lane planners shared an identical admission preamble and the join hand-rolled a prefix of it; `probe_lane_admission` is now the one copy, with the history gate in it ahead of the overlay-free return. Mutating that gate out fails exactly the two history pins. New: a JSON-LD join over a two-ledger dataset with divergent namespace codes must return the same rows as the SPARQL spelling — the cross-ledger re-encode path encoding at lowering newly reaches. With `reencode_sid` made a no-op it returns no rows.

#1865 made the probe lanes' policy gate per-predicate. `probe_lane_admission` now takes the predicate set and clears it through `policy_lane_for_predicates`, which is exactly the four sites' behaviour: a one-element set is the single- predicate verdict, the star passes its whole set, and the join passes an empty set when it has no batched predicate — which declines under any non-root policy, as its hand-rolled block did. `probe_lane_policy_clears` folds in.

bplatz added bug Something isn't working as expected area:query Query execution, planning, fast paths, overlay, result formatting performance Performance improvement (release notes: Performance) labels Sep 16, 2026

bplatz requested review from aaj3f and zonotope September 16, 2026 12:18

aaj3f mentioned this pull request Sep 16, 2026

fix(policy): empty rule set under an allow default is root; keep raw-row lanes for untouched predicates #1855

Merged

aaj3f approved these changes Sep 16, 2026

View reviewed changes

zonotope approved these changes Sep 16, 2026

View reviewed changes

bplatz mentioned this pull request Sep 18, 2026

JSON-LD lowering still defers object-position constant IRIs where SPARQL and Cypher encode them #1892

Open

bplatz merged commit 5dd113c into main Sep 18, 2026
16 checks passed

bplatz deleted the fix/jsonld-encode-constant-refs branch September 18, 2026 15:14

Conversation

bplatz commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changes

Tests

Relationship to #1855

Follow-up

Uh oh!

aaj3f left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zonotope left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bplatz commented Sep 16, 2026 •

edited

Loading