perf(virtual): R2RML/Iceberg virtual-dataset performance program — corpus, pruning, fused aggregation, budget forwarding (consolidates #1475–#1507)

[aaj3f]

perf(virtual): R2RML/Iceberg virtual-dataset performance program — corpus, pruning, fused aggregation, budget forwarding (consolidates #1475–#1507)

This is one of five surviving pull requests in the consolidated virtual-dataset review program, which collapses the #1475–#1529 lineage into five reviewable units at the maintainers' request. #1507 is the base of the readable stack (base = main) and carries the entire R2RML/Iceberg virtual-dataset performance program: the benchmark corpus and harness, the correctness-first fixes, the decode-wall and cold-floor levers, star/parent-lookup pruning, fused aggregation over FK joins, numeric pruning, scan-side top-k, and the F17 UNION/BIND budget forwarding at the tip. It is the one place besides #1520 that receives real CI once re-based to main. Reading order across the program: #1520 (standalone MoR correctness fix) → #1507 (this PR) → #1514 (serverless Lambda-usability + BYO-IAM) → #1528 (big-Iceberg-audit implementation) → #1529 (native-twin materialize builder). The outward-facing audit record is on the docs/virtual-dataset-audit-2026-07 branch — start with 00-MASTER-AUDIT.md §0, then the empirical results in C2-bench-wave1.md.

Original PR body

Scope expanded by consolidation; the tip PR's own change (F17 UNION/BIND budget forwarding) is the narrative below. The 19 pull requests folded in are rolled up under "Consolidated scope."

What

q029 ({ WebEvent, eventType="purchase" } UNION { WebEvent, eventType="add_to_cart" } LIMIT 100) re-scanned both FACT_WEB_EVENT branches ~253× — 1.94 M file decodes for a 100-row answer — because the LIMIT 100 budget never reached the branch scans. It died at two absorb points: UnionOperator and BindOperator (q029's branch root is BIND(const AS ?et)), both inheriting the trait's no-op set_row_budget; and once branch-1 filled the budget the union still drained branch-2.

Two changes, both gated by FLUREE_R2RML_UNION_BUDGET (default on):

1. Forward the budget through UnionOperator (the full k to each branch — a single branch may supply all k) and BindOperator (to its child; BIND is categorically 1:1/order-preserving, reclassified from absorb to forward). The R2RML scan then caps its materialize window at k.
2. Budget-met branch/input skip (secondary lever): the union stops building further branches / pulling further input once pending_output_rows >= row_budget. Sound on two counts — a budget reaches the union only under row/order-preserving operators up to the LIMIT (Sort/Distinct absorb ⇒ arbitrary-k-subset / rows_only), and the check fires only when the budget is actually MET (an under-filled branch-1 still runs branch-2).

Result

q029: 125 s (pre-slate) → 2.61 s in-protocol (3-rep per-head baseline, F17 + the #1503/artifact cache stack) — under the ≤3 s bar; burndown row closes. Deterministic, network-invariant evidence (live gate): scan_table 253 → 2, parquet decodes 1,940,510 → 7,703 (2 sweeps → 1), OFF byte-preserved, oracle-parity 0-mismatch. Full-corpus baseline: 59 records, 0 hash mismatches.

Cold isolated first-ask is ~57 s-class and is reported, not gated: the residual is one full FACT_WEB_EVENT sweep, S3-fetch-bound (decode trivial). Wall model: ~7 s fixed (loadTable+oauth+scan_plan) + N_sweeps × one-sweep-fetch; the union is sequential, so the lever removes one whole sweep. This is the shared PR-2a floor (q016/q031/q056) — fetch-rate-sensitive, not decode.

Honest notes

• NOT byte-identical (a sketch prediction, refuted + corrected in-tree): eventType is a post-decode residual and the scan window is geometric-growth, so the budget materializes a different valid first-100 → rows_only (same class as q055/q057). Oracle parity holds.
• Blast radius = q029 only (q042 is UNION without LIMIT; q029 is the sole BIND-under-LIMIT query in the corpus).

Perf violations at this head — documented, not blessed away

The 3-rep baseline flagged 5 pre-existing perf violations (q002/q004/q024/q030/q043, 2.5–8× over blessed). Attributed and NOT caused by F17:

q	blessed	observed	ratio	load_table (GET)
q002	1136 ms	2827 ms	2.49×	1.77 s
q004	415 ms	3320 ms	8.00×	1.52 s
q024	510 ms	2828 ms	5.55×	1.22 s
q030	1221 ms	3047 ms	2.50×	0.96 s
q043	2125 ms	12879 ms	6.06×	11.17 s (6 GETs)

• F17 verified inert for all five — none has a UNION or a BIND, so F17's switch-gated code never executes for them.
• fetch_bytes ≈ 0 for every one (files disk-served; artifact cache 90 MB, 5.7× under the 512 MiB cap) — disproves any cache-eviction / regression story.
• The wall is the loadTable catalog GET: ~1.5 s now vs ~0.4 s when the baselines were blessed — a network-path / environmental slowdown of the catalog endpoint. q043 is worst because it does 6 GETs = the edw:name name-dim fan-out (F20-family, pre-existing).
• Not re-blessed (that would lower the bar on a catalog-slow day) — tracked as register F21; re-check catalog latency in a later window.

#1503 scope note (comment for the reviewer): attributing F21 showed the #1503 loadTable-pointer cache is TTL-window-bounded — TTL default 300 s and get_metadata_location prunes on an expired read, so the pointer spares the GET only within ~5 min of a table's last touch (a first-ask after a quiet gap still pays it). Only one .pointer.json (FACT_WEB_EVENT) survived on disk. That is an honest narrowing of #1503's "credential-free first-ask" claim to an intra-window/intra-session win; whether it's purely TTL or also a per-table gap, and how to widen it, is F21's design question (separate sketch + PR). Dated note added to 21-loadtable-metadata-cache.md.

Tests

• Hermetic (fluree-db-query, 13 union tests): BIND forwards; Sort/Distinct absorb (decline proofs); union stores; Limit→Union correctness; 3 build-count lever tests (branch-1 fills k ⇒ branch-2 not built; under-fills ⇒ built; no budget ⇒ both run).
• Full -p fluree-db-query suite: 1256 passed, 0 failed.
• Scan-level effects (early-stop, ON vs OFF counters) are live-gated — only the R2RML scan honors row_budget and union branches are pattern-built, so there is no hermetic mock-scan seam (a #[cfg(test)] injection hook was considered and declined to keep the binary scaffolding-free).

Harness

The live-gate script's OFF arm ceiling was raised 180 s → 300 s (its ~167 s baseline is fragile at 180 — same headroom ratio as q056 168/180). Manifest timeout_s stays 180 (shipped F17-ON q029 is ~2.6 s in-protocol, far under it).

Stacked on #1506 (test/exploration-corpus-family). Commits: sketch → forwarding → doc-correction → lever → baseline-verdict/F21-docs.

Consolidated scope

The lineage below is one linear history; each row's commits are present on this branch's tip and each superseded PR closes as consolidated-at-maintainer-request, its pre-consolidation tip preserved under archive/pre-refactor-2026-07-21/<branch>. Rows are in base→tip order.

PR	What it did
#1475	Benchmark corpus, harness (vbench), and native-blessed baselines for the whole series — the evidence base. No engine-pathway change beyond implementing a reserved bench span layer.
#1476	PR-0 correctness: loud-refuse silently-empty virtual sub-scopes (a transitive path or subquery inside a GRAPH scope over an R2RML source previously "succeeded" with 0 rows; it now errors). Breaking, virtual-only.
#1478	PR-1: a bare, unfiltered COUNT(*) over one class answers from the Iceberg manifest record_count sum instead of decoding every Parquet file (three fact COUNTs DNF→sub-5s), behind a soundness contract that declines on deletes / unproven-nullable / absent stats.
#1482	PR-2: eliminate the per-file Parquet footer round-trips (parse the footer from bytes already fetched) and raise decode concurrency — the fact-scan DNF wall now completes cold.
#1484	PR-3: prune the star TriplesMap fan-out (all-members intersection + disjoint-class hint) so a single-table dimension star stops fanning out to every name-bearing table.
#1485	PR-4: memoize dimension parent lookups across child batches (window-capped, Arc-shared) so a fact⋈dim query stops rebuilding its parent map per 1,000-row batch.
#1487	PR-4b: admit subject-driven R2RML leaves to the batched correlated-OPTIONAL (hash-left-join) instead of the per-row operator rebuild.
#1490	PR-6: fuse GROUP-BY rollups over FK joins directly into the aggregate fold along a linear fact→dim chain, instead of materializing every join binding.
#1491	PR-8: the cold / first-touch floor — parallel catalog resolution, a persisted content-addressed catalog cache (no TTL, no secrets), and 429/503 retry hardening.
#1492	PR-8b: a query-scoped parent-lookup memo so an inner join with an interposed non-pushable FILTER+LIMIT stops re-scanning the parent per driving batch.
#1493	PR-4c: admit the same-subject star to the batched OPTIONAL — closing the last corpus DNF.
#1494	PR-7 (H4): push numeric (double / decimal) FILTER predicates down to Iceberg file / row-group pruning, so a numeric range/equality filter skips data files.
#1495	PR-5: scan-side top-k for ORDER BY DESC(<scan col>) LIMIT k over a single-table scan — read ~k files instead of the whole table.
#1499	F9: CURIE-align virtual graph-source IRIs in sparql_json so virtual output matches native (q002/q042 become byte-identical to the native oracle), gated and kill-switched.
#1509	Docs-only: the F18→F20 investigation record and slate reservations, opened so the trail is reviewed rather than riding to main unreviewed and so the stacked merge has no orphan.
#1501	PR-4d: batched-OPTIONAL seed coalescing (F14) — one inner scan for the whole bounded driving side.
#1502	F20: a RefObjectMap-target resolution prune that kills q031's shared-predicate fan-out.
#1503	The loadTable-METADATA cache — a persisted credential-free pointer plus lazy storage for a zero-REST first-ask, plus the F19 residual.
#1506	The exploration query family (q055–q059) — naive first-touch profiling. Corpus coverage only; additive, no engine change.
#1507	F17 (this PR's tip): UNION/BIND row-budget forwarding + budget-met branch skip (q029 125s→2.6s).

Verification of record

CI does not fire on the individual superseded PRs (their bases were stacked, not main); the verification of record for the program is the corpus benchmark, which runs native and virtual over the identical SF01 source and hash-compares.

Parity is clean: the wave-1 corpus run at the program tip reports 65 ok + 3 expected-error (the lang-tag / custom-datatype error-boundary queries) and 0 unexpected failures, with 0 hash mismatches — native and virtual produce byte-identical results on every deterministic query. 57 of the 65 ok queries land at or under 3 s. The full record, the per-query table, and the run JSONLs are in C2-bench-wave1.md and the deliverable data (native-full-10e073fe9.jsonl, virtual-full-10e073fe9.jsonl) under data/. The wave-2 probe battery reproduced every predicted coverage gap live and proved SPARQL and JSON-LD produce identical results on the same query — recorded in C2b-bench-wave2-probes.md.

The one honest caveat carried in the harness: the committed perf baseline was blessed from an old tip and is gate-blind on 62% of the corpus (28 members were pinned at their DNF timeout wall and 14 were missing), so the perf arm's numbers are re-baselined at the current head rather than trusted from the committed budgets — this is why the tip's five flagged perf violations (q002/q004/q024/q030/q043) are attributed to loadTable-GET catalog latency (tracked as F21) rather than to any engine regression; the correctness/hash arm is unaffected. The stale-baseline analysis is in C2-bench-wave1.md §1 and the red-team recomputation of the headline numbers in RT2-redteam-empirics.md.

Native-impact appendix

The program is overwhelmingly on the R2RML/Iceberg virtual path, with one engine-wide item and one breaking-behavior call-out. Every switch below defaults on and gated-off is byte-identical unless stated.

• F17 UNION/BIND set_row_budget forwarding (this PR's tip) — the one engine-wide edit. The forwarding lands on the generic UnionOperator and BindOperator, which native queries also traverse (BIND is reclassified from absorb to forward). Switch FLUREE_R2RML_UNION_BUDGET (default on; gates BIND too); gated-off is byte-preserved. When it fires it is not byte-identical: a LIMIT without ORDER BY over a UNION/BIND returns a different valid first-k — same count, bounded multiset (the rows_only class), never a wrong count. The native impact is bounded and disclosed: the native corpus legs are byte-identical at every head (0 hash mismatches), and any native LIMIT-over-UNION that hits the reorder is switch-revertible. Deterministic evidence from the live gate: scan_table 253→2, OFF byte-preserved, oracle-parity 0-mismatch, blast radius q029 only — the full mechanism and these scan-count numbers are recorded per the original perf(virtual): R2RML/Iceberg virtual-dataset performance program — corpus, pruning, fused aggregation, budget forwarding (consolidates #1475–#1507) #1507 PR record (perf(virtual): R2RML/Iceberg virtual-dataset performance program — corpus, pruning, fused aggregation, budget forwarding (consolidates #1475–#1507) #1507), in its What / Result / Honest-notes sections, and the program-wide parity backing is the corpus benchmark's 0-hash-mismatch result in C2-bench-wave1.md.
• The Operator::set_row_budget trait default is engine-wide but inert unless a budget is set. It is the shared mechanism the program's budget-forwarding levers ride; on a native query with no row budget it is a no-op, and the native corpus legs are byte-identical as evidence.
• PR-0 (fix(r2rml): PR-0 — loud-refuse silent-empty virtual sub-scopes (breaking: previously silent-empty queries now error) #1476) is a breaking behavior change, but virtual-only. Queries with a transitive property path or subquery inside a GRAPH scope over an R2RML source previously returned 0 rows as success and now raise a typed InvalidQuery. This is scoped to virtual sub-scopes; it is not a native path. It is the one breaking change in the wave and is labeled for the release notes.
• F9 (fix(format): CURIE-align virtual graph-source IRIs in sparql_json (F9) #1499) is a serialization surface change on the virtual path only. It routes virtual Binding::Iri values through the same compact_id_iri compaction native already uses, and it is gated three ways (from_graph_source && FLUREE_R2RML_CURIE_ALIGN && format == SparqlJson). Native results arrive as Binding::Sid and are untouched; only virtual graph-source IRIs in sparql_json are affected.
• Everything else in the wave — the COUNT-manifest shortcut, footer/decode levers, star and parent-lookup pruning, fused rollups, cold-floor caching, numeric-stats pruning, scan-side top-k, OPTIONAL seed coalescing, and the RefObjectMap-target prune — is on the R2RML/Iceberg virtual path.

Confirmed negatives across the whole span (git-checked, name-only diff clean): no arrow / datafusion / parquet dependency bump, and no .github/workflows or rust-toolchain edits anywhere in the lineage. The CI gates and the shared native-dependency surface are untouched.

Merge to main

Re-basing this branch onto main renders the PR conflicting: main advanced 122 commits past the lineage fork and eight files overlap the program (Cargo.lock, Cargo.toml, fluree-db-api/src/query/helpers.rs, fluree-db-api/src/view/query.rs, fluree-db-query/src/context.rs, fluree-db-query/src/execute/operator_tree.rs, fluree-db-query/src/optional.rs, fluree-db-query/src/r2rml/fused_aggregate.rs). The resolution is a single merge-of-current-main commit on the branch — not a rewrite — resolving the hard conflict in optional.rs (the batched-OPTIONAL hash-join-safe predicate arms) and letting Cargo.lock auto-merge, so mergeability goes green and CI runs meaningfully against the change. The merge is authored and adversarially reviewed before it is pushed.

One residual to disclose with that merge: optional.rs also carries the PR-4d (#1501) seed-coalescing path, which interacts with the batched-OPTIONAL predicate the merge resolves. The genuinely new eligibility after this merge is a single-source DefaultGraphSource inner (main's Cypher edge-annotation shape), which now satisfies the hash-join-safe predicate and can therefore seed-coalesce — a path neither parent had; no corpus member covers that shape today, coalescing there is double-guarded (multi-ledger checked at the coalesce decision and again in build_batch, with a per-row fallback), and FLUREE_R2RML_OPTIONAL_SEED_COALESCE is the kill switch. As regression assurance for the pre-existing path, re-run the batched-OPTIONAL corpus members (q016 and q050) and confirm coalescing still fires (one build_batch per driving side, not per window) with both hashes matching their native oracle. The eventual merge to main is a maintainer merge-time item, planned around those eight overlapping files, not a blocker for reviewing this PR as a unit.

[bplatz]

Approving with two definite issues flagged inline — I'd want both resolved before this lands, but neither is structural, so I don't think they need another review round.

Verified locally at this head: cargo test -p fluree-db-query → 1261 passed, 0 failed.

What I checked and can confirm

I concentrated on the engine-wide surface, since that's the part that reaches native queries, and it holds up:

• The forward/absorb classification is correct where it matters. Offset forwards budget.saturating_add(self.offset) — the off-by-offset that would have turned a LIMIT k OFFSET m into a short result isn't there. Join absorbs, with an accurate comment about why a join's output cardinality isn't its left input's. Graph deliberately doesn't forward to self.child.
• BindOperator forwarding is sound. BIND is genuinely 1:1 and order-preserving — an expression error leaves the variable unbound but keeps the row — so the reclassification from absorb to forward is right.
• Float pruning is NaN-safe, which is the classic way this goes wrong. value_codec.rs:50-59 makes a NaN operand yield None rather than Some(false), and bounds_can_contain's unwrap_or(true) turns that into keep. The bounds_can_contain_keeps_on_nan_bound test pins it.
• The top-k k arithmetic is right (LIMIT + OFFSET), and OffsetOperator deliberately does not implement set_topk, so the directive can't cross an OFFSET under-counted. Declining is the safe direction.
• The COUNT(*) manifest shortcut declines on delete manifests independently of #1520's guard, so those two don't conflict.

The two definite issues

1. insert_dim_gkeys treats an equal-value duplicate dim join-key as harmless. It's harmless for group assignment but not for multiplicity — the fused fold under-counts relative to the generic pipeline. Detail inline.
2. PersistedScanFiles carries no delete flag (unlike PersistedCountStats, which does). Once #1520 is on main, that lets the MoR override leak past a process restart. Detail inline.

Three worth a look items inline as well — none blocking.

Two notes on the program, not this diff

• The stated merge order is load-bearing, not just convenient. Issue 2 is contained only because #1520 lands first and #1528's CACHE_FORMAT_VERSION bump lands after. Worth stating that as a constraint on the merge rather than leaving it implicit in the reading order — if #1507 ever landed ahead of #1520, disk entries written by a guard-less build would be served by a guard-ful one.
• On the five flagged perf violations (F21): the attribution to catalog-GET latency convinced me — fetch_bytes ≈ 0 and none of the five contains a UNION or BIND, so F17 is provably inert for them. Agree with not re-blessing on a catalog-slow day.

The consolidation itself reads well, and the #1503 scope-narrowing note in the body — walking back the "credential-free first-ask" claim to intra-window once F21 attribution showed the TTL bound — is the kind of correction that's easy to leave out. Good that it's there.

[bplatz]

Definite issue — the equal-value duplicate arm under-counts.

The conflicting-duplicate decline is right, and the reasoning above it is right. But Some(_) => true keeps a single map entry for an equal-value duplicate, and GroupKeyResolver.map is probed once per fact row, contributing once. The reference pipeline doesn't do that: a fact row joining to two dim rows produces two output rows.

Concretely — join on a non-PK column, group by an attribute:

DIM:  (id=1, region_code='X', region_name='West')
      (id=2, region_code='X', region_name='West')
FACT: joins on region_code, GROUP BY region_name

Both dim rows generate distinct subject IRIs (the subject template keys on id, not region_code), so there's no RDF triple-dedup to collapse them. The generic path emits two rows for each matching fact row, both landing in group West; the fused fold emits one. COUNT(*) comes back at half, and SUM likewise.

The prev != gkeys test is the wrong discriminator: it asks whether the duplicates disagree about which group, but the cardinality bug is present precisely when they agree. What makes an equal-value duplicate safe isn't equal group-keys — it's the join key functionally determining the dim subject, which is what actually collapses the two rows to one.

The conditions are the ones the doc comment already names as reachable (name-based FK inference, hand-written mappings, unverified subject-keys), and SF01's unique PKs are exactly why the corpus can't catch it.

Simplest sound fix is to decline on any duplicate join key, equal-valued or not, unless the join key is known to determine the dim subject. dim_dup_join_key_conflict_declines_equal_dup_kept would need its second assertion inverted — it currently pins the behavior in question.

[aaj3f]

Confirmed — this is precisely the latent defect the audit line had already identified and fixed (decline on ANY duplicate parent key), and your functional-determination framing is the right articulation of why equal group-keys was the wrong discriminator. Forward-ported here in 0b4269a per your ask: the equal-dup arm now declines, the pinning test is inverted and renamed (dim_dup_join_key_always_declines), and the stale outer doc sentence ("harmless and kept") is corrected — it was wrong at both rungs. The function body, in-body comment, and test are byte-identical with the audit line's version, so the eventual merges auto-resolve with no divergence.

[bplatz]

Definite issue — this struct needs the delete flag its sibling has, once #1520 is on main.

PersistedCountStats (just below) persists has_delete_manifests. PersistedScanFiles doesn't, and the scan-files hit arm in r2rml.rs rebuilds FileScanTasks from the cached data_files without re-entering plan_scan — which is where #1520's fail-closed guard lives.

For the in-memory cache that's harmless: entries are only written from the guarded miss arm, the key is an immutable metadata_location, and the cache dies with the process. Persisting it breaks the last of those three:

1. Operator sets FLUREE_ICEBERG_ALLOW_MOR_DELETES=1 to read a delete-bearing table.
2. The scan proceeds under the override and writes the file list to disk.
3. Operator unsets the env and restarts, believing the guard is back on.
4. The next query hits the persisted entry, rebuilds tasks around plan_scan, and silently returns deleted rows — with the guard nominally active.

So the override stops being a per-process escape hatch and becomes a durable one, which I don't think is the intent.

I see #1528 closes this (delete flag on the entry, re-check on both hit arms, CACHE_FORMAT_VERSION 2→3). Since #1507 lands first, either pull that flag forward here, or don't persist scan-files entries written under the override. The CACHE_FORMAT_VERSION bump alone doesn't fix it — a fresh post-bump entry written under the override has the same problem.

[aaj3f]

Confirmed as a defect, with one correction to the remediation path: the audit-line fix cannot be pulled forward as-is, because this rung has no MoR guard at all — mor_guard.rs, the override env, and ensure_no_delete_manifests all arrive with #1520, which branched from much newer main, and the cache-arm commit calls those symbols directly. What landed instead (0247c79) is guard-independent and closes the same hole standalone: PersistedScanFiles gains has_delete_manifests (field and serde shape byte-identical to the audit line's, so the schemas converge), stamped from a manifest-list-only detection mirroring what the COUNT path already does at this rung, and BOTH hit arms treat a flagged entry as a miss — a delete-bearing table simply never serves a cached scan plan here, it re-plans. CACHE_FORMAT_VERSION 2→3 rides along, so pre-flag v2 entries are invalidated wholesale rather than deserialized with a default. Net effect on your sequencing concern: no intermediate state of main carries the override-persistence leak in any order — once #1520 and the audit line land, the override-aware guard_cached_scan_files supersedes this conservative arm at the already-anticipated reconciliation point.

[bplatz]

Worth a look — the can_project_distinct_before_sort branch above computes k and can_topk identically but never offers set_topk.

I believe the omission is correct: DISTINCT sits below the sort on that path, so k rows out of the scan can dedup to fewer than k and the pushdown would genuinely lose rows. But nothing says so — both branches now look like near-copies where one has the offer and the other doesn't, which reads as an oversight rather than a decision.

Given this comment block does a good job explaining why ASC declines, it'd be worth a matching line on the other branch explaining why DISTINCT-before-sort declines. Otherwise the obvious "clean up the asymmetry" edit is a silent correctness regression, and no test would catch it — topk_directive_carries_limit_plus_offset only exercises this branch.

[aaj3f]

Comment added in 067e792 stating the decline rationale (DISTINCT below the sort means k scan rows can dedup to fewer than k, so offering set_topk would under-produce — deliberate omission, mirror of the ASC note), so the "clean up the asymmetry" edit now has a written reason not to happen.

[bplatz]

Worth a look — the COUPLING note is the right instinct, but it under-describes the surface it's protecting.

It points at the forward/absorb classification in the Operator::set_row_budget doc, which reads as "an operator either forwards to its child or absorbs." GraphOperator is neither: it absorbs w.r.t. self.child but threads the budget into the per-parent-batch correlated inner subplan (graph.rs:357 and :536). So a union inside a GRAPH scope can receive a budget by a route the absorb-list framing doesn't describe.

I worked through it and I believe it's still sound — the inner subplan's rows flow to the same LIMIT, so budget buffered rows remain a valid k-subset, and each parent batch rebuilds the inner tree with a fresh budget. But that's a second, subtler argument than the one written here, and it's the one a future editor is most likely to miss. Worth a sentence naming the correlated-inner route explicitly, since that's the path where "row/order-preserving all the way up to the LIMIT" isn't literally true.

[aaj3f]

Extended in 8aaa1eb: the note now names the correlated-inner route explicitly (absorbs w.r.t. self.child, threads the budget into per-parent-batch inner subplans) and the second soundness argument — fresh budget per batch, inner rows flowing to the same LIMIT — that a future editor would otherwise have to rediscover.

[bplatz]

Worth a look — DefaultHasher's output isn't stable across Rust releases.

std::collections::hash_map::DefaultHasher is explicitly documented as not guaranteeing a stable algorithm between versions. The full-key collision guard means this is never a correctness problem — a changed hash just misses. But it means a toolchain bump silently orphans the entire on-disk cache: every entry becomes unreachable and un-reclaimable, and the first-ask cold-floor work this PR is built around quietly stops paying off until the cache refills.

Given the cache is persistent and the whole point is surviving across processes, a fixed-algorithm hash would make the file names reproducible. Not blocking — mostly worth knowing so it isn't misdiagnosed as a cache-eviction regression later, which is exactly the story F21 had to rule out.

[aaj3f]

Fixed in 0247c79 by porting the audit line's stable hasher back to this rung (xxh64 seed 0 via the already-in-tree xxhash_rust, plus the golden pin test that locks the file-name derivation), riding the same v3 invalidation as the delete-flag change so the one-time re-key costs nothing extra. File names are now reproducible across toolchains, which also removes the future misdiagnosis-as-eviction scenario you flagged.

Superseded — switching to request-changes on the two definite issues below.

[bplatz]

Two blockers, both detailed inline above:

1. fused_aggregate.rs — insert_dim_gkeys under-counts on an equal-value duplicate dim join-key. Equal group-keys is the wrong discriminator; the cardinality bug is present precisely when the duplicates agree. COUNT(*) returns half.
2. disk_catalog_cache.rs — PersistedScanFiles needs the delete flag PersistedCountStats already has. Once #1520 is on main, the MoR override survives a restart via the disk cache and the scan-files hit arm skips the guard.

Three non-blocking worth a look comments inline as well.

The rest verified clean: cargo test -p fluree-db-query → 1261 passed. Offset forwards budget + offset correctly, Join/Graph absorb correctly, BIND's absorb→forward reclassification is sound, float pruning is NaN-safe, and the COUNT(*) manifest shortcut declines on deletes independently of #1520.

Also worth making explicit: the #1520 → #1507 → #1528 merge order is load-bearing, not just a reading order. Blocker 2 is contained only because of it.

[aaj3f]

Both blockers and all three worth-a-look items are addressed at head 8aaa1eb (four commits: 0b4269a B1, 0247c79 B2+W3, 067e792 W1, 8aaa1eb W2), and the merge-order note is taken with one improvement: rather than stating the constraint, the fixes remove most of it — B1 and B2 are now correct at this rung standalone (B2 via a guard-independent conservative arm, since the guard itself only exists from #1520 onward; the audit line's override-aware version supersedes it at merge). The dim-duplicate and cache regions here are byte-identical with the audit line, so the stack's eventual merges auto-resolve. CI is green at this head.

[bplatz]

All five items resolved at 8aaa1eb, verified locally:

• B1: insert_dim_gkeys now declines any duplicate parent join key, the pinning test is inverted (dim_dup_join_key_always_declines), and the stale "harmless and kept" doc sentence is gone. Function body is byte-identical with the #1528 rung, so the merge auto-resolves.
• B2: the guard-independent route is the right call given this rung has none of #1520's symbols — flag stamped from a manifest-list-only probe, both hit arms decline through servable_scan_files, v3 bump invalidates pre-flag entries. Stricter than the override-aware arm, correct in every merge order, superseded cleanly when the audit line lands. This resolves my merge-order note by dissolving the constraint, which is better than documenting it.
• W1/W2/W3: decline rationale, correlated-inner route note, and the stable xxh64 hasher + golden pin all landed as discussed. disk_catalog_cache.rs diffs empty against the audit line.

Re-verified: cargo test -p fluree-db-query → 1261 passed; fluree-db-api --features iceberg --lib → 771 passed including the new cache/servable tests. CI green at head.