perf(r2rml): coverage widenings — scan-side pruning/budget/IO + fold-side aggregate admission (audit Tier-2)#1522
Draft
aaj3f wants to merge 10 commits into
Draft
perf(r2rml): coverage widenings — scan-side pruning/budget/IO + fold-side aggregate admission (audit Tier-2)#1522aaj3f wants to merge 10 commits into
aaj3f wants to merge 10 commits into
Conversation
…it item 7, F-AUD-5] The pruning backend already evaluates `Expression::In` (both manifest-level `can_contain_file` and row-group `row_group_can_contain`); nothing emitted it. Add a `ScanCmpOp::In` + `ScanValue::Set` arm to the pushdown vocabulary and emit it from (a) `FILTER(?x IN (c1..cN))` and (b) single-var `VALUES` blocks feeding a scalar-column POM (the exactly-one-scalar-POM soundness gate the existing pushdown uses). Whole-or-nothing: a member that can't push declines the entire IN (a partial IN could prune a file a dropped member needs). Bounded by `FLUREE_R2RML_IN_PUSHDOWN_MAX` (=64). An emitted `In` drives file + row-group pruning; it is transparent at the Arrow row filter (a sibling comparison still builds its filter) and the in-engine FILTER/VALUES join stays authoritative. Kill switch: `FLUREE_R2RML_IN_PUSHDOWN` (default on). This commit also introduces the shared scan-side pushdown-vocabulary changes (the `ScanValue` enum arms and the `build_iceberg_filter` -> `scan_value_to_literal` refactor) that items 8/10 build on. Corpus: q069 (FILTER-IN FK-ref parity; scalar prune shown by q070), q070 (scalar VALUES prune-to-zero). FK-IRI IN pruning is a documented follow-on.
…em 8, F-AUD-6] Scan-side top-k was DESC-only: SPARQL orders unbound (NULL) values FIRST under ASC, so a nullable column could hide an unread top-k row. Admit ASC ONLY when the sort column is REQUIRED (non-nullable per the Iceberg schema) — then no NULL rows exist and the ASC mirror (read files by `lower_bound` ascending, stop when the next `lower_bound` strictly exceeds the k-th bound) is sound. The `TopKBound` engine is generalized worst-first (the DESC regression tests are preserved and pass); the direction threads `set_topk(var, k, ascending)` -> `ScanTopK.ascending` -> `TopKConfig`, and the provider re-checks `field.required` before honoring an ASC directive (declining just falls through to the full parallel scan — the authoritative SortOperator applies the exact order + LIMIT). DISTINCT-kill untouched. Kill switch: `FLUREE_R2RML_TOPK_ASC` (default on; OFF is byte-identical to the pre-item-8 DESC-only behavior). Corpus: q071 (ASC ORDER BY LIMIT with a unique tiebreaker -> deterministic, Full-hash).
…g [audit item 10, F-AUD-11]
`ScanValue` had no timestamp arm, so dateTime predicates never reached pruning
even though the Iceberg manifest value-codec decodes timestamp/timestamptz bounds
unambiguously. Add `ScanValue::Timestamp { micros, tz }` (emitted from a
`FILTER` on an xsd:dateTime column; `tz` = whether the literal is timezone-aware)
and a matching `LiteralValue::TimestampTz`. The push is FRAME-MATCHED at build
time — a UTC (tz-aware) literal only against a `timestamptz` column, a naive
(wall-clock) literal only against a `timestamp` column — so the micros are
directly comparable to the file's decoded bounds; any mismatch declines (the
in-engine FILTER enforces). Pruning is MANIFEST-LEVEL ONLY: no row-group
`stat_bounds` timestamp arm is added, because a Parquet INT64 timestamp's logical
unit (milli/micro/nano) is ambiguous from row-group stats alone (A1 §4) — the
`stat_bounds` catch-all keeps every row group for a timestamp predicate.
Kill switch: `FLUREE_ICEBERG_TIMESTAMP_STATS` (default on, mirroring
NUMERIC_STATS). Corpus: q072 (EVENT_TS range on FACT_WEB_EVENT). Prunes when
EVENT_TS is `timestamptz`; if it is `timestamp` (ntz), the frame gate declines and
the FILTER still enforces — correct either way.
…ervable [audit item 11, F-AUD-7]
The engine-wide `set_row_budget` default was a silent no-op. OPTIONAL (left-outer
join) now forwards the budget to its REQUIRED (outer) side — sound because each
required row yields >=1 output row (matched or null-padded) and required order is
preserved, so bounding the outer side to k still yields >=k output for the LIMIT
to truncate; the inner/optional side is deliberately NOT budgeted. This closes
probe-04's measured 68,828x read amplification.
For FILTER/SORT/DISTINCT/HAVING/AGGREGATE/GROUP-AGGREGATE/SUBQUERY the swallow was
replaced with an EXPLICIT documented decline (a debug span) so it is observable in
traces. MINUS also declines: DEVIATION from the item brief ("MINUS forwards to
primary") on soundness grounds — an anti-join's output is a subset of its primary
with multiplicity <=1, so a finite primary budget can under-produce (the identical
reasoning the inner nested-loop join uses to absorb rather than forward,
join.rs:1127).
Kill switch: `FLUREE_R2RML_BUDGET_OPTIONAL` (default on; OFF swallows the budget,
byte-identical results). Corpus: q073 (OPTIONAL budget) + q074 (its plain-LIMIT
control).
… [audit item 12, B1-AppD] The `read_ranges` bounded-concurrency reader was zero-caller AND used `buffer_unordered` (completion-order output that can't be paired back to input ranges). Add `read_ranges` to the `IcebergStorage`/`SendIcebergStorage` traits (default = sequential, order-preserving), override on S3 with the parallel path fixed to `buffered` (ORDER-PRESERVING) and switch-gated, and wire it into both sparse-Parquet coalesced-fetch loops (`io/parquet.rs` and the Send path `io/send_parquet.rs`, replacing the sequential `for` loop that the code's own comment flagged "could be parallelized"). Non-S3 backends (incl. the local-file wrapper) keep the correct sequential default. Kill switch: `FLUREE_ICEBERG_PARALLEL_RANGE_GETS` (default on; OFF = the byte-identical pre-item-12 sequential fetch). No new corpus member — the existing cold members exercise this path; PR-HARNESS's cold-subset re-run is its evidence. Also commits the scan-side gate record (docs/audit-impl/cov-scan-gates.md).
The item-8 ASC top-k `set_topk(var, k, true)` arm (operator_tree.rs) trips the workspace `[lints.clippy] semicolon_if_nothing_returned = deny`; the scan-side clippy run predated the arm, so it landed red on the branch. One-char `;`, no behavior change — surfaced by the fold-side `cargo clippy -p fluree-db-query`.
… 9b, F-AUD-8] Two coverage widenings to the single-`GRAPH`/single-source fused-aggregate fold, both riding the EXISTING `FLUREE_FUSED_R2RML_AGG` master switch. They are committed together because both add fields to the shared `Resolved` struct + its two constructions, so neither compiles without the other. Per A2's kill-switch lesson: switch-OFF reverts BOTH widenings (back to the pre-PR full-materialize decline). [9] MIN/MAX fused admission (F-AUD-8). Adds `AggregateFn::Min`/`Max` to the fold: a new `Fold::MinMax`/`Acc::MinMax` materializes ONLY the candidate object term (no subject IRI, no per-row BindingRow) via the same `materialize_object_from_batch` + `LiteralEncoder` path the FILTER fold uses, and keeps the running extreme by `compare_bindings` — so the result is byte-identical to the generic `agg_min`/ `agg_max`, but STREAMING (O(1) memory) instead of buffering every value. Scoped to numeric (int/long/double/decimal) + date/timestamp PLAIN-LITERAL columns (`minmax_admissible_datatype` + a `TermType::Literal`/no-lang gate); string (collation) and un-annotated (→ xsd:string) columns decline. Grouped + ungrouped. No manifest/column-stats shortcut in this PR (fold-only — the ungrouped-stats shortcut is a documented follow-on). Corpus: q075 (ungrouped) + q076 (grouped, 4 channels), blessed native oracles. [9b] Filtered-COUNT fused admission — the q038 class (D-c5 slice 1.5). The single-table path previously DECLINED any `star_constraints` (a constant-object flag like `isCurrent true`) to avoid an over-count, forcing q038's constrained COUNT to full-materialize at ~7k rows/s (57.6s virtual / 301x native). It now resolves each constraint to a per-row scalar-column check and APPLIES it in the fold, via the SAME `resolve_star_constraint_checks` / `row_satisfies_constraints` machinery the join path (E2) already uses — a constrained COUNT excludes the non-matching (and NULL-column) rows exactly as the materialized answer does. A constraint that does not resolve to a scalar column still declines (no silent over-count), and the COUNT(*) manifest shortcut stays declined for a constraint-bearing plan (it requires `fact_constraints.is_empty()` — the delete- blind `record_count` cannot see per-row matches). Removes the now-obsolete `fold_over_star_constraints` guard + its "must-decline" tripwire test, replaced by `slice_1_5_admits_and_applies_a_single_table_flag_constraint` and `multi_constraint_requires_all_to_match` (AND-semantics). Corpus: q077 multi- constraint COUNT (isCurrent ∧ segment=Enterprise = 50038, blessed native oracle); the existing q022/q061 grouped members remain the over-count tripwires (they now FUSE with the constraint applied — oracle unchanged). Gates + oracles recorded in docs/audit-impl/cov-scan-gates.md (fold-side section).
…s [audit item 14] C2's manifest-backed /info stats route was gated on an empty native shell (`t == 0`), so `get_data_model` lost the virtual model the moment a virtual-dataset shell received any native write (`t > 0`). Item 14 makes routing per-member: a graph-source-registered ledger serves its manifest-backed virtual stats regardless of the native `t`. An empty shell (t == 0) serves virtual-only (unchanged); a HYBRID (t > 0 + a graph source) builds BOTH members and merges — native ledger/commit/index metadata stays authoritative and native-only classes/properties are preserved, while the graph-source member's classes are overlaid by IRI (the graph source wins a shared IRI; a UNION, never a SUM, so no double-count) and its `source` block (snapshot + counts) is attached. Bypasses the response cache (hybrids are rare); a deserialize failure falls back to the virtual model (fail-safe). New default-on switch `FLUREE_R2RML_INFO_MEMBER_ROUTING` (off = the strict t==0 reroute). Keeps the existing `FLUREE_ICEBERG_INFO_COUNT_BUDGET_MS` time-budget semantics. MoR rider (#1520 F-AUD-1): `fetch_virtual_table_row_counts` derives per-table counts from the snapshot summary `total-records`, which OVER-COUNTS a merge-on-read table (it does not subtract position/equality deletes — recognized but not yet applied). It now flags such a table via `mor_guard::summary_indicates_deletes` (zero I/O) into a new `mor-approximate-tables` list on the `source` block, so the count is reported as an honest upper bound rather than silently authoritative — matching the existing `StatsCompleteness.has_delete_files` convention. Consumers that ignore the new key (the /data-model + MCP readers, all null-safe per C-adversarial O4) are unaffected. Hermetic tests: routing default-on parse, the hybrid class-union merge (graph source wins, no double-count, native metadata kept), and the MoR upper-bound flag surfacing. Gate record: docs/audit-impl/cov-scan-gates.md (fold-side).
The MIN/MAX fold replaced its running extreme only on `Ordering::Greater`
(FIRST-on-ties), but the generic `agg_max` (`max_by`) keeps the LAST of equal
maximums. For values that compare Equal yet render differently — a double column
holding both `+0.0` and `-0.0` ("0" vs "-0") — that picked the wrong element and
broke the byte-parity claim. Extracted `minmax_should_replace(is_max, ord)`: MAX
now replaces on `ord != Less` (last-on-ties, == max_by); MIN stays `== Less`
(first-on-ties, == min_by). Candidates are already materialized before the
compare, so replacing on `Equal` costs no extra work. New unit test
`minmax_tie_break_matches_generic_agg` cross-checks the fold against agg_min/agg_max
on the ±0.0 case (fails under the old first-on-ties MAX).
…ure predicate Extract the bare-COUNT manifest-shortcut eligibility (one CountRows fold, no FILTER / GROUP BY / folded constant-object constraints) out of the inline `next_batch` condition into a pure `count_shortcut_eligible` fn, and add a direct unit test. This is the D-c5 soundness line for item 9b: a constraint-bearing COUNT (e.g. `isCurrent true`) MUST decline the delete-blind `record_count` shortcut and count matching rows in the fold instead — previously that decline was only verified by inspection (R-1522). The extraction is behavior-preserving (all existing fused-aggregate tests unchanged); the test pins the decline for the filtered / grouped / constrained / non-CountRows cases, so a future edit can't silently re-admit an over-counting shortcut.
aaj3f
force-pushed
the
perf/audit-coverage
branch
from
July 19, 2026 04:02
3ece996 to
729cd68
Compare
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.
Forest map
PR 3 of 4 in the big-iceberg-audit implementation phase (audit working set
audit-2026-07/00-MASTER-AUDIT.md§6 Tier-2; bundling decisionaudit-2026-07/decisions/DEC-001-pr-bundling.md— both in the team's audit set, not committed to the repo). It bundles the eight Tier-2 coverage widenings — every one an ADMISSION gap where a shape misses a fast-path gate and falls to full per-row RDF materialization (F-AUD-15 ceiling; C2b measured each cliff). The PR is rigidly sectioned along the DEC-001 soundness seam: Section 1 (scan-side) = bounds/prune/budget/IO (does the prune drop only provably-non-matching data?), Section 2 (fold-side) = aggregate/stats (over-count guards, term parity). Between them it closes/advances F-AUD-5, F-AUD-6, F-AUD-7, F-AUD-8, F-AUD-11 and audit items 12 + 14. Every mechanism ships with a kill switch, corpus member(s) with blessed native oracles, and the local gate record (§Verification) — CI does NOT fire here (base ≠ main), so the local reproduction is the verification of record and the live virtual re-bless lands in PR-HARNESS (PR 4).Base =
perf/audit-mem-guards(PR-SAFE-MEM, #1521). Sibling stack: PR-SAFE-MOR (#1520, on main) → integrationperf/audit-tier012→ PR-SAFE-MEM (#1521) → this → PR-HARNESS (leaf, re-bless).Section 1 — scan-side (bounds / prune / budget / IO)
The soundness question is "does the prune / early-termination drop only provably-non-matching data / preserve completeness?" C2b already proved the machinery works when admitted (ASK pruned 7,670→0 files; FILTER-decimal pruned 7,670/7,670); these five widenings admit more shapes to it.
[7] FILTER-IN / VALUES →
Expression::In(966224d6b, F-AUD-5).ScanCmpOp::In+ScanValue::Set, emitted fromFILTER … INand single-varVALUESover an exactly-one-scalar-POM; whole-or-nothing per member; capFLUREE_R2RML_IN_PUSHDOWN_MAX=64. SwitchFLUREE_R2RML_IN_PUSHDOWN. Scoped: FK-IRI IN (probe-02) still declines to prune (the FK-key template reversal is a documented follow-on) — q069 ships as a parity member; the scalar prune win is q070.[8] ASC scan-side top-k (
b58c9c2d3, F-AUD-6). Admits ASC for a REQUIRED (non-nullable) column only (the nulls-first soundness subtlety that declined ASC);TopKBound/plan_topk_readgeneralized worst-first (DESC regression tests preserved). SwitchFLUREE_R2RML_TOPK_ASC. Member q071.[10] Timestamp manifest pushdown (
1a3d0d104, F-AUD-11).ScanValue::Timestamp{micros,tz}+LiteralValue::TimestampTz, frame-matched at build (UTC↔timestamptz, naive↔timestamp). MANIFEST-LEVEL ONLY (no row-group Int64 arm — parquet logical-unit ambiguity). SwitchFLUREE_ICEBERG_TIMESTAMP_STATS. Member q072.[11] Row-budget forwarding (
5db220551, F-AUD-7). OPTIONAL forwards its LIMIT budget to the required side (sound: ≥1 output/required row — closes probe-04's 68,828× read amplification). MINUS DECLINES (does NOT forward): its anti-join output ⊆ primary with multiplicity ≤1, so a finite primary budget can under-produce — the brief's "MINUS forwards" is unsound. FILTER/SORT/DISTINCT/HAVING/subquery get explicit observable declines. SwitchFLUREE_R2RML_BUDGET_OPTIONAL. Members q073 (cliff) + q074 (control).[12]
read_rangesparallel GETs (26bde327a, B1-AppD/F-AUD-16). Wired the zero-callerread_rangesinto the coalesced sparse-fetch loops; fixed a latent bug — the dormant fn used completion-orderbuffer_unordered(would have paired wrong bytes to wrong ranges) → order-preservingbuffered. SwitchFLUREE_ICEBERG_PARALLEL_RANGE_GETS. No new member by design (the cold corpus members exercise it; PR-HARNESS's cold subset is its evidence).Section 2 — fold-side (aggregate admission)
The soundness question is entirely different — over-count guards and term parity. Both fold widenings ride the EXISTING
FLUREE_FUSED_R2RML_AGGmaster switch as WIDENINGS (no new switch); per A2's lesson this is called out explicitly: switch-OFF reverts the widenings too (back to the pre-PR full-materialize decline).[9] MIN/MAX fused admission (
c3c5f4c4e+ tie-break fixe7c32b9c8, F-AUD-8).AggregateFn::Min/Maxnow fold:Fold::MinMaxmaterializes ONLY the candidate object term (no subject IRI, no per-row BindingRow) through the samematerialize_object_from_batch+LiteralEncoderpath the FILTER fold uses, keeping the running extreme bycompare_bindingswith the SAME tie-break asmin_by/max_by(MIN keeps first-on-ties, MAX keeps last-on-ties — so an equal-but-differently-rendered extreme like a double+0.0/-0.0resolves to the exact element the generic aggregate returns) — byte-parity with the genericagg_min/agg_maxby construction, but STREAMING (O(1) memory) instead of buffering every value. Scoped to numeric (int/long/double/decimal) + date/timestamp PLAIN-LITERAL columns (string collation, lang/IRI, and un-annotated → xsd:string decline). Grouped + ungrouped. No manifest/column-stats shortcut in this PR (fold-only — that ungrouped-stats shortcut is a documented follow-on; deletes + filters complicate the per-file bounds). Members: q075 (ungrouped), q076 (grouped, 4 channels), blessed native oracles.[9b] Filtered-COUNT fused admission — the q038 class (
c3c5f4c4e, C2 / D-c5 slice-1.5; the highest single-query payoff in the corpus). q038 = a COUNT with a constant-object constraint (isCurrent true) that blocks BOTH the manifest shortcut AND the fused fold → full materialize at ~7k rows/s (57.6s virtual, 301× native, DNF at SF20). The fused single-table path previously DECLINED anystar_constraintsto avoid an over-count (the D-c5 O1 ship-blocker); it now RESOLVES each constraint to a per-row scalar-column check and APPLIES it in the fold, via the SAMEresolve_star_constraint_checks/row_satisfies_constraintsmachinery the join path (E2) already uses. The D-c5 guard discipline is honored exactly: (a) the decline is removed ONLY for shapes the fold now applies constraints for — a constraint that doesn't resolve to a scalar column (a RefObjectMap object) still declines, never a silent over-count; (b) parity is pinned at two levels — the fused-vs-materialized equality is the corpus native oracle (q038 + the new multi-constraint q077 = isCurrent∧segment=Enterprise = 50038, plus the grouped q022/q061 over-count tripwires, which now FUSE with the constraint applied, oracle unchanged), and the AND-semantics + NULL-column drop are hermetic unit tests (multi_constraint_requires_all_to_match,e2_row_satisfies_boolean_flag); (c) the COUNT(*) manifest shortcut stays DECLINED for a constraint-bearing plan (it requiresfact_constraints.is_empty()— the delete-blindrecord_countcannot see per-row matches). Predicted telemetry shift on q038 (LOCAL native check; live virtual confirmation is PR-HARNESS's re-bless): scan_table n=1 fused, wall from ~57.6s virtual → seconds class.[14]
get_data_modelbeyondt==0+ the/infoMoR rider (50368671e, C1 residual + #1520 F-AUD-1). C2's manifest-backed/inforoute was gated on an empty native shell (t==0), soget_data_modellost the virtual model the moment a virtual-dataset shell took any native write. Routing is now per-member: a graph-source-registered ledger serves its manifest-backed stats regardless of nativet— an empty shell serves virtual-only (unchanged); a HYBRID (t>0+ graph source) MERGES the two members (native ledger/commit metadata authoritative + native-only classes preserved; the graph-source member's classes overlaid by IRI — a UNION, never a SUM, so no double-count — with itssource/snapshot block attached), fail-safe to the virtual model on a deserialize miss. New default-on switchFLUREE_R2RML_INFO_MEMBER_ROUTING(off = the strict t==0 reroute); theFLUREE_ICEBERG_INFO_COUNT_BUDGET_MStime-budget is unchanged. RIDER:fetch_virtual_table_row_countsderived per-table counts from the snapshot-summarytotal-records, which OVER-COUNTS a merge-on-read table; it now flags such a table (viamor_guard::summary_indicates_deletes, zero I/O) into a newmor-approximate-tableslist on thesourceblock, so the count is an honest upper bound rather than silently authoritative — matching the existingStatsCompleteness.has_delete_filesconvention; consumers that ignore the new key (the /data-model + MCP readers, all null-safe per C-adversarial O4) are unaffected. Hermetic tests for hybrid routing, the class-union merge, and the MoR flag.Verification
CI does NOT fire on this PR (base ≠ main; the workflow's
pull_request: branches:[main]filter matches the target). The full gate record is committed atdocs/audit-impl/cov-scan-gates.md(scan-side + fold-side sections) and is the verification of record; the perf arm is ADVISORY until PR-HARNESS re-blesses at the final integrated head (the committed vbench baseline is 62% gate-blind — C2/RT2). The correctness/hash arm is live: all new members carry blessed native oracles.Fold-side run (this PR's delta), all green:
hash_join.rsbase drift reverted, per db-verify-gotchas).-p fluree-db-query --lib --tests --bins --no-deps→ 0 (also fixed a pre-existing scan-sidesemicolon_if_nothing_returneddeny atoperator_tree.rs:3401, so the whole branch is clippy-green);-p fluree-db-api --features iceberg --all-targets --no-deps→ 0 (one pre-existing non-gatingquestion_markwarning on unchanged code).-p fluree-db-query(lib 1306 + all it_/grp_ bins) → 0;-p fluree-db-api --features iceberg→ 0 (incl. the 3 new item-14 tests);-p fluree-bench-virtual --bins→ 33 pass,shipped_corpus_is_valid= 77 members.vbench baseline --expected --targets native-sf01(local ledger, no creds): q075/q076/q077. q077's native run took 133s (dev build — a native-planner cost for a doubly-constrained COUNT; the fused/virtual path applies both constraints in one scan, which is the point).Residuals (carried to PR-HARNESS / follow-ons)
FLUREE_R2RML_INFO_MEMBER_ROUTING; the six scan-side switches + this one are pending SWITCHES.md rows in PR-HARNESS. Items 9/9b ride the existingFLUREE_FUSED_R2RML_AGG(widening — documented in the gate doc + Section 2).