Wave 4: resolve the eight deferred follow-ups from the #1462 review (#1467-#1474) - #1483
Open
aaj3f wants to merge 11 commits into
Open
Wave 4: resolve the eight deferred follow-ups from the #1462 review (#1467-#1474)#1483aaj3f wants to merge 11 commits into
aaj3f wants to merge 11 commits into
Conversation
…VE/ADD (#1467) The graph-management Transfer arm re-homed a graph by rewriting each flake's flake-level `g`, but an edge annotation also encodes its edge's graph in the `f:reifiesGraph` OBJECT. Rewriting `g` without the object desynced the two, so `EdgeKey::from_reifies_facts` returned `GraphMismatch` and both readers (JSON-LD hydration + attachment indexer) silently dropped the annotation. Wave-3 blocked this with a fail-loud guard; this replaces the guard with a correct re-home. Three cases, keyed off the destination and whether the source is the default graph: - named->named: rewrite the anchor OBJECT (and `g`) to the dest graph. - named->default: DROP the anchor (the default graph carries none). - default->named: SYNTHESIZE one anchor per reifier subject naming the dest. clear_dest now keys on the RE-HOMED source contents so COPY/MOVE stay symmetric (no same-`t` retract+re-assert of an anchor that already names the dest graph). ADD (clear_dest == false) can still merge two bundles that share an explicit reifier `@id` but reify different edges in source and dest, producing a `Duplicate` that drops both annotations. That case is now detected and rejected loudly; COPY/MOVE are immune because clear_dest retracts the colliding dest bundle first. Blank-minted reifiers never collide. Tests replace the wave-3 rejection with a survival matrix: COPY/MOVE/ADD across named->named / named->default / default->named, each asserting a clean `from_reifies_facts` decode with the anchor naming the expected graph, source intact for COPY and gone for MOVE, plus the ADD collision error, COPY/MOVE non-triggering on the same setup, the same-`t` churn (COPY onto an identical annotation), and an indexed round-trip proving both graph anchors survive the seal-time decode in the durable arena.
A stored language-tagged literal was decoded by `lit_to_comparable` to a bare `ComparableValue::String`, dropping the tag. On the eval/FILTER path this made `"x"@en = "x"@fr` collapse to equal (both became `String("x")`), disagreeing with the constant form which lowers to a lang `TypedLiteral` and compares tag-aware. The `EqKind::Lang`/`lang_parts` machinery already handled lang literals correctly but never received stored ones. Route the stored `LangTag` arm to `ComparableValue::TypedLiteral{LangTag}` (reusing the existing variant — no new one, avoiding the exhaustiveness trap; the tag needs no snapshot). To keep the string builtins that accept a language-tagged argument (§17.4.3) transparent to the tag rather than erroring on the new variant, add a narrow `ComparableValue::string_arg` choke-point (same accept-set as CONCAT: plain / xsd:string / lang, but NOT an IRI or a foreign-datatype literal) and route CONTAINS, STRSTARTS, STRENDS, REGEX, REPLACE, SUBSTR, and LANGMATCHES through it. Sites that already read the tag off the binding (CONCAT, UCASE/LCASE, STRBEFORE/ STRAFTER, LANG), normalize via `unwrap_typed_literal` (cast.rs), or already match `TypedLiteral{String}` (geo, temporal, DATATYPE, to_binding) are unaffected; STRDT/STRLANG/IRI correctly keep rejecting a lang literal. Full tag propagation through the builtins (§17.4.3, `UCASE("x"@en)`= `"FOO"@en`) stays deferred. Net W3C register delta: zero.
Two eval-path inconsistencies between the late-materialized `Binding::EncodedLit` path and the Lit/constant path: - An xsd:float is folded to `FlakeValue::Double` at decode (the NUM_F64 fast path in `context.rs`), so `datatype(?f + ?f)` came back xsd:double on the encoded path while the Lit path (`lit_to_comparable`) keeps the float tag. Consult the in-scope `dt_id` in the EncodedLit decode arm and re-tag FLOAT as `ComparableValue::Float` — one integer compare on the hot arm; the numeric / xsd:string fast path stays untouched. - A language-tagged literal decoded to a bare string and read string-truthy in the EBV path, whereas the Lit path (`lit_effective_bool`) raises a type error. Check `lang_id` before decoding and error, so an encoded lang literal has no effective boolean value (§17.2.2). This is a consistency fix aligning the encoded path with the Lit/constant path, not a W3C-conformance claim (RDF 1.0's plain-literal reading is contested; no register test pins it). Foreign-datatype carry (register eq-4) and constant-side typed-literal carry (open-eq-05/06) stay deferred. Tests query a reindexed, novelty-free ledger so scans late-materialize to `EncodedLit` (a trailing insert would silently drop them to the Lit path — §5.4); verified they fail without the fix. Net W3C register delta: zero.
Per SPARQL 1.1 §17.4.1.9, `E IN (E1..En)` is `(E=E1) || … || (E=En)` under OR three-valued logic: true if any element matches by value equality; otherwise an error if any comparison errored; otherwise false. NOT IN is its negation (NOT of an error is an error). eval_in / eval_not_in previously swallowed both error sources — an EqOutcome::TypeError from rdf_term_equal (incomparable datatypes, the primary case) fell into the Ok(Some(_)|None) catch-all, and a demotable element-eval error (e.g. 1/0) was dropped — so a no-match result read as false (IN) / true (NOT IN) instead of a type error. Mirror eval_or's pending_error accumulation: a definitive match still short-circuits and discards any pending error; otherwise a pending error is returned. The synthesized in-list type error is ComparisonError::TypeMismatch, matching =/!= and demoting to unbound in Extend / false in FILTER (catchable by COALESCE). The shipped value-equality behavior (67eca54) is unchanged. Adds eval_in/eval_not_in unit tests and SPARQL + JSON-LD parity integration tests (BIND/BOUND, FILTER, and COALESCE observing contexts, since ASK+FILTER hides error from false). No W3C register delta.
A present blank `mf:result` whose predicates none resolved to content — e.g. `[ ut:graphData <unresolvable> ]` where get_graph_data yields nothing — had result_present == true and was mis-treated as an "expected empty store", passing trivially against a coincidentally-empty store. The existing guard only caught an ENTIRELY-absent result. The manifest parser now records result_has_predicates (whether a blank result node carried ANY outgoing predicate), and the guard bails when a present blank result carried predicates but none resolved to ut:data/ut:graphData/ut:result content. A bare `mf:result []` (zero predicates — the deliberate empty-store expectation used by DROP ALL / update-silent) has result_has_predicates == false and still passes, so the real W3C corpus is unchanged (net-zero live impact). Adds manifest-parser unit tests over a hand-built graph covering the bare `[]`, present-but-content-empty, and IRI-result shapes.
Per SPARQL §13.2 the default graph of a dataset is the RDF merge (a set) of its FROM graphs, so a triple present in two members must be emitted once and COUNT(*) must count it once. The within-ledger/cross-ledger DatasetOperator concatenated per-member output (next_batch) and summed per-member cardinalities (drain_count) — a bag union. Deduplicate emitted triples across members, active only for a current-mode >= 2-member default union (named scopes always resolve to one member; history rows keep per-event assert/retract metadata the dedup key ignores). The dedup reuses DistinctOperator's EqualityNorm hashing so encoded and decoded twins of a value collapse, and runs after provenance stamping so the cross-ledger path keys on IRI-level values. drain_count falls back to counting the deduplicated next_batch stream for that case. Because a pruned emit column would collapse two distinct triples into one row, thread a multi_default_graph flag through PlanningContext: the planner forces EmitMask::ALL on the first scan (the only prunable-mask scan) and declines the fused count/aggregate fast paths (which assume bag cardinality over the union). A default graph named twice collapses to one member at dataset-build time, keeping FROM <g> FROM <g> on the single-graph fast path. FROM NAMED / GRAPH ?g semantics are unchanged (named members stay per-graph). Net W3C register delta: zero.
…1473) Every SPARQL entry method re-lexed the query string: query_with_options built the within-ledger dataset (parse #1) and then lowered it (parse #2); the FROM-default single-ledger route parsed a third time inside the dataset impl to test for a dataset clause. That is 2x for all SPARQL and up to 3x on the FROM path. Parse + validate once, then reuse the AST for both dataset-clause resolution and IR lowering. helpers gains lower_sparql_ast (inject-prefixes-then-lower from an owned AST), inject_default_prefixes, and sparql_ast_has_dataset; parse_sparql_to_ir is now a thin parse-then-lower wrapper. build_within_ledger _dataset_from_ast resolves the clause from an already-parsed AST. The four SPARQL entry methods (query_with_options, query_tracked_with_options, query_tracked_with_r2rml_options, query_view_with_r2rml_options) parse once and thread the owned AST into the dataset path via new prepared entries on query_dataset_with_options / query_dataset_tracked_with_options (their public signatures, with many external callers, stay unchanged). Behavior is identical. The boxed-future boundary that breaks the mutual- recursion Send auto-trait cycle is preserved: only owned data (the AST) is threaded through it, never opaque async recursion. A cfg(test) parse counter plus helpers unit tests assert the single-parse guarantee by construction; the existing dataset/tracked/r2rml suites and the W3C testsuite cover behavioral equivalence. No register change.
…1474) Add rust-toolchain.toml pinning channel 1.97.0 so local dev and CI build on a deterministic toolchain. dtolnay/rust-toolchain@stable ignores the file, so pin the ref itself (@1.97.0) on all four ci.yml toolchain steps and the bench.yml step. Flip -D warnings onto the main clippy job now that the workspace is lint-clean and the pin means lints only change on a deliberate bump. (History note: this commit originally landed paired with a cherry-pick of main's 2c51620 lint chore; that cherry-pick deduplicated away — as planned — when the stack rebased onto the post-merge burndown/wave-3, which now carries the original via origin/main. The two rustfmt hunks for r2rml/rewrite.rs and sort.rs it left unformatted are likewise already clean on the merged base.)
…re tag-aware #1468 carried the language tag through lit_to_comparable, but the sibling EncodedLit decode arm — the default path for a reindexed, novelty-free ledger — still collapsed a stored lang literal to a bare String (FlakeValue cannot carry the tag), so stored-lang `=`/`!=`/IN were tag-aware on the memory/Lit path and tag-blind on the indexed path: `"x"@en = "x"@fr` compared equal, and a constant-vs-stored `FILTER(?l = "x"@fr)` returned zero rows. The engine was also internally inconsistent on one ledger — JOIN/DISTINCT hash lang_id and were already lang-aware where FILTER equality was not. Re-tag from the in-scope lang_id as a lang TypedLiteral — symmetric to the FLOAT re-tag beside it (#1470) and the lang_id check in binding_effective_bool. One integer compare on the hot decode arm; the meta decode runs only for lang-tagged rows. Indexed twins (seed_indexed, range_provider-asserted) cover stored-vs- stored `=`/`!=`, constant-vs-stored `=`, IN, and the JSON-LD pair; verified they fail without the fix.
…d:string; parse-once pinned end-to-end Three review follow-ups on the wave-4 work: - The UPDATE-WHERE path builds its own multi-member default union for `USING <g1> USING <g2>` without setting the #1469 multi_default_graph planning flag. The OPERATOR-level set-merge self-arms on >= 2 members, so end-to-end set semantics already held for the shapes we could construct (verified: a shared triple binds one WHERE solution with or without the flag) — but arming was out of contract with the emit_is_full soundness coupling, which requires the plan to force EmitMask::ALL whenever a multi-default scan can arm dedup (a future pruned-mask plan on this path would over-dedup in release builds), and the fused count/aggregate fast paths assume bag cardinality. execute_where_streaming now arms the flag from the dataset shape, giving every caller of the primitive the same planning contract as the query path; single-graph datasets are byte-identical. The new test pins the end-to-end §13.2 set semantics on the UPDATE path (one minted template blank for a triple present in both USING graphs). - string_arg rejected TypedLiteral{Explicit(xsd:string)} while its sibling str_arg_and_lang accepts it, so STRBEFORE(STRDT(x, xsd:string), y) worked where CONTAINS type-errored. The two choke-points now share one accept-set (an explicit xsd:string is a simple literal); foreign datatypes stay type errors. - The #1473 parse-once guarantee was asserted only on the primitives; a lib test now drives the ENTRY methods (buffered no-FROM, buffered within-ledger FROM, tracked) and pins sparql_parse_count == 1 end-to-end. Also corrects the #1472 guard's motivating example in both comments: an IRI ut:graphData object resolves at parse time regardless of file existence (it fails later at load), so the guard's real cases are a nested graphData missing its rdfs:label or a result carrying only non-content predicates.
…narios Six coverage adds from the #1483 review, all confirming behavior that had only been verified by inspection: - ADD with the SAME reifier reifying the SAME edge in source and dest succeeds (the collision guard's false-positive boundary — equal edge signatures dedup cleanly; only genuinely-divergent reifiers error). - named→default MOVE and ADD drop the f:reifiesGraph anchor like the already-covered COPY; MOVE retracts the source, ADD keeps it. - Multi-hop COPY a→b→c re-homes the anchor at each hop — all three graphs hold an independently-decodable annotation naming its own graph. - default→named at the DURABLE-INDEX layer: the SYNTHESIZED anchor (`COPY DEFAULT TO <g>` — the source never had one) survives the attachment indexer's seal-time from_reifies_facts decode; exactly one anchor row / one distinct reified graph in the arena. - Multi-FROM + OFFSET/LIMIT: the §13.2 set-merge sits below the slice, so paging is stable over the deduplicated sequence. - A var-var two-pattern join where both patterns carry the cross-member duplicate on the shared subject joins 1×1, not the bag's 2×2. Docs: the N3 schema-clobber note now covers COPY/MOVE TO DEFAULT (same clear_dest retraction), and with_multi_default_graph documents the one place a multi-FROM union stays a bag (history mode, per-event rows).
bplatz
approved these changes
Jul 31, 2026
bplatz
left a comment
bplatz
left a comment
Contributor
There was a problem hiding this comment.
Approving. The branch is green on everything I could measure locally: cargo clippy --all --all-features --all-targets -- -D warnings is clean, cargo nextest over fluree-db-api / fluree-db-query / fluree-db-transact with --all-features passes 4822 tests, and the full testsuite-sparql W3C run is 36/36 suites green with no stale register entries.
Nothing below blocks the merge — none of it is a reproduced failure. Please triage which of these you think are worth fixing here versus deferring or closing out; happy either way.
1. BatchDeduper turns a streaming path into an unbounded-memory one
dataset_operator.rs:200-270
Every FROM <a> FROM <b> query now accumulates a Vec<Binding> signature per distinct row into a HashMap that lives for the whole scan. SELECT * FROM <g1> FROM <g2> WHERE { ?s ?p ?o } goes from bounded-memory streaming to O(distinct triples) resident, with full Binding clones as keys and no fuel or memory accounting. drain_count compounds it: COUNT(*) over a union abandons the per-member count-only shortcut and materializes the entire distinct set.
The comment notes that hashing mirrors DistinctOperator, which is fair, but DISTINCT is user-requested and visible in the query text; this is implicit on any multi-FROM. Options, cheapest first: store a u64/u128 hash rather than cloned bindings; charge fuel per insertion; document the cliff the way scan_graph_flakes documents its own (stage.rs:1058-1063).
2. The emit-mask / dedup coupling is enforced only by debug_assert!
dataset_operator.rs:518
PlanningContext::multi_default_graph forces EmitMask::ALL in make_first_scan only, but DatasetOperator::scan is built at seven sites. Five are safe — optional.rs:434, where_plan.rs:2599 and fast_group_count_firsts.rs (x2) hard-code ALL, and operator_tree.rs:2450's pruned mask sits behind the fused fast paths this PR declines. Two take a planner-computed mask that can prune:
join.rs:1435/1457—make_right_scan(self.right_emit, ...), fed straight fromemit_mask_for_tripleproperty_join.rs:983— the existence-only{ s: true, p: false, o: false }branch, which failsemit_is_full()by construction
I probed 11 query shapes over 2-member unions (plain join, LIMIT, DISTINCT, ORDER BY, COUNT, three-pattern, OPTIONAL, UNION, and property-join existence-only) with open() instrumented, and every one reported emit_full = true and returned the correct set-merged multiset. So this is latent, not live. But the assert compiles out in release, so if a future plan shape does prune, the symptom is silently wrong cardinality in production and a panic only under test.
Cheapest hardening if you want it: apply the same if planning.multi_default_graph { EmitMask::ALL } in build_scan_or_join's Some(left) branch — one edit, covers both remaining sites — and/or promote the debug_assert! to a returned error.
3. The encoded lang re-tag cannot see overlay / ephemeral lang ids
eval.rs:196-209
The re-tag resolves through ctx.binary_store, the persisted BinaryIndexStore. dict_overlay::decode_meta is the one that handles assign_lang_id()-allocated ephemeral ids; the persisted store's language_tags.get(lang_id - 1) simply misses, returns lang: None, and the value falls through to the old tag-blind String. That would put "x"@en = "x"@fr back to true for a novelty-resident lang literal sitting on top of an indexed ledger — the exact #1468 bug, reappearing as a function of materialization.
extract_lang_tag (string.rs:63-71) has the identical shape, so if this is reachable it is one shared fix. A test that stages a language-tagged literal after indexing would settle it.
4. Smaller items
- Per-row double allocation on the lang re-tag.
decode_metaclones the tagString, thenArc::from(tag)allocates again — two allocations per lang-tagged row per expression evaluation, on what the comment calls the hot decode arm. Alang_id -> Arc<str>cache onExecutionContextwould make it free and also coverextract_lang_tag. drain_count's dedup branch silently forgoes count-only. Worth surfacing inplan_detailsso EXPLAIN says why.temporal_mode.rs:122— an orphaned sentence fragment is left in the doc comment (/// full per-event stream rather than a deduplicated set.).view/query.rs:836—validate_sparql_for_viewstill carries the inlinematch &ast.bodythatsparql_ast_has_datasetwas extracted to replace, and its error string is now duplicated verbatim inquery_r2rml_with_options. Three copies of one rule.stage.rs:1391—Vec::with_capacity(src_flakes.len() + 1)is under-sized for the default -> named synthesis case (one extra per reifier). Cosmetic.- No
docs/changes. Candidates:docs/design/query-execution.mdfor the §13.2 set-merge and its cost, dev-setup/contributing for the toolchain pin and a bump policy, andCLAUDE.mditself — its "CI runs" line now omits-D warnings, and its "Pre-existing Warnings" section is stale now that clippy--all-featuresis clean.
5. One question
COPY <g1> TO <g2> with an explicit reifier @id now leaves the same reifier subject carrying f:reifiesSubject / f:reifiesPredicate / f:reifiesObject in two graphs. The new ADD guard exists precisely because two bundles on one subject make EdgeKey::from_reifies_facts return Duplicate and drop both. transfer_named_to_named_survives_reindex proves the attachment indexer is graph-scoped — is the JSON-LD hydration reader too, or can it ever assemble a bundle by subject across graphs?
The test work here is unusually strong. emit_is_full() as a self-describing soundness check on the builder trait, the zero-column ground-pattern case (seen_empty) that is easy to miss entirely, the OFFSET/LIMIT test pinning that dedup sits below the slice, the reindex-level gate proving the re-homed anchor survives into the durable arena, and the thread-local parse counter asserting parse-once through the real entry methods rather than the primitives. The IN / NOT IN pending-error handling is right, including a definitive match dominating a pending error. And pairing the toolchain pin with -D warnings in the same commit is the correct order to do those two things in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 4: resolve the eight deferred follow-ups from the #1462 review
Stacked on #1462 (
burndown/wave-3). Resolves all eight issues filed out of that review: closes #1467, closes #1468, closes #1469, closes #1470, closes #1471, closes #1472, closes #1473, closes #1474.Restacked twice onto the moving #1462 head (post-review-fixes, then post-
origin/main-merge). The lint-chore cherry-pick deduplicated away on the second restack exactly as planned — main now provides the original2c5162014through #1462's merge — so the branch is 8 issue commits + 3 review-response commits (see the new section below), with the toolchain-pin commit standing alone (its message records the history).Each issue got a code-verified audit before design (issue claims checked against current code; three issue texts had stale refs or wrong framing — corrected below), an adversarially-reviewed plan, and an adversarially-reviewed implementation. One commit per issue; the two
chorecommits at the tip are a coupled pair (see #1474 below) — don't bisect between them.What each commit does
fix(transact): reification-aware f:reifiesGraph re-homing for COPY/MOVE/ADD (#1467)— Whole-graph Transfer now re-homes annotation bundles instead of rejecting them: named→named rewrites eachf:reifiesGraphobject to the dest graph Sid, named→default drops the flake, default→named synthesizes one. The wave-3 fail-loud guard is gone, replaced by a round-trip acceptance matrix (COPY/MOVE/ADD × all three graph-direction cases × JSON-LD@annotation+ SPARQL{| |}survival, decode with noGraphMismatch, source intact for COPY / gone for MOVE). One new fail-loud case discovered during plan review ships with it:ADDwhere the same explicit reifier@idreifies a different edge in source and dest would silently merge bundles and drop both annotations — that is now detected and rejected.clear_destuses a symmetric rehomed-contents model, so COPY onto an identical annotation leaves the already-correct dest anchor in place rather than churning retract+assert at the samet.fix(query): carry language tag through lit_to_comparable (#1468)— Stored lang-tagged literals now reach FILTER evaluation asTypedLiteral{LangTag}instead of bareString, so stored"x"@en = "x"@fris false and"x"@en = "x"@enis true (§17.4.1.7), matching what the constant path already did. The string builtins that must stay tag-transparent go through a narrow string-arg helper that accepts plain and lang-tagged strings only — deliberately notas_str(), which would have silently accepted IRIs and foreign-datatype literals where the spec requires type errors; regression tests pin IRI/foreign-dt args as type errors at every swept site. Builtins that were already tag-correct off theBinding(CONCAT, UCASE/LCASE, STRBEFORE/STRAFTER, LANG) are untouched.fix(query): tag encoded xsd:float and error on encoded lang EBV (#1470)— TheEncodedLit(binary-store) decode arm now consultsdt_id: a storedxsd:floatbecomesComparableValue::Float, so?f + ?fon an indexed float yieldsxsd:float(wasxsd:double). The EBV arm errors onlang_id != 0, matching the constant/Lit path. The EBV half is a consistency fix, not a W3C-conformance claim — RDF 1.0's plain-literal reading of §17.2.2 would make lang-tagged EBV truthy, RDF 1.1's makes it an error; the two engine paths disagreeing with each other was the actual bug. Tests run against indexed fixtures (memory-backed ledgers never exercise this path).fix(query): three-valued error propagation in IN / NOT IN (#1471)— Completes the O2 work from #1462 (value-equality shipped there; this ships the §17.4.1.9 error rule). The audit found the issue text undercounted: there were two swallow sites per function —EqOutcome::TypeErrorfalling into a catch-all, and demotable eval errors — in botheval_inandeval_not_in. Now a definitive match still short-circuits, but no-match-with-≥1-errored-comparison is a type error (row-drop in FILTER, unbound in BIND, catchable by COALESCE), mirroringeval_or's three-valued pattern.1 IN (1.0)stays true.fix(testsuite): tighten UpdateEvaluationTest empty-result guard (#1472)— The manifest parser now exposes predicate-presence on the result node, so a present but content-emptymf:result(e.g. an unresolvableut:graphData) is treated as a parse problem instead of "expected empty store" — closing a false-pass risk. Bare[]still means expected-empty. No W3C test outcome changes.fix(query): default-graph FROM union is a set, not a bag (#1469)— Per §13.2 the dataset default graph is the RDF merge (a set) of theFROMgraphs. The audit found the bag-union bug in two places, not one:DatasetOperator::next_batchand thedrain_countCOUNT(*) fast path. Both now dedup at(s,p,o)triple identity across default-graph members — armed only when the default union has ≥2 members, so no-FROM, single-FROM, and FROM-NAMED-only paths take exactly the prior code path (fused fast paths and pruned emit masks intact). Implementation review caught that ground/all-constant BGPs (empty solution schema) escaped the dedup and double-counted; the deduper now collapses the empty mapping to at most one solution per scan, with regression tests that fail without it. Adebug_assertties dedup arming to full variable-identity emission so a future dataset path can't silently over-dedup. History-mode unions are excluded (assert/retract rows must not merge). Known, pre-existing limitation (unchanged by this PR): property-path and SERVICE operators build their own scans and bypassDatasetOperator, so they are not deduped over a multi-default union.refactor(api): parse SPARQL once on the within-ledger dataset path (#1473)— The audit confirmed (and slightly worsened) the issue's count: every SPARQL query parsed ≥2×, the FROM/dataset route up to 3×. All four SPARQL entry methods now parse+validate once and thread the AST through dataset-clause resolution and lowering. Behavior identical (validation-error messages included); acfg(test)parse counter pins parse-once on both the FROM and no-FROM paths. The boxed-futureSendboundary is preserved.chore: rustfmt & clippy fixes for new rust 1.97— (historical; no longer on the branch) This was a cherry-pick of main's2c5162014(author preserved; patch-id verified identical), needed because wave-3 branched before it landed on main. It deduplicated away when the stack restacked onto the post-merge #1462 head, as predicted here.chore(ci): pin rust-toolchain 1.97.0, honor pin in CI, deny warnings (#1474)—rust-toolchain.tomlpins 1.97.0 (with rustfmt+clippy components); the five CI toolchain steps get explicit@1.97.0refs (dtolnay/rust-toolchain@stableignores the toml — the floating@stablewas the root cause of the surprise lint-debt this issue was filed about); the main clippy job now runs-D warnings. (The formerly-coupled cherry-pick has deduplicated away — see above — so this commit now stands alone.) One knock-on worth knowing:release.ymlinstalls Rust via barerustup(latest stable), but the new rootrust-toolchain.tomlnow transitively pins release builds to 1.97.0 too — previously they floated with stable. No conflict (cargo-dist ships as a prebuilt binary), just a behavior change to be aware of.Review-response commits (from the pre-review of this PR)
Three commits at the tip resolve the pre-review's findings on this PR's own work:
fix(query): re-tag encoded language-tagged literals so indexed =/!= are tag-aware— the pre-review's one blocker: query: datatype-aware =/!= for stored language-tagged literals (§17.4.1.7) #1468's tag carry stopped at theBinding::Litpath, so stored-lang=/!=/INwere tag-aware on the memory path and tag-blind on the indexed (EncodedLit) path — the production-typical one ("x"@en = "x"@frcompared equal; constant-vs-storedFILTER(?l = "x"@fr)returned zero rows). The decode arm now re-tags fromlang_id, symmetric to query: xsd:float first-class on the EncodedLit (binary-store) path #1470's FLOAT re-tag beside it; indexed twins (seed_indexed,range_provider-asserted) cover=/!=/IN/constant-vs-stored on both surfaces and fail without the fix.fix(query): set-merge multi-USING WHERE unions; string_arg accepts xsd:string; parse-once pinned end-to-end— the UPDATE-WHERE path now arms the query: within-ledger FROM default-graph union should be a set-merge, not a bag #1469multi_default_graphplanning flag forUSING <g1> USING <g2>(the operator-level dedup already self-armed, so this closes theemit_is_fullsoundness-coupling gap and declines bag-cardinality fast paths, with an end-to-end set-semantics test);string_argandstr_arg_and_langnow share one accept-set (an explicitxsd:stringis a simple literal — CONTAINS and STRBEFORE previously disagreed onSTRDT(?x, xsd:string)); and the query: G2 within-ledger FROM path re-parses SPARQL up to 3× (cold-path) #1473 parse-once guarantee is pinned END-TO-END through the entry methods, not just the primitives. Also corrects testsuite harness: tighten the UpdateEvaluationTest empty-result guard #1472's motivating example in the code comments (an IRIut:graphDataobject resolves at parse time — the guard's real cases are a nested graphData missingrdfs:label, or non-content predicates).test(sparql): pin the review's untested re-homing and union-dedup scenarios— six coverage adds, all confirming inspected-only behavior: ADD same-edge/same-reifier succeeds (the collision guard's false-positive boundary), named→default MOVE/ADD, multi-hop COPY a→b→c, the synthesized default→named anchor surviving the durable-index seal, multi-FROM + OFFSET/LIMIT paging the deduplicated sequence, and a var-var join where both scans carry the cross-member duplicate (1×1, not 2×2). Plus two doc notes: the N3 schema-clobber note coversCOPY/MOVE TO DEFAULT, and history mode is documented as the one place a multi-FROM union stays a bag.Issue-text corrections found during audit
open-eq-02,eq-graph-*,quotes-3/4,dawg-lang-3) is blocked on the scan path (matches_datatype_constraint), which no eval-path fix touches;eq-4additionally needs eval-path foreign-datatype carry andopen-eq-05/06a constant-side carry, both deliberately deferred here. A correcting comment will be posted on the issue.drain_count(COUNT fast path), anddataset_operator.rsis not undereval/.[ ut:graphData <unresolvable> ]) was imprecise — an IRI object resolves at parse time regardless of file existence and fails later at load; the guard's true catches are a nestedut:graphDatablank missing itsrdfs:label/ut:graph, or a result carrying only non-content predicates. The code comments now say so (the original commit message keeps the imprecise wording; corrected here rather than rewriting history).Verification
testsuite-sparqlW3C harness (36 suites),cargo fmt --all --check, and CI-exactcargo clippy --all --all-features --all-targets -- -D warningsall green on the final tip (clippy verified cold aftercargo clean -pon every crate in the lint table, ruling out the warm-cache false-negative).