perf(r2rml): admit subject-driven R2RML leaves to the batched correlated-OPTIONAL (PR-4b) - #1487
Closed
aaj3f wants to merge 7 commits into
Closed
perf(r2rml): admit subject-driven R2RML leaves to the batched correlated-OPTIONAL (PR-4b)#1487aaj3f wants to merge 7 commits into
aaj3f wants to merge 7 commits into
Conversation
…uction (PR-4b P1) Audit result: NO live omission — the old produced_vars delegation was complete because scan_filters and consumed_filter reference only vars the pattern itself produces (by documented invariant). De-fragilize anyway: exhaustively destructure all 15 fields (no , so a new var-bearing field is a compile error here until classified) and include the filter vars explicitly rather than by invariant. Output identical for the current struct; completeness test covers all 7 var-bearing fields. Precursor to admitting R2RML leaves into the batched-OPTIONAL hash-join, whose correlation-set computation depends on referenced_vars being exhaustive.
…ted-OPTIONAL (PR-4b) A correlated OPTIONAL whose inner is an R2RML pattern always fell to the per-row build path — inner_pattern_is_hash_join_safe admitted only Triple/Filter/PropertyPath, and the R2RML rewrite converts OPTIONAL inner triples to Pattern::R2rml — so the entire inner operator tree was rebuilt per correlated row (q050: 430+ scans, DNF; this also reset every operator-scoped cache, which is why PR-4's memo could not help here). Admit the NARROW shape: subject-driven scalar-POM / single-valued RefObjectMap leaves (subject_var + object_var + predicate_filter set; no stars, star-constraints, type-var, wildcard predicate, or bound subject — excluded with pointers to the design doc, docs/audit/.../07). Kill switch FLUREE_R2RML_BATCHED_OPTIONAL (default on; off = per-row exactly). Correctness evidence, both halves: (A) live q050 batched result hash == the NATIVE oracle on all 30,000 rows including genuine OPTIONAL-misses (covers the null/miss partition path directly; SF01 has referential integrity so dangling-FK only by code-path identity there); (B) hermetic batched-vs-per-row differential on a mock provider with a GENUINE dangling FK — row-for-row equality incl. the dangling row's miss (covers dangling-FK directly, CI-enforced). Live SF01 (16 cores): q050 DNF@120s -> ok 16.7s, 430+ -> 92 scans; switch-off reproduces the per-row DNF signature. q016 verified declining on the STAR exclusion (its inner fuses to a same-subject star), not object-correlation — future widening targets star admission. Native corpus 54/54, 0 hash mismatches; q005/q033 sentinels ok. Query suite 1204/1204; api suite all bins green. Prerequisite P1 (referenced_vars correct-by-construction) is df5cb35. Composes with PR-4: the parent memo now applies inside the single batched inner.
…oritization memo (WP9) Full-corpus re-measure on the 10-PR stack: 49 ok / 2 dnf / 3 loud-by-design of 54 (was 24 ok / 30 dnf), always-ok queries a median 3.8x faster. Attribution: PR-2 carries 24 of 28 flips + the across-the-board speedup; PR-1 the three COUNTs; PR-4b(+3) q050; PR-0 the silent-wrong conversions. Remaining reds conclusively diagnosed: q031 = H3-family correlated parent re-scan on the INNER-JOIN seam (1+1305=1306 scans, arithmetic-proven; uncovered by PR-4's within-operator memo and PR-4b's OPTIONAL-only fix); q016 = object-correlated OPTIONAL star (07 doc's open widening). Slow tail classified: q008/q009/q032 operator-bound (H6 rollup materialization, now visible post-PR-2), q012/q029 scan-bound. Cold subset re-measured: the ~1.6-2.1s/table OAuth+catalog floor is untouched — PR-8's exact target. Memo recommendation: F9-formatter -> PR-6 rollup fused-agg -> PR-8 cold floor -> q031+q016 (H3 family, P1-gated) -> PR-7+PR-5 as one cold-scoped unit, decided on PR-8's numbers. Fixes q013 corpus bookkeeping (loud-refuse expected, mirroring q034/q051). Supersedes the pre-restart 08 scaffold.
The restack merge onto the reviewed PR-4 head (4b2629f) brings the ancestor per-PR rustfmt commits; this formats the PR-4b-authored optional.rs hunks so cargo fmt --all --check is green at this HEAD.
… (post-restack lint) redundant_closure_for_method_calls fires at -D on the restacked toolchain; same idiom as the existing call at optional.rs:1584. Scoped clippy back to 0 errors.
…ion-invariant doc, family switch, q013→F2 All three optional items from the #1487 review (the blocking own-delta fmt was cleared by the restack's delta-fmt commit), plus the register note: - Binding-VALUE assertion in the hermetic differential (recommended): the batched-vs-per-row comparison now asserts the per-required-row (row, binding) MULTISETS agree, not just row counts + miss sets — a batched path binding ?s to the wrong supplier IRI at the right cardinality now fails. Values compare over the vars the OPTIONAL side contributes; the correlation var is excluded because the two paths carry it differently BY DESIGN (per-row output includes the seeded var, batched partition batches don't re-project it — the partition row index pins the association, and the multiset compare keys on it, so wrong-row partitioning still fails). The first strengthened run surfaced exactly that schema asymmetry, which is why the comment spells it out. - Fusion-invariant sentence on r2rml_leaf_is_hash_join_safe (doc completeness): consumed_filter/scan_filters need no exclusion because filter fusion only folds FILTERs whose operands are all produced by the scan (rewrite.rs::consume_scan_local_filters requires vars ⊆ produced_vars) — the fused filter reads only values carried by the produced row, so it evaluates identically batched or per-row. If that operand rule ever loosens, this admission is where it breaks. - batched_optional_r2rml_enabled routes through the family r2rml::env_switch_enabled helper (the restack brought it in-crate) — ends the third falsy-spelling variant; "no" now disables it like every other R2RML switch. - Findings register: q013 cross-linked to F2 alongside q051 (entry + summary table) — same subquery root, DNF-masked until this stack made it runnable, expected_status bookkeeping in 09-stacked-rebaseline.md §4. Gates: fluree-db-query 1264/1264 (nextest); scoped clippy 0; cargo fmt --all --check green.
aaj3f
added a commit
that referenced
this pull request
Jul 21, 2026
…tion) Author the merge of origin/main (bbf92e8) into the #1507 f17 head (c948f0a) so the consolidated perf-program PR is mergeable and CI-runnable. main advanced 122 commits past the merge-base (a49ac3f). CONFLICT: fluree-db-query/src/optional.rs (the only content conflict; Cargo.lock and 7 other overlapping files auto-merged and were spot-checked). Both sides independently extended the SAME batched-OPTIONAL hash-join admission lane, so `inner_pattern_is_hash_join_safe` was defined twice: - HEAD (lineage / PR-4b #1487, PR-4c #1493, PR-4d): admits R2RML leaves (`Pattern::R2rml` arm) via r2rml_leaf_is_hash_join_safe / r2rml_star_is_hash_join_safe behind FLUREE_R2RML_BATCHED_OPTIONAL[_STAR], plus seed-coalescing (supports_seed_coalescing). - main (Cypher edge-annotation work, 57d9a0d / 4bc0c6c / f77c259): admits single-source `Pattern::DefaultGraphSource` chains and adds the AnnotationValueOptionalBuilder / AnnotationSidecarMaps that answer a value-only reified-edge OPTIONAL by draining the three f:reifies* predicates once instead of per row. RESOLUTION (DEC-004 F2 — preserve BOTH native semantics): * Merged the two `inner_pattern_is_hash_join_safe` bodies into one match carrying BOTH arms — the R2rml arm (from HEAD) and the DefaultGraphSource arm (from main). The variants are disjoint, so neither side's admission changes. This is required, not cosmetic: the auto-merged build_batch (lines ~1226/1237) already relies on the DefaultGraphSource arm (main's dataset guard) AND the R2rml arm (HEAD), and HEAD's supports_seed_coalescing calls the same predicate. * Kept HEAD's R2RML switches + r2rml_leaf/star helpers verbatim. * Kept main's AnnotationValueOptionalBuilder / AnnotationSidecarMaps block verbatim. * Tests: kept BOTH sides' additions — HEAD's PR-4d seed-coalescing differential (pr4d_pull_coalesced_required_*, pr4d_multi_batch_driver_*) and main's annotation_sidecar_maps_preserve_multi_target_values. Auto-merge semantic gap fixed: fluree-db-query/src/r2rml/fused_aggregate.rs (one of the 8 overlapping files) — main added a `cypher_vocab` field to `query::Query`; the lineage's two PR-6 HAVING tests construct Query with exhaustive struct literals, so they needed `cypher_vocab: None` added. This is filling the field the merged struct now requires, not a semantics change. Interaction note (sound, kill-switchable): the merged predicate now makes a single-source DefaultGraphSource inner eligible for PR-4d seed-coalescing (main never had that path). Coalescing is inner-shape-agnostic and byte-identical for any hash-join-safe inner, is double-guarded (the generic build_batch still applies the multi-ledger + dataset+DefaultGraphSource guard), and reverts via FLUREE_R2RML_OPTIONAL_SEED_COALESCE. Gates at this head: cargo check --workspace OK; cargo test -p fluree-db-query 1312/0; cargo test -p fluree-db-api 2871/0; rustfmt --check clean on the resolved + edited files; scoped clippy -p fluree-db-query --no-deps clean.
Contributor
Author
|
Consolidating at maintainer request. #1487 (PR-4b — batched correlated-OPTIONAL admission for subject-driven R2RML leaves) is carried forward in #1507 (the R2RML/Iceberg virtual-dataset performance program), which brings the #1475–#1507 lineage forward as a single reviewable unit. Its verification of record is the program's corpus benchmark, published as C2-bench-wave1.md (native == virtual, 0 hash mismatches). The pre-consolidation branch tip is preserved at tag |
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
1 participant
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.
Companion to #1485 (PR-4), fixing the class PR-4 explicitly deferred: the correlated-OPTIONAL per-row operator rebuild.
Mechanism
inner_pattern_is_hash_join_safeadmitted onlyTriple/Filter/PropertyPath— and the R2RML rewrite converts OPTIONAL inner triples toPattern::R2rml— so every OPTIONAL over a virtual dataset fell to the per-row build: the whole inner operator tree rebuilt per correlated row (q050: 430+ scans, DNF; each rebuild also reset every operator-scoped cache, which is why #1485's memo couldn't help here). This PR admits the narrow shape — subject-driven scalar-POM / single-valued RefObjectMap leaves; stars, type-var, wildcard-predicate, and bound-subject shapes stay per-row with pointers to the soundness analysis (docs/audit/2026-07-virtual-dataset-perf/07-pr4b-batched-optional.md). Kill switchFLUREE_R2RML_BATCHED_OPTIONAL(default on; off = per-row exactly).Prerequisite (own commit,
df5cb353e):R2rmlPattern::referenced_varshardened to correct-by-construction (exhaustive destructure + completeness test) — the correlation-set computation depends on it, and the audit confirmed no live omission existed.Results (SF01, 16 cores)
q016 verified declining on the STAR exclusion (its OPTIONAL inner fuses to a same-subject star) — checked from the admission path, not inferred; future widening targets star admission, and its inner is fact-scale anyway (H1 territory).
Correctness evidence — two halves, deliberately
batched_equals_per_row_on_dangling_fkdrivesbuild_batchand the per-row build head-to-head on a mock provider with a genuine dangling FK — row-for-row equality including the dangling row's miss. A live batched-vs-per-row comparison is impossible on q050 (the per-row side is the thing that DNFs), which is exactly why this hermetic artifact exists rather than a construction argument alone.Suites: query 1204/1204, api all bins green. Composes with #1485: the parent memo now applies inside the single long-lived batched inner.
Stacks on #1485 (PR-4) ← #1484 ← #1482 ← #1478 ← #1476 ← #1475 ← #1450.