diff --git a/docs/audit-impl/cov-scan-gates.md b/docs/audit-impl/cov-scan-gates.md new file mode 100644 index 000000000..c527b367e --- /dev/null +++ b/docs/audit-impl/cov-scan-gates.md @@ -0,0 +1,144 @@ +# PR-COVERAGE — scan-side gate record + +Branch `perf/audit-coverage` (base = `perf/audit-mem-guards` @ 1441e0038). Scan-side +section: audit items 7, 8, 10, 11, 12. Recorded by the scan-side implementer; the +fold-side implementer appends its section below. The single PR body cites this file. + +CI does not fire on this branch (base ≠ main); this is the local reproduction of +record. The **perf arm is ADVISORY until PR-HARNESS re-blesses** at the final +integrated head — the committed perf baseline is 62% gate-blind (C2/RT2). The +correctness/hash arm is live: new members carry blessed native oracles (below). + +Toolchain: env `cargo`/`clippy` (no repo pin). Two known-pre-existing quirks handled +per `db-verify-gotchas`: (1) `cargo fmt` also touches `hash_join.rs` (pre-existing +drift) — reverted, left out of this branch; (2) the `smoke_minio` **example** fails to +compile without `--features aws` (its own doc requires aws) and is broken on the base +too — excluded from the scoped gates via `--lib --tests --bins`. + +## Scan-side gates (verbatim) + +- **fmt:** `cargo fmt` then reverted the unrelated `hash_join.rs` hunk → the 26 + changed source files are fmt-clean (`cargo fmt --check` clean on them). + +- **clippy (query):** `cargo clippy -p fluree-db-query --lib --tests --bins --no-deps` + → no warnings/errors in any item-7/8/10/11 file (env clippy's pre-existing + workspace lints are out of scope per db-verify-gotchas; CI main clippy has no + `-D warnings`). + +- **clippy (api, iceberg):** `cargo clippy -p fluree-db-api --features iceberg + --all-targets --no-deps` → exit 0. + +- **clippy (iceberg, aws):** `cargo clippy -p fluree-db-iceberg --features aws + --lib --tests --bins --no-deps` → clean for item-10/12 files (examples excluded: + `smoke_minio` is aws-doc'd and pre-existing-broken). + +- **test (query):** `cargo test -p fluree-db-query --lib` → **1306 passed; 0 failed**. + Full `cargo test -p fluree-db-query` (incl. grp_* integration bins) → exit 0. + +- **test (iceberg, aws):** `cargo test -p fluree-db-iceberg --features aws --lib` + → **246 passed; 0 failed**. + +- **test (api, iceberg):** `cargo test -p fluree-db-api --features iceberg` → exit 0 + (live/`#[ignore]` tests skipped; hermetic tests pass). + +- **test (bench-virtual):** `cargo test -p fluree-bench-virtual --bins` → exit 0; + the 7 corpus meta-tests (incl. `shipped_corpus_is_valid` = 74 members, + `rows_only_hash_gate_marks_nondeterministic_limits`) pass. + +## New corpus members + blessed native oracles (item 7/8/10/11) + +Promoted from the A3 probe battery; run `vbench run --targets native-sf01` (local +ledger `/Users/ajohnson/vbench/.fluree`, NO credentials — uses the on-disk iceberg +cache), blessed via `vbench baseline --expected`. **Native oracles only** — the live +virtual gate is PR-HARNESS's job. + +| member | item | probe | native rows | hash (12) | exec | hash_gate | +|--------|------|-------|-------------|-----------|------|-----------| +| q069 filter_in_fkref | 7 | probe-02 | 1100 | 67a2035630da | from | full | +| q070 scalar_values_glaccount | 7 | probe-03 | 0 | e3b0c44298fc | from | full | +| q071 asc_topk_order_total | 8 | probe-01 | 10 | 7dc1c655cb0c | from | full | +| q072 timestamp_range_webevent | 10 | probe-07 | 5000 | 498045e221ce | from | rows_only | +| q073 optional_budget_order_customer | 11 | probe-04 | 50 | b86545e2b0e2 | from | rows_only | +| q074 limit_budget_control | 11 (control) | probe-04b | 50 | 6e0f430cd272 | from | rows_only | + +Item 12 (`read_ranges` parallel GETs) adds NO member by design — the existing cold +corpus members exercise the coalesced-fetch path; PR-HARNESS's cold-subset re-run is +its evidence. + +## Notes carried to the PR body / SWITCHES.md (PR-HARNESS) + +New kill switches (all default-on): `FLUREE_R2RML_IN_PUSHDOWN`, +`FLUREE_R2RML_IN_PUSHDOWN_MAX` (=64), `FLUREE_R2RML_TOPK_ASC`, +`FLUREE_ICEBERG_TIMESTAMP_STATS`, `FLUREE_R2RML_BUDGET_OPTIONAL`, +`FLUREE_ICEBERG_PARALLEL_RANGE_GETS`. + +# PR-COVERAGE — fold-side gate record + +Fold-side section: audit items 9, 9b, 14. Recorded by the fold-side implementer; +same toolchain + two known-pre-existing quirks (`hash_join.rs` fmt drift; the +`smoke_minio` aws-doc'd example) as the scan-side, handled identically. + +## Fold-side gates (verbatim) + +- **fmt:** `cargo fmt` then reverted the unrelated `hash_join.rs` hunk (the same + pre-existing base drift the scan-side noted) → the fold-side source files + (`fused_aggregate.rs`, `ledger_info.rs`, `corpus.rs`, `operator_tree.rs`) are + fmt-clean. + +- **clippy (query):** `cargo clippy -p fluree-db-query --lib --tests --bins + --no-deps` → **exit 0**. This run SURFACED + FIXED a pre-existing scan-side + `semicolon_if_nothing_returned` deny at `operator_tree.rs:3401` (item 8's ASC + `set_topk` arm; the workspace `[lints.clippy]` denies that lint and the + scan-side clippy run predated the arm) — a one-char `;`, no behavior change, + so the whole branch is now clippy-green. + +- **clippy (api, iceberg):** `cargo clippy -p fluree-db-api --features iceberg + --all-targets --no-deps` → **exit 0**. One pre-existing `question_mark` warning + at `ledger_info.rs:1626` (the `load_table` match, UNCHANGED by this PR — the + env clippy-1.97 drift per db-verify-gotchas) — non-gating (not in the workspace + deny set; CI main has no `-D warnings`). + +- **test (query):** `cargo test -p fluree-db-query` (lib 1306 + all it_*/grp_* + integration bins + doc-tests) → **exit 0**. Includes the new/updated + fused-aggregate unit tests: `minmax_admissible_datatype_scope`, + `slice_1_5_admits_and_applies_a_single_table_flag_constraint`, + `multi_constraint_requires_all_to_match` (the D-c5 AND-semantics guard). + +- **test (api, iceberg):** `cargo test -p fluree-db-api --features iceberg` → + **exit 0** (live/`#[ignore]` skipped; hermetic tests pass, incl. the 3 new + item-14 `ledger_info` tests: `info_member_routing_default_on_and_falsy_off`, + `merge_virtual_into_native_unions_classes_graph_source_wins`, + `mor_approximate_tables_flag_surfaces_in_source`). + +- **test (bench-virtual):** `cargo test -p fluree-bench-virtual --bins` → **33 + pass**; `shipped_corpus_is_valid` now asserts **77 members** (74 + the 3 + fold-side members below). + +## New corpus members + blessed native oracles (items 9, 9b) + +Blessed via `vbench baseline --expected` against `native-sf01` (local ledger +`/Users/ajohnson/vbench/.fluree`, NO credentials). **Native oracles only** — the +live virtual re-bless is PR-HARNESS's job. + +| member | item | shape | native rows | native oracle (hash 64) | note | +|--------|------|-------|-------------|-------------------------|------| +| q075 minmax_order_total | 9 | ungrouped MIN/MAX (xsd:double) | 1 | 9e28b2469a7fe95b | orderTotal min+max, one implicit group | +| q076 minmax_order_total_by_channel | 9 | grouped MIN/MAX (4 channels) | 4 | ee98f6bfed407816 | one MinMax accumulator per group | +| q077 count_current_enterprise_customers | 9b | multi-constraint COUNT | 1 | 45a3f31410f08193 | isCurrent∧segment=Enterprise = 50038 (= q022's Enterprise current row); native took 133s (dev build; `timeout_s`=300) — the fused fold applies BOTH constraints in one scan | + +The EXISTING members **q022/q038/q061** already gate the ungrouped + grouped +single-constraint constant-object COUNT: their native oracles are the CONSTRAINED +(materialized) counts, so a fused over-count fails them (q061's own comment is the +D-c5 tripwire — its former "MUST decline" premise is now "fuses WITH the constraint +applied", oracle unchanged). + +## Notes carried to the PR body / SWITCHES.md (PR-HARNESS) + +- Items **9 + 9b** ride the EXISTING `FLUREE_FUSED_R2RML_AGG` master switch as + WIDENINGS (no new switch). Per A2's lesson this is documented explicitly: + **switch-OFF reverts the widening too** — it reverts BOTH the MIN/MAX fold AND + the constant-object constraint application (back to the pre-PR full-materialize + decline). `FLUREE_FUSED_R2RML_AGG_JOIN` (the join sub-switch) is unchanged. +- Item **14** adds ONE new default-on switch **`FLUREE_R2RML_INFO_MEMBER_ROUTING`** + (off = the strict `t == 0` reroute, the prior behavior). Needs a SWITCHES.md row. + diff --git a/fluree-bench-virtual/baselines/expected/q069.json b/fluree-bench-virtual/baselines/expected/q069.json new file mode 100644 index 000000000..0966abe73 --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q069.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q069", + "result_hash": "67a2035630da668915a94d6dba25dc2aae6a088f909eb9f4ec492bce2f544b01", + "rows": 1100, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q070.json b/fluree-bench-virtual/baselines/expected/q070.json new file mode 100644 index 000000000..3ac259c7a --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q070.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q070", + "result_hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "rows": 0, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q071.json b/fluree-bench-virtual/baselines/expected/q071.json new file mode 100644 index 000000000..13b77151f --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q071.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q071", + "result_hash": "7dc1c655cb0c8a1c93a9b970b2499530e850e01bec7c2afb7c17e511efa322f2", + "rows": 10, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q072.json b/fluree-bench-virtual/baselines/expected/q072.json new file mode 100644 index 000000000..be53fd485 --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q072.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q072", + "result_hash": "498045e221ce6057c5f6559608955146298d0bfae9967231cea367c6048a785e", + "rows": 5000, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q073.json b/fluree-bench-virtual/baselines/expected/q073.json new file mode 100644 index 000000000..e49061298 --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q073.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q073", + "result_hash": "b86545e2b0e2179a6426bb6fcfa13ae0222187866ee705ff3103a03d2f3e7a94", + "rows": 50, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q074.json b/fluree-bench-virtual/baselines/expected/q074.json new file mode 100644 index 000000000..4493e700a --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q074.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q074", + "result_hash": "6e0f430cd2728d4e8ddf6fe4a2022aa9cc116f3878d021e93a2632e4eec8e7c9", + "rows": 50, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVT6C3DFJJWEWYXWX75QEWR", + "commit": "1441e0038" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q075.json b/fluree-bench-virtual/baselines/expected/q075.json new file mode 100644 index 000000000..75aef8f14 --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q075.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q075", + "result_hash": "9e28b2469a7fe95b33932b59b113d070601f25ef314529db098e795ad9c629ad", + "rows": 1, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVYPZQ3CQ38DA2FA7WR6HDV", + "commit": "26bde327a" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q076.json b/fluree-bench-virtual/baselines/expected/q076.json new file mode 100644 index 000000000..04394dc76 --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q076.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q076", + "result_hash": "ee98f6bfed4078167d7e3da31a3c3dc491faf1047b3d68a5a30a18dff382da9d", + "rows": 4, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVYPZQ3CQ38DA2FA7WR6HDV", + "commit": "26bde327a" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/baselines/expected/q077.json b/fluree-bench-virtual/baselines/expected/q077.json new file mode 100644 index 000000000..41bfe7cda --- /dev/null +++ b/fluree-bench-virtual/baselines/expected/q077.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "query_id": "q077", + "result_hash": "45a3f31410f0819330edd8d241161283e5894d9a30c640e14e863b0962e42c18", + "rows": 1, + "head_rows": [], + "blessed_from": { + "target": "native-sf01", + "run_id": "01KXVZ2AC33A83G0VDBVGP56FK", + "commit": "26bde327a" + } +} \ No newline at end of file diff --git a/fluree-bench-virtual/corpus/manifest.json b/fluree-bench-virtual/corpus/manifest.json index 336f6be29..9205cc7a9 100644 --- a/fluree-bench-virtual/corpus/manifest.json +++ b/fluree-bench-virtual/corpus/manifest.json @@ -1,5 +1,5 @@ { - "corpus_version": 2, + "corpus_version": 3, "description": "vbench corpus: 54 SPARQL queries (design Q01-Q54) derived from 26 BI questions over the ENTERPRISE_DEMO.DW star schema. Every query runs identically against the native materialized ledger (enterprise-sf01) and, once registered, an R2RML/Iceberg virtual target. See docs/audit/2026-07-virtual-dataset-perf/03-corpus-design.md for the feature x hypothesis coverage matrices and expected-rows derivation.", "queries": [ { @@ -1500,7 +1500,7 @@ { "id": "q066", "file": "queries/q066_shipments_by_year_and_method_dataset.rq", - "bi_question": "How many shipments per (year, ship method)? (W4-2 MIXED fact+dim GROUP BY keys \u2014 shipMethod on the fact, yearNum on the date dim; round-3b #7 shape, COUNT-only for a deterministic hash)", + "bi_question": "How many shipments per (year, ship method)? (W4-2 MIXED fact+dim GROUP BY keys — shipMethod on the fact, yearNum on the date dim; round-3b #7 shape, COUNT-only for a deterministic hash)", "tags": [ "aggregate", "count", @@ -1528,7 +1528,7 @@ { "id": "q067", "file": "queries/q067_tickets_by_year_and_priority_dataset.rq", - "bi_question": "How many tickets and total CSAT per (year, priority)? (W4-2 MIXED fact+dim GROUP BY keys with an integer SUM \u2014 priority on the fact, yearNum on the date dim, csatScore is xsd:integer so the SUM is hash-deterministic)", + "bi_question": "How many tickets and total CSAT per (year, priority)? (W4-2 MIXED fact+dim GROUP BY keys with an integer SUM — priority on the fact, yearNum on the date dim, csatScore is xsd:integer so the SUM is hash-deterministic)", "tags": [ "aggregate", "count", @@ -1554,7 +1554,7 @@ { "id": "q068", "file": "queries/q068_orderline_detail_crawl_dataset.rq", - "bi_question": "Show everything about the line-1 order lines (const-object folded onto a `?s ?p ?o` crawl). W4-1b folded-crawl EXECUTION coverage \u2014 the shape that returned zero triples under the ship-blocker; also the ex:order parent-lookup F-16 telemetry probe.", + "bi_question": "Show everything about the line-1 order lines (const-object folded onto a `?s ?p ?o` crawl). W4-1b folded-crawl EXECUTION coverage — the shape that returned zero triples under the ship-blocker; also the ex:order parent-lookup F-16 telemetry probe.", "tags": [ "wildcard", "exploration" @@ -1574,6 +1574,198 @@ "timeout_s": 180, "subsets": [], "exec_path": "from" + }, + { + "id": "q069", + "file": "queries/q069_filter_in_fkref_dataset.rq", + "bi_question": "Coverage (item 7 / F-AUD-5): FILTER(?fk IN (...)) over an FK-ref var — parity member; FK-IRI IN-prune is a documented follow-on (scalar prune shown by q070).", + "tags": [ + "filter_iri", + "join", + "fk_chain", + "bgp_star" + ], + "tables": [ + "FACT_ORDER", + "DIM_STORE" + ], + "class": "fact-touching", + "expected_rows": [ + 0, + 1000000 + ], + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q070", + "file": "queries/q070_scalar_values_glaccount_dataset.rq", + "bi_question": "Coverage (item 7 / F-AUD-5): single-var VALUES over a scalar column (glAccountName) lowers to Iceberg Expression::In → files_pruned>0. Literals match no value → 0 rows (clean prune-to-zero + parity).", + "tags": [ + "values", + "aggregate", + "group_by" + ], + "tables": [ + "FACT_GL_JOURNAL" + ], + "class": "fact-touching", + "expected_rows": [ + 0, + 10 + ], + "order_sensitive": "none", + "timeout_s": 180, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q071", + "file": "queries/q071_asc_topk_order_total_dataset.rq", + "bi_question": "Coverage (item 8 / F-AUD-6): ASC ORDER BY … LIMIT over a required scalar column admits scan-side ASC top-k (unique tiebreaker → deterministic, Full-hash).", + "tags": [ + "order_by", + "bgp_star" + ], + "tables": [ + "FACT_ORDER" + ], + "class": "fact-touching", + "expected_rows": [ + 0, + 10 + ], + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q072", + "file": "queries/q072_timestamp_range_webevent_dataset.rq", + "bi_question": "Coverage (item 10 / F-AUD-11): xsd:dateTime range FILTER over EVENT_TS pushes to MANIFEST-level pruning under frame-match (UTC literal ↔ timestamptz column).", + "tags": [ + "filter_date", + "filter_range", + "bgp_star" + ], + "tables": [ + "FACT_WEB_EVENT" + ], + "class": "fact-touching", + "hash_gate": "rows_only", + "expected_rows": [ + 0, + 5000 + ], + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q073", + "file": "queries/q073_optional_budget_order_customer_dataset.rq", + "bi_question": "Coverage (item 11 / F-AUD-7): a LIMIT above an OPTIONAL forwards the row budget to the required side (closes probe-04’s 68,828× read amplification). Control: q074.", + "tags": [ + "optional", + "join", + "fk_chain", + "bgp_star" + ], + "tables": [ + "FACT_ORDER", + "DIM_CUSTOMER" + ], + "class": "fact-touching", + "hash_gate": "rows_only", + "expected_rows": [ + 0, + 50 + ], + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q074", + "file": "queries/q074_limit_budget_control_dataset.rq", + "bi_question": "Coverage (item 11 control): the same LIMIT shape WITHOUT the OPTIONAL — the budget already forwards into the scan; the q073-vs-q074 delta is the OPTIONAL cliff item 11 closes.", + "tags": [ + "bgp_star" + ], + "tables": [ + "FACT_ORDER" + ], + "class": "fact-touching", + "hash_gate": "rows_only", + "expected_rows": [ + 0, + 50 + ], + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q075", + "file": "queries/q075_minmax_order_total_dataset.rq", + "bi_question": "Coverage (item 9 / F-AUD-8): ungrouped MIN/MAX over a numeric fact column (orderTotal) — folded from the scanned column with byte-parity to agg_min/agg_max, previously a full FACT_ORDER materialize.", + "tags": [ + "aggregate", + "bgp_star" + ], + "tables": [ + "FACT_ORDER" + ], + "class": "fact-touching", + "expected_rows": 1, + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q076", + "file": "queries/q076_minmax_order_total_by_channel_dataset.rq", + "bi_question": "Coverage (item 9 / F-AUD-8): GROUPED MIN/MAX over a numeric fact column keyed by a fact-inline column (orderChannel) — one MinMax accumulator per group in the fused single-table fold.", + "tags": [ + "aggregate", + "group_by", + "bgp_star" + ], + "tables": [ + "FACT_ORDER" + ], + "class": "fact-touching", + "expected_rows": 4, + "order_sensitive": "none", + "timeout_s": 120, + "subsets": [], + "exec_path": "from" + }, + { + "id": "q077", + "file": "queries/q077_count_current_enterprise_customers_dataset.rq", + "bi_question": "Coverage (item 9b / the q038 class, multi-constraint): COUNT with TWO folded constant-object constraints (isCurrent true AND segment=Enterprise) — the fused fold applies both (oracle 50038 = q022's Enterprise row); dropping either over-counts.", + "tags": [ + "count", + "aggregate", + "bgp_star" + ], + "tables": [ + "DIM_CUSTOMER" + ], + "class": "dims-only", + "expected_rows": 1, + "order_sensitive": "none", + "timeout_s": 300, + "subsets": [], + "exec_path": "from" } ] -} \ No newline at end of file +} diff --git a/fluree-bench-virtual/corpus/queries/q069_filter_in_fkref_dataset.rq b/fluree-bench-virtual/corpus/queries/q069_filter_in_fkref_dataset.rq new file mode 100644 index 000000000..b03277908 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q069_filter_in_fkref_dataset.rq @@ -0,0 +1,14 @@ +# q069 filter_in_fkref_dataset — promoted from A3 probe-02 (audit item 7, F-AUD-5) +# Coverage: a bounded FILTER(?fk IN (...)) over an FK-REF var. Item 7 lowers a +# bounded IN/VALUES set to an Iceberg Expression::In for scalar-column POMs; an +# FK-IRI IN (this shape) still declines to prune (the FK-key template reversal is +# a documented follow-on), so here it is a PARITY member (native==virtual) and a +# pruning member once FK-IRI lowering lands. The scalar prune win is shown by q070. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?oid ?tot +FROM +WHERE { ?o a edw:Order ; edw:orderId ?oid ; edw:orderTotal ?tot ; edw:store ?st . + FILTER(?st IN (, + , + )) } diff --git a/fluree-bench-virtual/corpus/queries/q070_scalar_values_glaccount_dataset.rq b/fluree-bench-virtual/corpus/queries/q070_scalar_values_glaccount_dataset.rq new file mode 100644 index 000000000..810b29b18 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q070_scalar_values_glaccount_dataset.rq @@ -0,0 +1,14 @@ +# q070 scalar_values_glaccount_dataset — promoted from A3 probe-03 (audit item 7, F-AUD-5) +# Coverage: a single-var VALUES over a SCALAR-attribute column (edw:glAccountName → +# the GL_ACCOUNT_NAME string column). Item 7 lowers the VALUES set to an Iceberg +# Expression::In, pruning the FACT_GL_JOURNAL scan (files_pruned>0 where C2b +# measured 0). The literals below intentionally match no glAccountName value, so +# the IN-prune prunes ALL files → 0 rows on both engines (a clean prune signal + +# byte-identical parity), mirroring the ASK/FILTER-decimal prune-to-zero controls. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?acct (SUM(?deb) AS ?tot) +FROM +WHERE { VALUES ?acct { "4000 - Revenue" "5000 - COGS" "6000 - Opex" } + ?j a edw:GlJournalEntry ; edw:glAccountName ?acct ; edw:debitAmount ?deb . } +GROUP BY ?acct diff --git a/fluree-bench-virtual/corpus/queries/q071_asc_topk_order_total_dataset.rq b/fluree-bench-virtual/corpus/queries/q071_asc_topk_order_total_dataset.rq new file mode 100644 index 000000000..2e1f565c2 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q071_asc_topk_order_total_dataset.rq @@ -0,0 +1,13 @@ +# q071 asc_topk_order_total_dataset — promoted from A3 probe-01 (audit item 8, F-AUD-6) +# Coverage: an ASCENDING single-column ORDER BY … LIMIT over a scalar scan column. +# Item 8 admits ASC scan-side top-k when the sort column is REQUIRED (non-nullable); +# the scan then reads files in lower_bound-ASC order and stops early (mirroring the +# DESC q046). A unique tiebreaker (?oid) makes the top-10 DETERMINISTIC, so this is +# a Full-hash member. edw:orderTotal is a required column on FACT_ORDER. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?oid ?tot +FROM +WHERE { ?o a edw:Order ; edw:orderId ?oid ; edw:orderTotal ?tot . } +ORDER BY ASC(?tot) ?oid +LIMIT 10 diff --git a/fluree-bench-virtual/corpus/queries/q072_timestamp_range_webevent_dataset.rq b/fluree-bench-virtual/corpus/queries/q072_timestamp_range_webevent_dataset.rq new file mode 100644 index 000000000..dc708b1fe --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q072_timestamp_range_webevent_dataset.rq @@ -0,0 +1,16 @@ +# q072 timestamp_range_webevent_dataset — promoted from A3 probe-07 (audit item 10, F-AUD-11) +# Coverage: an xsd:dateTime range FILTER over the sole dateTime column +# (edw:eventTs → FACT_WEB_EVENT.EVENT_TS). Item 10 adds a Timestamp ScanValue arm +# that pushes to MANIFEST-level pruning when the literal's tz frame matches the +# column (a UTC "…Z" literal against a `timestamptz` column; naive against +# `timestamp`). If EVENT_TS is `timestamptz`, files_pruned>0; otherwise the frame +# gate declines and the in-engine FILTER enforces (still correct). rows_only: +# the LIMIT truncates an unordered matching set. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +PREFIX xsd: +SELECT ?e ?et +FROM +WHERE { ?e a edw:WebEvent ; edw:eventType ?et ; edw:eventTs ?ts . + FILTER(?ts > "2024-06-01T00:00:00Z"^^xsd:dateTime) } +LIMIT 5000 diff --git a/fluree-bench-virtual/corpus/queries/q073_optional_budget_order_customer_dataset.rq b/fluree-bench-virtual/corpus/queries/q073_optional_budget_order_customer_dataset.rq new file mode 100644 index 000000000..26e9d56fb --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q073_optional_budget_order_customer_dataset.rq @@ -0,0 +1,13 @@ +# q073 optional_budget_order_customer_dataset — promoted from A3 probe-04 (audit item 11, F-AUD-7) +# Coverage: a top-level LIMIT above an OPTIONAL (left-outer join). Item 11 forwards +# the LIMIT row budget to the REQUIRED (outer) side — sound because each required +# row yields ≥1 output — so the driving FACT_ORDER scan stops early instead of +# full-scanning. This closes probe-04's measured 68,828× read amplification. Its +# control (no OPTIONAL) is q074. rows_only: the LIMIT truncates an unordered set. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?oid ?tot ?cust +FROM +WHERE { ?o a edw:Order ; edw:orderId ?oid ; edw:orderTotal ?tot . + OPTIONAL { ?o edw:customer ?cust . } } +LIMIT 50 diff --git a/fluree-bench-virtual/corpus/queries/q074_limit_budget_control_dataset.rq b/fluree-bench-virtual/corpus/queries/q074_limit_budget_control_dataset.rq new file mode 100644 index 000000000..415854e6e --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q074_limit_budget_control_dataset.rq @@ -0,0 +1,12 @@ +# q074 limit_budget_control_dataset — promoted from A3 probe-04b (audit item 11 control) +# Coverage: the POSITIVE CONTROL for q073 — the same LIMIT shape WITHOUT the OPTIONAL. +# Here the LIMIT budget already forwards into the R2RML scan (FLUREE_R2RML_LIMIT_ +# PUSHDOWN, default on), so the scan window caps to the remaining budget and stops +# early. The q073-vs-q074 delta is exactly the OPTIONAL budget-forwarding cliff that +# item 11 closes. rows_only: the LIMIT truncates an unordered set. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?oid ?tot +FROM +WHERE { ?o a edw:Order ; edw:orderId ?oid ; edw:orderTotal ?tot . } +LIMIT 50 diff --git a/fluree-bench-virtual/corpus/queries/q075_minmax_order_total_dataset.rq b/fluree-bench-virtual/corpus/queries/q075_minmax_order_total_dataset.rq new file mode 100644 index 000000000..322a82a53 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q075_minmax_order_total_dataset.rq @@ -0,0 +1,12 @@ +# q075 minmax_order_total_dataset — promoted from A3 probe-06 (audit item 9, F-AUD-8) +# Coverage: ungrouped MIN/MAX over a numeric (xsd:double) fact column. Before item 9 +# MIN/MAX declined the fused fold (resolve_agg_folds returned None for anything but +# COUNT/SUM/AVG) and full-materialized the FACT_ORDER star. Item 9 folds MIN/MAX +# directly from the scanned column via the same term-materialization + compare_bindings +# the generic agg_min/agg_max uses (byte-parity), streaming instead of buffering every +# value. Rides the existing FLUREE_FUSED_R2RML_AGG switch (a widening; switch-off reverts). +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT (MIN(?tot) AS ?lo) (MAX(?tot) AS ?hi) +FROM +WHERE { ?o a edw:Order ; edw:orderTotal ?tot . } diff --git a/fluree-bench-virtual/corpus/queries/q076_minmax_order_total_by_channel_dataset.rq b/fluree-bench-virtual/corpus/queries/q076_minmax_order_total_by_channel_dataset.rq new file mode 100644 index 000000000..47733cbb9 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q076_minmax_order_total_by_channel_dataset.rq @@ -0,0 +1,11 @@ +# q076 minmax_order_total_by_channel_dataset — audit item 9 (F-AUD-8), grouped MIN/MAX +# Coverage: GROUPED MIN/MAX over a numeric (xsd:double) fact column, keyed by a fact-inline +# scalar column (orderChannel). Exercises the fused single-table fold with MIN/MAX +# aggregates AND a GROUP BY key together (one MinMax accumulator per group). Native +# oracle = the materialized per-channel min/max; a fused mis-fold would disagree. +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT ?ch (MIN(?tot) AS ?lo) (MAX(?tot) AS ?hi) +FROM +WHERE { ?o a edw:Order ; edw:orderTotal ?tot ; edw:orderChannel ?ch . } +GROUP BY ?ch diff --git a/fluree-bench-virtual/corpus/queries/q077_count_current_enterprise_customers_dataset.rq b/fluree-bench-virtual/corpus/queries/q077_count_current_enterprise_customers_dataset.rq new file mode 100644 index 000000000..83a407eb9 --- /dev/null +++ b/fluree-bench-virtual/corpus/queries/q077_count_current_enterprise_customers_dataset.rq @@ -0,0 +1,15 @@ +# q077 count_current_enterprise_customers_dataset — audit item 9b (the q038 class), +# MULTI-CONSTRAINT case (D-c5 over-count guard). +# Coverage: a COUNT carrying TWO folded constant-object constraints — edw:isCurrent true +# (xsd:boolean) AND edw:segment "Enterprise" (xsd:string). Item 9b removes the +# single-table decline-on-star_constraints guard and APPLIES each constraint per row in +# the fused fold (resolve_star_constraint_checks + row_satisfies_constraints). The oracle +# is the CONSTRAINED count (current Enterprise customers = 50038, i.e. q022's Enterprise +# row): dropping EITHER constraint over-counts (300000 if segment is ignored; all-segment +# Enterprise incl. non-current if isCurrent is ignored). The manifest COUNT shortcut stays +# declined for a constraint-bearing plan. Rides FLUREE_FUSED_R2RML_AGG (switch-off reverts). +# Exec path: query_from() FROM (the deployed solo/chat shape). +PREFIX edw: +SELECT (COUNT(*) AS ?n) +FROM +WHERE { ?c a edw:Customer ; edw:isCurrent true ; edw:segment "Enterprise" . } diff --git a/fluree-bench-virtual/src/corpus.rs b/fluree-bench-virtual/src/corpus.rs index dca279140..6bb094b7a 100644 --- a/fluree-bench-virtual/src/corpus.rs +++ b/fluree-bench-virtual/src/corpus.rs @@ -338,14 +338,19 @@ mod tests { let corpus = Corpus::load(&dir).expect("shipped corpus must validate"); assert_eq!( corpus.queries.len(), - 68, + 77, "full corpus: 54 design queries (Q01-Q54) + 5 exploration (q055-q059) + \ 4 C5 dataset-path members (q060 family-A, q061 family-B over-count trap, \ q062 family-C fact-dim SUM, q063 family-A ORDER BY/OFFSET) + \ 1 E1 shared-predicate member (q064 Product-by-category) + \ 1 E2 join+flag member (q065 orders-by-current-customer-segment) + \ 2 W4-2 mixed fact+dim group-key members (q066 COUNT, q067 COUNT+intSUM) + \ - 1 W4-1b folded-crawl sentinel (q068 orderline detail crawl)" + 1 W4-1b folded-crawl sentinel (q068 orderline detail crawl) + \ + 6 PR-COVERAGE scan-side members promoted from the A3 probe battery \ + (q069 FILTER-IN/F-AUD-5, q070 scalar-VALUES/F-AUD-5, q071 ASC-top-k/F-AUD-6, \ + q072 timestamp-prune/F-AUD-11, q073 OPTIONAL-budget/F-AUD-7, q074 its control) + \ + 3 PR-COVERAGE fold-side members (q075 ungrouped MIN/MAX + q076 grouped MIN/MAX \ + for item 9/F-AUD-8, q077 multi-constraint COUNT for item 9b/the q038 class)" ); // The smoke subset is a cheap, dims-heavy cover of every feature tag. let smoke = corpus.select(Some("smoke")); @@ -455,6 +460,11 @@ mod tests { // unordered (p,o) set of the key-constrained subject's crawl — // rows-only for the same LIMIT-nondeterminism reason. "q068", + // PR-COVERAGE scan-side members whose LIMIT truncates an UNORDERED set + // (no ORDER BY / no unique tiebreaker): q072 timestamp-range LIMIT 5000, + // q073 OPTIONAL-budget LIMIT 50, q074 its plain-LIMIT control. (q071 ASC + // top-k stays Full — its `ORDER BY ASC(?tot) ?oid` tiebreaks uniquely.) + "q072", "q073", "q074", ] .into_iter() .collect(); diff --git a/fluree-db-api/src/graph_source/r2rml.rs b/fluree-db-api/src/graph_source/r2rml.rs index 5b7058518..2bab11130 100644 --- a/fluree-db-api/src/graph_source/r2rml.rs +++ b/fluree-db-api/src/graph_source/r2rml.rs @@ -164,6 +164,23 @@ fn build_iceberg_filter( let Some(field) = schema.field_by_name(&f.column) else { continue; }; + // Item 7 (F-AUD-5): a set-membership filter builds `Expression::In` — the + // backend keeps a file iff ANY member could lie in its column bounds + // (pruning.rs). It is pushed whole or not at all: if a single member + // cannot be represented against this physical column, the WHOLE `In` is + // dropped (a partial `In` could prune a file a missing member's rows need). + if f.op == ScanCmpOp::In { + if let ScanValue::Set(members) = &f.value { + if let Some(values) = set_members_to_literals(field.type_string(), members) { + comparisons.push(Expression::In { + field_id: field.id, + column: f.column.clone(), + values, + }); + } + } + continue; + } let op = match f.op { ScanCmpOp::Eq => ComparisonOp::Eq, ScanCmpOp::NotEq => ComparisonOp::NotEq, @@ -171,98 +188,11 @@ fn build_iceberg_filter( ScanCmpOp::LtEq => ComparisonOp::LtEq, ScanCmpOp::Gt => ComparisonOp::Gt, ScanCmpOp::GtEq => ComparisonOp::GtEq, + // Handled above; `In` never reaches the scalar `ComparisonOp` mapping. + ScanCmpOp::In => continue, }; - let value = match &f.value { - ScanValue::Bool(b) => LiteralValue::Boolean(*b), - // Push a Date literal only against a physically-`date` column. The - // Arrow reader applies it as an exact row filter (casting the column - // to text), but the operator enforces with a lenient `Date::parse` - // that also accepts `"2024-01-15Z"` / offset forms. On a physically - // string column the operator would keep such a row while the row - // filter drops it — so gate the pushdown to keep it a strict subset. - ScanValue::Date(d) => match field.type_string() { - Some("date") => LiteralValue::Date(*d), - _ => continue, - }, - // Iceberg `int` is 32-bit, `long` 64-bit; against a `decimal` column an - // integer pushes as an EXACT scale-0 decimal when numeric pushdown is - // on (else stays `Int64` → no prune). An out-of-i32-range literal on an - // `int` column is skipped rather than wrapped. See `int_pushdown_literal`. - ScanValue::Int(n) => { - match int_pushdown_literal(*n, field.type_string(), iceberg_numeric_stats_enabled()) - { - Some(v) => v, - None => continue, - } - } - ScanValue::Str(s) => LiteralValue::String(s.clone()), - // xsd:double / xsd:float FILTER value. Push only against a physically - // `double` column (exact f64 bounds); a `float` column would need an - // f64→f32 narrowing that can round the literal and over-prune a range, - // so skip it — the in-engine FILTER still applies. - ScanValue::Double(d) => match field.type_string() { - Some("double") => LiteralValue::Float64(*d), - // A binary float → decimal coercion is not exact in general, so a - // double literal is NOT pushed against a decimal column (keep is - // correct; the in-engine FILTER enforces). Breadcrumb per the - // decline-observably ruling. - Some(t) if t.starts_with("decimal") => { - debug!( - column = %f.column, - "double literal vs decimal column: pushdown declined (inexact float→decimal); in-engine FILTER enforces" - ); - continue; - } - _ => continue, - }, - // xsd:decimal FILTER value. Push only against a `decimal(...)` column; - // the literal keeps its own scale and the bound compare normalizes it - // against the column's scale. Row-group stats prune only when the - // column is FLBA-encoded (see `prunable_stats`); file-level manifest - // bounds prune regardless. - ScanValue::Decimal { - unscaled, - precision, - scale, - } => match field.type_string() { - Some(t) if t.starts_with("decimal") => LiteralValue::Decimal { - unscaled: *unscaled, - precision: *precision, - scale: *scale, - }, - // A decimal literal against an integer column has no exact - // cross-type bound compare, so it is NOT pushed (keep is correct). - // Breadcrumb per the decline-observably ruling. - Some("int" | "long") => { - debug!( - column = %f.column, - "decimal literal vs integer column: pushdown declined (no exact cross-type bound compare); in-engine FILTER enforces" - ); - continue; - } - _ => continue, - }, - // A reversed subject-template key: coerce the raw string to the - // column's physical type. A key that parses as an integer pushes as an - // integer literal against an `int`/`long`/`decimal` column — including - // a `decimal` of any scale (the Arrow reader casts the integer to the - // column's decimal type; row-group stats conservatively skip - // decimals). A `string` column pushes the raw string. A key that is - // not integer-valued, or any other physical type - // (float/date/timestamp/boolean), skips the pushdown — the operator - // still enforces the subject equality either way. - ScanValue::TemplateKey(s) => match field.type_string() { - Some("int") => match s.parse::() { - Ok(v) => LiteralValue::Int32(v), - Err(_) => continue, - }, - Some(t) if t == "long" || t.starts_with("decimal") => match s.parse::() { - Ok(v) => LiteralValue::Int64(v), - Err(_) => continue, - }, - Some("string") => LiteralValue::String(s.clone()), - _ => continue, - }, + let Some(value) = scan_value_to_literal(field.type_string(), &f.value) else { + continue; }; comparisons.push(Expression::Comparison { field_id: field.id, @@ -278,6 +208,142 @@ fn build_iceberg_filter( } } +/// Map each member of an `IN` set to an Iceberg [`LiteralValue`] against the +/// column's physical type, or `None` if ANY member cannot be represented — a +/// partial `IN` could prune a file that a dropped member's rows live in, which +/// the in-engine FILTER could then never recover. Reuses the exact scalar +/// conversion the single-value pushdown uses, so member soundness is identical. +fn set_members_to_literals( + field_type: Option<&str>, + members: &[ScanValue], +) -> Option> { + let mut out = Vec::with_capacity(members.len()); + for m in members { + out.push(scan_value_to_literal(field_type, m)?); + } + Some(out) +} + +/// Translate one scalar [`ScanValue`] to an Iceberg pushdown [`LiteralValue`] +/// against a column of the given physical type, or `None` to skip the push +/// (leaving the in-engine FILTER as the authority). Shared by the single-value +/// `Comparison` path and the set-membership `In` path. +fn scan_value_to_literal(field_type: Option<&str>, value: &ScanValue) -> Option { + Some(match value { + ScanValue::Bool(b) => LiteralValue::Boolean(*b), + // Push a Date literal only against a physically-`date` column. The + // Arrow reader applies it as an exact row filter (casting the column + // to text), but the operator enforces with a lenient `Date::parse` + // that also accepts `"2024-01-15Z"` / offset forms. On a physically + // string column the operator would keep such a row while the row + // filter drops it — so gate the pushdown to keep it a strict subset. + ScanValue::Date(d) => match field_type { + Some("date") => LiteralValue::Date(*d), + _ => return None, + }, + // Iceberg `int` is 32-bit, `long` 64-bit; against a `decimal` column an + // integer pushes as an EXACT scale-0 decimal when numeric pushdown is + // on (else stays `Int64` → no prune). An out-of-i32-range literal on an + // `int` column is skipped rather than wrapped. See `int_pushdown_literal`. + ScanValue::Int(n) => { + match int_pushdown_literal(*n, field_type, iceberg_numeric_stats_enabled()) { + Some(v) => v, + None => return None, + } + } + ScanValue::Str(s) => LiteralValue::String(s.clone()), + // xsd:double / xsd:float FILTER value. Push only against a physically + // `double` column (exact f64 bounds); a `float` column would need an + // f64→f32 narrowing that can round the literal and over-prune a range, + // so skip it — the in-engine FILTER still applies. + ScanValue::Double(d) => match field_type { + Some("double") => LiteralValue::Float64(*d), + // A binary float → decimal coercion is not exact in general, so a + // double literal is NOT pushed against a decimal column (keep is + // correct; the in-engine FILTER enforces). Breadcrumb per the + // decline-observably ruling. + Some(t) if t.starts_with("decimal") => { + debug!( + field_type = ?field_type, + "double literal vs decimal column: pushdown declined (inexact float→decimal); in-engine FILTER enforces" + ); + return None; + } + _ => return None, + }, + // xsd:decimal FILTER value. Push only against a `decimal(...)` column; + // the literal keeps its own scale and the bound compare normalizes it + // against the column's scale. Row-group stats prune only when the + // column is FLBA-encoded (see `prunable_stats`); file-level manifest + // bounds prune regardless. + ScanValue::Decimal { + unscaled, + precision, + scale, + } => match field_type { + Some(t) if t.starts_with("decimal") => LiteralValue::Decimal { + unscaled: *unscaled, + precision: *precision, + scale: *scale, + }, + // A decimal literal against an integer column has no exact + // cross-type bound compare, so it is NOT pushed (keep is correct). + // Breadcrumb per the decline-observably ruling. + Some("int" | "long") => { + debug!( + field_type = ?field_type, + "decimal literal vs integer column: pushdown declined (no exact cross-type bound compare); in-engine FILTER enforces" + ); + return None; + } + _ => return None, + }, + // A reversed subject-template key: coerce the raw string to the + // column's physical type. A key that parses as an integer pushes as an + // integer literal against an `int`/`long`/`decimal` column — including + // a `decimal` of any scale (the Arrow reader casts the integer to the + // column's decimal type; row-group stats conservatively skip + // decimals). A `string` column pushes the raw string. A key that is + // not integer-valued, or any other physical type + // (float/date/timestamp/boolean), skips the pushdown — the operator + // still enforces the subject equality either way. + ScanValue::TemplateKey(s) => match field_type { + Some("int") => match s.parse::() { + Ok(v) => LiteralValue::Int32(v), + Err(_) => return None, + }, + Some(t) if t == "long" || t.starts_with("decimal") => match s.parse::() { + Ok(v) => LiteralValue::Int64(v), + Err(_) => return None, + }, + Some("string") => LiteralValue::String(s.clone()), + _ => return None, + }, + // Item 10 (F-AUD-11): a dateTime pushes to MANIFEST-level pruning, and only + // when the literal's frame matches the column's timestamp type — a tz-aware + // (UTC) literal against `timestamptz`, a naive (wall-clock) literal against + // `timestamp`. Any other pairing (frame mismatch or non-timestamp column) + // declines: the micros would not be comparable to the file's decoded bounds, + // and pushing anyway could over-prune. The in-engine FILTER stays authority. + ScanValue::Timestamp { micros, tz } => match (field_type, tz) { + (Some("timestamptz"), true) => LiteralValue::TimestampTz(*micros), + (Some("timestamp"), false) => LiteralValue::Timestamp(*micros), + _ => { + debug!( + field_type = ?field_type, + tz_aware = tz, + "dateTime pushdown declined: literal frame does not match the column's timestamp type; in-engine FILTER enforces" + ); + return None; + } + }, + // A set is never a scalar literal — the `In` path handles it member by + // member (each member IS a scalar `ScanValue`, so this is unreachable for + // a well-formed filter; declining defensively is never wrong). + ScanValue::Set(_) => return None, + }) +} + // ============================================================================= // Iceberg/R2RML Graph Source Creation // ============================================================================= @@ -1971,10 +2037,11 @@ impl FlureeR2rmlProvider<'_> { // incrementally instead of the whole table being collected here. let footers = cache.parquet_footers(); - // PR-5 scan-side top-k. When a resolvable single-column DESC directive is - // present, read files in `upper_bound(sort_col)`-DESC order with a running - // k-th bound and stop once no unread file can beat it — streaming a strict - // SUPERSET of the top-k (the `SortOperator` above is authoritative). The + // PR-5 / item 8 scan-side top-k. When a resolvable single-column directive + // is present, read files best-first (DESC by `upper_bound`, ASC by + // `lower_bound`) with a running k-th bound and stop once no unread file can + // beat it — streaming a strict SUPERSET of the top-k (the `SortOperator` + // above is authoritative). ASC is admitted only for a required column. The // pruned subset MUST bypass the operator's scan cache (handled by its // `cacheable` guard gaining `&& topk.is_none()`); the disk *artifact* cache // is keyed by file path+size with whole-file entries, so a pruned subset @@ -1984,16 +2051,26 @@ impl FlureeR2rmlProvider<'_> { // bound / a heap that can't fill), the remaining files are handed to the // normal parallel reader so the topk path can never be slower than it. if let Some(tk) = topk { - if let Some(field) = schema.field_by_name(&tk.sort_column) { + // Item 8 (F-AUD-6): ASC top-k is SOUND ONLY for a REQUIRED (non-nullable) + // column — SPARQL orders unbound values FIRST under ASC, so a nullable + // column could hold an unread NULL row that belongs ahead of the top-k. + // DESC is sound for any column (unbound sorts last). A declined directive + // simply falls through to the normal parallel scan below (ignoring a + // top-k is always correct — the `SortOperator` above is authoritative). + let admitted = schema + .field_by_name(&tk.sort_column) + .filter(|field| !tk.ascending || field.required); + if let Some(field) = admitted { let sort_field_id = field.id; let sort_type = field.type_string().map(str::to_string); let order = plan_topk_read( tasks.iter().map(|t| &t.data_file), sort_field_id, sort_type.as_deref(), + tk.ascending, ); - let mut bound = TopKBound::new(tk.k); + let mut bound = TopKBound::new(tk.k, tk.ascending); let mut collected: Vec = Vec::new(); let mut tail: Vec = Vec::new(); let mut reads = 0usize; @@ -2044,11 +2121,12 @@ impl FlureeR2rmlProvider<'_> { } collected.extend(batches); reads += 1; - // Stop iff the heap is full and the NEXT (highest-remaining) - // file's upper_bound is strictly below the k-th (over-keep on - // ties; a no-bound next → never stops). See `TopKBound::can_stop`. - if let Some((_, next_upper)) = order.get(pos + 1) { - if bound.can_stop(next_upper.as_ref()) { + // Stop iff the heap is full and the NEXT (best-remaining) file's + // bound is strictly WORSE than the k-th (below it for DESC, above + // it for ASC; over-keep on ties; a no-bound next → never stops). + // See `TopKBound::can_stop`. + if let Some((_, next_bound)) = order.get(pos + 1) { + if bound.can_stop(next_bound.as_ref()) { break; } } @@ -2547,6 +2625,129 @@ mod tests { assert!(build_iceberg_filter(&[dbl("str_key")], &s).is_none()); } + fn in_filter(col: &str, values: Vec) -> ScanFilter { + ScanFilter { + column: col.to_string(), + op: ScanCmpOp::In, + value: ScanValue::Set(values), + } + } + + #[test] + fn in_set_builds_expression_in_over_scalar_members() { + // Item 7: a set filter lowers to `Expression::In` with one literal per + // member, resolved against the column's physical type. + let s = key_schema(); + let f = in_filter( + "str_key", + vec![ + ScanValue::Str("a".into()), + ScanValue::Str("b".into()), + ScanValue::Str("c".into()), + ], + ); + match build_iceberg_filter(&[f], &s) { + Some(Expression::In { column, values, .. }) => { + assert_eq!(column, "str_key"); + assert_eq!( + values, + vec![ + LiteralValue::String("a".into()), + LiteralValue::String("b".into()), + LiteralValue::String("c".into()), + ] + ); + } + other => panic!("expected Expression::In, got {other:?}"), + } + } + + #[test] + fn in_set_over_int_column_coerces_each_member() { + let s = key_schema(); + let f = in_filter( + "int_key", + vec![ScanValue::Int(1), ScanValue::Int(2), ScanValue::Int(3)], + ); + match build_iceberg_filter(&[f], &s) { + Some(Expression::In { values, .. }) => assert_eq!( + values, + vec![ + LiteralValue::Int32(1), + LiteralValue::Int32(2), + LiteralValue::Int32(3) + ] + ), + other => panic!("expected Expression::In, got {other:?}"), + } + } + + #[test] + fn in_set_declines_whole_when_any_member_cannot_push() { + // A Date member against a physically-`string` column can't push; the WHOLE + // In is dropped — a partial In could prune a file the Date member's rows + // live in, and the in-engine FILTER could never recover them. + let s = key_schema(); + let f = in_filter( + "str_key", + vec![ScanValue::Str("a".into()), ScanValue::Date(19_737)], + ); + assert!(build_iceberg_filter(&[f], &s).is_none()); + } + + #[test] + fn in_set_on_unknown_column_is_skipped() { + let s = key_schema(); + let f = in_filter("nope", vec![ScanValue::Int(1)]); + assert!(build_iceberg_filter(&[f], &s).is_none()); + } + + fn ts_schema() -> Schema { + Schema { + schema_id: 0, + identifier_field_ids: vec![], + fields: vec![ + field(1, "ts_ntz", json!("timestamp")), + field(2, "ts_tz", json!("timestamptz")), + ], + } + } + + fn ts_filter(col: &str, micros: i64, tz: bool) -> ScanFilter { + ScanFilter { + column: col.to_string(), + op: ScanCmpOp::Gt, + value: ScanValue::Timestamp { micros, tz }, + } + } + + #[test] + fn timestamp_pushdown_is_frame_matched() { + // Item 10: a tz-AWARE (UTC) literal pushes only against a `timestamptz` + // column; a NAIVE (wall-clock) literal only against a `timestamp` column. + let s = ts_schema(); + assert!(matches!( + only_literal(&[ts_filter("ts_tz", 1_717_200_000_000_000, true)], &s), + Some(LiteralValue::TimestampTz(1_717_200_000_000_000)) + )); + assert!(matches!( + only_literal(&[ts_filter("ts_ntz", 42, false)], &s), + Some(LiteralValue::Timestamp(42)) + )); + // Frame mismatch declines (the micros would not be comparable to the file + // bounds): a tz-aware literal on a `timestamp` column, or vice versa. + assert!(build_iceberg_filter(&[ts_filter("ts_ntz", 42, true)], &s).is_none()); + assert!(build_iceberg_filter(&[ts_filter("ts_tz", 42, false)], &s).is_none()); + } + + #[test] + fn timestamp_pushdown_declines_non_timestamp_column() { + // A dateTime literal against a non-timestamp column never pushes. + let s = key_schema(); + assert!(build_iceberg_filter(&[ts_filter("int_key", 42, false)], &s).is_none()); + assert!(build_iceberg_filter(&[ts_filter("str_key", 42, true)], &s).is_none()); + } + #[test] fn int_literal_coerces_to_scale0_decimal_only_when_numeric_stats_on() { // On: an integer against a decimal column → EXACT scale-0 decimal (prunable). diff --git a/fluree-db-api/src/ledger_info.rs b/fluree-db-api/src/ledger_info.rs index a2f9987ed..703098eab 100644 --- a/fluree-db-api/src/ledger_info.rs +++ b/fluree-db-api/src/ledger_info.rs @@ -311,9 +311,24 @@ pub struct Source { /// Catalog descriptor (REST or Direct-mode). Omitted when neither is known. #[serde(default, skip_serializing_if = "Option::is_none")] pub catalog: Option, - /// Authoritative per-table row counts (Iceberg manifest metadata). + /// Per-table row counts (Iceberg snapshot-summary `total-records`). Authoritative + /// for append-only / copy-on-write tables; for any table listed in + /// [`Self::mor_approximate_tables`] the count is an **upper bound** (it sums live + /// data-file records without subtracting merge-on-read position/equality deletes, + /// which Fluree recognizes but does not yet apply — F-AUD-1). #[serde(rename = "table-row-counts", default)] pub table_row_counts: BTreeMap, + /// Tables whose [`Self::table_row_counts`] entry is an **approximate upper bound** + /// because the current Iceberg snapshot carries merge-on-read delete files + /// (`total-delete-files`/`-position-deletes`/`-equality-deletes` > 0). Empty (and + /// omitted) for the common append-only case. Consumers that ignore unknown keys + /// (the `/data-model` + MCP readers, all null-safe) are unaffected. + #[serde( + rename = "mor-approximate-tables", + default, + skip_serializing_if = "Vec::is_empty" + )] + pub mor_approximate_tables: Vec, } /// Source catalog descriptor. `type`/`warehouse` emit explicit `null` (not @@ -1114,6 +1129,9 @@ pub struct VirtualSourceMeta { pub tables: Vec, /// Current Iceberg snapshot id (used as the virtual dataset "version"). pub snapshot_id: Option, + /// Tables whose snapshot carries merge-on-read delete files, so their row count + /// is an upper bound (see [`Source::mor_approximate_tables`]). + pub mor_approximate_tables: Vec, } /// Sum two optional row counts, treating `None` as "unknown" rather than zero: @@ -1382,6 +1400,7 @@ pub fn build_virtual_ledger_info( .iter() .map(|(t, c)| (t.clone(), *c)) .collect(), + mor_approximate_tables: meta.mor_approximate_tables.clone(), }; LedgerInfo { @@ -1532,7 +1551,7 @@ async fn fetch_virtual_table_row_counts( record: &GraphSourceRecord, cfg: &fluree_db_iceberg::IcebergGsConfig, tables: &[String], -) -> (HashMap, Option) { +) -> (HashMap, Option, Vec) { use fluree_db_iceberg::catalog::{ parse_table_identifier, RestCatalogClient, RestCatalogConfig, SendCatalogClient, }; @@ -1549,7 +1568,7 @@ async fn fetch_virtual_table_row_counts( .. } = &cfg.catalog else { - return (HashMap::new(), None); + return (HashMap::new(), None, Vec::new()); }; // Reuse (or warm) the process-wide REST client under the SAME cache key the @@ -1569,14 +1588,14 @@ async fn fetch_virtual_table_row_counts( Ok(a) => a, Err(e) => { tracing::debug!(error = %e, "virtual ledger-info: auth secret resolution failed; counts omitted"); - return (HashMap::new(), None); + return (HashMap::new(), None, Vec::new()); } }; let auth_provider = match hydrated_auth.create_provider_arc() { Ok(p) => p, Err(e) => { tracing::debug!(error = %e, "virtual ledger-info: auth provider build failed; counts omitted"); - return (HashMap::new(), None); + return (HashMap::new(), None, Vec::new()); } }; let catalog_config = RestCatalogConfig { @@ -1592,7 +1611,7 @@ async fn fetch_virtual_table_row_counts( } Err(e) => { tracing::debug!(error = %e, "virtual ledger-info: catalog client build failed; counts omitted"); - return (HashMap::new(), None); + return (HashMap::new(), None, Vec::new()); } } } @@ -1604,7 +1623,7 @@ async fn fetch_virtual_table_row_counts( // client. `.buffered` (not `buffer_unordered`) preserves request order, so the // snapshot pin (first successful table) is deterministic for the sorted // `tables`. - let per_table: Vec, i64)>> = + let per_table: Vec, i64, bool)>> = futures::stream::iter(tables.iter().cloned()) .map(|table| { let catalog = Arc::clone(&catalog); @@ -1626,9 +1645,17 @@ async fn fetch_virtual_table_row_counts( } }; let metadata = load.metadata.as_ref()?; + // MoR rider (F-AUD-1): the snapshot-summary `total-records` this + // count derives from OVER-COUNTS a merge-on-read table (it does not + // subtract position/equality deletes, which Fluree recognizes but + // does not yet apply). Flag such a table so its count is reported + // as an upper bound. Zero-I/O — reads only the in-memory summary. + let has_deletes = metadata + .current_snapshot() + .is_some_and(fluree_db_iceberg::mor_guard::summary_indicates_deletes); let schema = crate::graph_source::table_schema_from_metadata(&api_id, metadata).ok()?; - Some((table, schema.row_count, schema.snapshot.id)) + Some((table, schema.row_count, schema.snapshot.id, has_deletes)) } }) .buffered(INFO_COUNT_FETCH_CONCURRENCY) @@ -1637,7 +1664,11 @@ async fn fetch_virtual_table_row_counts( let mut counts = HashMap::new(); let mut snapshot_id = None; - for (table, row_count, snap) in per_table.into_iter().flatten() { + let mut mor_approximate_tables = Vec::new(); + for (table, row_count, snap, has_deletes) in per_table.into_iter().flatten() { + if has_deletes { + mor_approximate_tables.push(table.clone()); + } if let Some(rc) = row_count { counts.insert(table, rc); } @@ -1645,7 +1676,8 @@ async fn fetch_virtual_table_row_counts( snapshot_id = Some(snap); } } - (counts, snapshot_id) + mor_approximate_tables.sort(); + (counts, snapshot_id, mor_approximate_tables) } /// Async orchestration for the Iceberg/R2RML virtual-info path: resolve the @@ -1713,7 +1745,11 @@ async fn build_iceberg_virtual_info( // catalog degrades counts to empty (and snapshot to `None`) while the schema // (classes/properties, derived from the mapping) still renders on time. let budget_ms = info_count_budget_ms(); - let (counts, fetched_snapshot): (HashMap, Option) = match cfg.as_ref() { + let (counts, fetched_snapshot, mor_approximate_tables): ( + HashMap, + Option, + Vec, + ) = match cfg.as_ref() { Some(cfg) if budget_ms > 0 => { match tokio::time::timeout( std::time::Duration::from_millis(budget_ms), @@ -1728,15 +1764,16 @@ async fn build_iceberg_virtual_info( budget_ms, "virtual ledger-info: row-count fetch exceeded budget; counts omitted" ); - (HashMap::new(), None) + (HashMap::new(), None, Vec::new()) } } } - _ => (HashMap::new(), None), + _ => (HashMap::new(), None, Vec::new()), }; if meta.snapshot_id.is_none() { meta.snapshot_id = fetched_snapshot; } + meta.mor_approximate_tables = mor_approximate_tables; Ok(build_virtual_ledger_info(record, mapping.as_deref(), &meta, &counts).into_json()) } @@ -2109,6 +2146,53 @@ const fn serve_virtual_stats_from_metadata(ledger_t: i64, graph_source_registere ledger_t == 0 && graph_source_registered } +/// Env switch for per-member `/info` routing (item 14, C1 residual). Default ON. +/// With it on, a graph-source-registered ledger serves its virtual (manifest- +/// backed) stats regardless of the native ledger's `t`: an empty shell (`t == 0`) +/// serves virtual-only (`serve_virtual_stats_from_metadata`, unchanged), and a +/// **hybrid** (`t > 0` with a registered graph source) MERGES the native members +/// with the graph-source member instead of taking the native-only path — so +/// `get_data_model` no longer loses the virtual model the moment the shell crosses +/// `t > 0`. Set to a falsy value (`0`/`false`/`off`/`no`) to restore the strict +/// `t == 0`-only reroute (a hybrid then takes the native path, the prior behavior). +const INFO_MEMBER_ROUTING_ENV: &str = "FLUREE_R2RML_INFO_MEMBER_ROUTING"; + +fn info_member_routing_enabled() -> bool { + info_member_routing_from_env(std::env::var(INFO_MEMBER_ROUTING_ENV).ok().as_deref()) +} + +/// Pure default-ON parse (split out from [`info_member_routing_enabled`] so it is +/// unit-testable without mutating the shared process environment): absent or any +/// non-falsy value is ON; `0`/`false`/`off`/`no` (case-insensitive, trimmed) is OFF. +fn info_member_routing_from_env(value: Option<&str>) -> bool { + !matches!( + value.map(|v| v.trim().to_ascii_lowercase()).as_deref(), + Some("0" | "false" | "off" | "no") + ) +} + +/// Merge a graph-source (virtual) `LedgerInfo` into the native one for a **hybrid** +/// ledger — committed native data (`t > 0`) that ALSO has a registered graph +/// source. The native ledger/commit/index/`t` metadata is authoritative and kept; +/// the graph-source member's `classes`/`properties` are overlaid by (compacted) +/// IRI — the graph source is authoritative for the classes it maps, so it wins a +/// key collision — while native-only classes/properties are preserved (a union, +/// never a sum, so a shared IRI is never double-counted). The virtual `source` +/// block (snapshot id + per-table counts) is attached so the hybrid is marked +/// virtual-backed while still reporting its native members. Top-level +/// `stats.flakes`/`size` stay native (the native ledger's own totals; per-class +/// virtual counts carry the graph-source detail). +fn merge_virtual_into_native(mut native: LedgerInfo, virt: LedgerInfo) -> LedgerInfo { + for (iri, class) in virt.stats.classes { + native.stats.classes.insert(iri, class); + } + for (iri, prop) in virt.stats.properties { + native.stats.properties.insert(iri, prop); + } + native.source = virt.source; + native +} + impl<'a> LedgerInfoBuilder<'a> { /// Create a new builder (called by `Fluree::ledger_info()`). pub(crate) fn new(fluree: &'a Fluree, ledger_id: String) -> Self { @@ -2205,14 +2289,19 @@ impl<'a> LedgerInfoBuilder<'a> { Err(e) => return Err(e), }; - // T1.2: reroute an empty-shell virtual dataset's stats to the metadata-only - // graph-source path, so `/info` (e.g. MCP `get_data_model`) reads counts - // from Iceberg manifests + the mapping instead of scanning every backing - // table (the 900s runaway). The graph-source lookup runs ONLY for an empty - // shell (`t == 0`) — no extra work on the native hot path — and a lookup - // failure falls through to native (fail-safe). See - // `serve_virtual_stats_from_metadata` for the precise predicate. - let graph_source = if ledger.t() == 0 { + // Per-member routing (item 14, C1 residual). A graph-source-registered + // ledger serves its virtual (manifest-backed) stats regardless of the native + // `t`, so `/info` (e.g. MCP `get_data_model`) reads counts from Iceberg + // manifests + the mapping instead of scanning every backing table (the 900s + // runaway) — and no longer loses that model the moment an empty shell crosses + // `t > 0`. T1.2 (`t == 0`) serves virtual-only; a HYBRID (`t > 0` + a graph + // source) MERGES the native members with the graph-source member. The lookup + // runs only when it can change the result — `t == 0`, OR member routing is on + // — so the pure native hot path is untouched; any lookup/build failure falls + // through to native (fail-safe). Gated by `FLUREE_R2RML_INFO_MEMBER_ROUTING` + // (default on): with it off, only the `t == 0` reroute fires (prior behavior). + let member_routing = info_member_routing_enabled(); + let graph_source = if ledger.t() == 0 || member_routing { self.fluree .nameservice() .lookup_graph_source(&self.ledger_id) @@ -2222,10 +2311,48 @@ impl<'a> LedgerInfoBuilder<'a> { } else { None }; - if serve_virtual_stats_from_metadata(ledger.t(), graph_source.is_some()) { - if let Some(gs) = graph_source { + if let Some(gs) = graph_source { + if serve_virtual_stats_from_metadata(ledger.t(), true) { + // Empty shell: the graph source IS the dataset (no native members). return build_graph_source_info(self.fluree, &gs).await; } + if member_routing { + // Hybrid (t > 0 + a graph source): build both members and merge, so + // the native members keep the native path and the graph-source member + // is served from manifest-backed counts. Bypasses the response cache + // (hybrids are rare); reuses the same native builder the cache path + // uses below. + let storage = self + .fluree + .backend() + .admin_storage_cloned() + .ok_or_else(|| { + ApiError::config("ledger_info requires a managed storage backend") + })?; + let native_json = build_ledger_info_with_options( + &ledger, + &storage, + self.context, + self.options.clone(), + ) + .await + .map_err(|e| ApiError::internal(format!("ledger_info failed: {e}")))?; + let virtual_json = build_graph_source_info(self.fluree, &gs).await?; + // Best-effort typed merge; on any deserialize failure prefer the + // virtual model (it carries the authoritative graph-source classes + // item 14 needs — the native-only response was the bug being fixed). + return Ok( + match ( + serde_json::from_value::(native_json), + serde_json::from_value::(virtual_json.clone()), + ) { + (Ok(native), Ok(virt)) => { + merge_virtual_into_native(native, virt).into_json() + } + _ => virtual_json, + }, + ); + } } // Optional API-level cache: when ledger caching is enabled, a global LeafletCache @@ -2338,11 +2465,123 @@ mod tests { assert!(!serve_virtual_stats_from_metadata(0, false)); assert!(!serve_virtual_stats_from_metadata(42, false)); // (e) hybrid: a ledger with committed native data (t>0) that ALSO has a - // graph source registered → native path, so its real flakes are never - // under-reported by the manifest-only path. + // graph source registered. This predicate stays `false` (it gates the + // virtual-ONLY reroute); item 14's `execute` routing now handles the hybrid + // separately when `info_member_routing_enabled()` — see + // `merge_virtual_into_native_unions_classes_graph_source_wins`. assert!(!serve_virtual_stats_from_metadata(42, true)); } + /// Item 14 switch: default ON, only explicit falsy values turn it off (so a + /// hybrid keeps serving its virtual model by default; an operator can revert to + /// the strict `t == 0` reroute with `FLUREE_R2RML_INFO_MEMBER_ROUTING=0`). + #[test] + fn info_member_routing_default_on_and_falsy_off() { + assert!(info_member_routing_from_env(None), "absent → default on"); + for on in ["1", "true", "on", "yes", "whatever"] { + assert!(info_member_routing_from_env(Some(on)), "{on:?} → on"); + } + for off in ["0", "false", "OFF", " no ", "No"] { + assert!(!info_member_routing_from_env(Some(off)), "{off:?} → off"); + } + } + + /// Item 14 hybrid merge: native members keep the native path (native-only + /// classes/properties + native ledger/commit metadata 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. + #[test] + fn merge_virtual_into_native_unions_classes_graph_source_wins() { + use serde_json::json; + let native: LedgerInfo = serde_json::from_value(json!({ + "ledger": {"alias":"ds","t":7,"commit-t":7,"index-t":7,"flakes":10,"size":0, + "named-graphs":[{"iri":"urn:default","g-id":0,"flakes":10,"size":0}]}, + "graph": "default", + "stats": {"flakes":10,"size":0, + "classes":{"A":{"count":5},"C":{"count":5}}, + "properties":{"p1":{"count":5}}}, + "commit": null, + "commitId": "nativecommit" + })) + .unwrap(); + let virt: LedgerInfo = serde_json::from_value(json!({ + "ledger_id":"ds","t":42, + "ledger": {"alias":"ds","t":42,"commit-t":null,"index-t":null,"flakes":300,"size":0, + "named-graphs":[{"iri":"urn:default","g-id":0,"flakes":300,"size":0}]}, + "graph": "default", + "stats": {"flakes":300,"size":0, + "classes":{"B":{"count":200},"C":{"count":100}}, + "properties":{"p2":{"count":200}}}, + "commit": null, + "source": {"virtual":true,"type":"Iceberg","tables":["t"],"snapshot":42, + "table-row-counts":{"t":300}} + })) + .unwrap(); + + let merged = merge_virtual_into_native(native, virt); + assert_eq!( + merged.stats.classes.get("A").and_then(|c| c.count), + Some(5), + "native-only class preserved" + ); + assert_eq!( + merged.stats.classes.get("B").and_then(|c| c.count), + Some(200), + "virtual-only class added" + ); + assert_eq!( + merged.stats.classes.get("C").and_then(|c| c.count), + Some(100), + "graph source wins the shared IRI (union, not sum — no double-count)" + ); + assert!(merged.stats.properties.contains_key("p1")); + assert!(merged.stats.properties.contains_key("p2")); + assert_eq!( + merged.commit_id.as_deref(), + Some("nativecommit"), + "native ledger metadata authoritative" + ); + assert_eq!(merged.ledger.t, Some(7), "native ledger t kept"); + assert_eq!( + merged.source.as_ref().and_then(|s| s.snapshot), + Some(42), + "virtual source block attached" + ); + } + + /// Item 14 MoR rider: a table whose snapshot carries merge-on-read deletes keeps + /// its (upper-bound) row count but is flagged in `mor-approximate-tables`, so the + /// `/info` payload is honest rather than silently over-reporting. Consumers that + /// ignore the new key (all null-safe) are unaffected. + #[test] + fn mor_approximate_tables_flag_surfaces_in_source() { + use serde_json::json; + let record = virtual_record(&secret_bearing_config()); + let mapping = two_table_mapping(); + let mut counts = HashMap::new(); + counts.insert("openflights.airlines".to_string(), 100); + let mut meta = iceberg_meta(); + meta.mor_approximate_tables = vec!["openflights.airlines".to_string()]; + let info = serde_json::to_value(build_virtual_ledger_info( + &record, + Some(&mapping), + &meta, + &counts, + )) + .unwrap(); + let source = &info["source"]; + assert_eq!( + source["table-row-counts"]["openflights.airlines"], 100, + "the upper-bound count is still reported (flagged, not dropped)" + ); + assert_eq!( + source["mor-approximate-tables"], + json!(["openflights.airlines"]), + "the MoR table is flagged approximate" + ); + } + #[test] fn test_compute_selectivity() { assert_eq!(compute_selectivity(100, 50), 2); @@ -2566,6 +2805,7 @@ mod tests { "openflights.routes".to_string(), ], snapshot_id: Some(42), + mor_approximate_tables: Vec::new(), } } diff --git a/fluree-db-iceberg/src/io/arrow_reader.rs b/fluree-db-iceberg/src/io/arrow_reader.rs index f9a4e1153..868773550 100644 --- a/fluree-db-iceberg/src/io/arrow_reader.rs +++ b/fluree-db-iceberg/src/io/arrow_reader.rs @@ -389,6 +389,14 @@ fn collect_and_comparisons( true } Expression::And(children) => children.iter().all(|c| collect_and_comparisons(c, out)), + // An `In`/`NotIn` set filter has no row-level Arrow representation here. + // Treat it as transparent (don't push, don't abort) so a sibling + // comparison in the same `And` still builds its row filter. Sound: the row + // filter may only ever keep MORE rows than the true predicate — file / + // row-group pruning (which DO evaluate `In`) and the in-engine FILTER stay + // the authority. A lone `In` yields an empty plan ⇒ no row filter, exactly + // as before this arm existed. + Expression::In { .. } | Expression::NotIn { .. } => true, _ => false, } } @@ -452,7 +460,12 @@ fn literal_to_array(value: &LiteralValue) -> ArrayRef { LiteralValue::String(s) => Arc::new(StringArray::from(vec![s.clone()])), LiteralValue::Bytes(b) => Arc::new(BinaryArray::from(vec![b.as_slice()])), LiteralValue::Date(d) => Arc::new(Date32Array::from(vec![*d])), - LiteralValue::Timestamp(t) => Arc::new(TimestampMicrosecondArray::from(vec![*t])), + // Both timestamp flavors carry micros-since-epoch; `cast` adapts to the + // column's own (tz-tagged or not) timestamp type. If the cast can't adapt, + // `eval_comparison` keeps every row and the in-engine FILTER decides. + LiteralValue::Timestamp(t) | LiteralValue::TimestampTz(t) => { + Arc::new(TimestampMicrosecondArray::from(vec![*t])) + } LiteralValue::Decimal { unscaled, precision, diff --git a/fluree-db-iceberg/src/io/parquet.rs b/fluree-db-iceberg/src/io/parquet.rs index 0296f5ac3..8dcc59ce9 100644 --- a/fluree-db-iceberg/src/io/parquet.rs +++ b/fluree-db-iceberg/src/io/parquet.rs @@ -389,12 +389,14 @@ impl<'a, S: IcebergStorage> ParquetReader<'a, S> { "Range-reading Parquet file" ); - // Fetch all ranges (could be parallelized with bounded concurrency) - let mut range_data: Vec<(u64, Bytes)> = Vec::with_capacity(coalesced.len()); - for (start, end) in &coalesced { - let data = self.storage.read_range(path, *start..*end).await?; - range_data.push((*start, data)); - } + // Item 12 (B1-AppD): fetch the coalesced ranges via `read_ranges` — bounded + // concurrent GETs on S3 (order-preserving), sequential on other backends. + // The bytes come back in the SAME order as the input ranges, so pair each + // back to its start offset for the sparse-buffer assembler. + let range_reqs: Vec> = coalesced.iter().map(|(s, e)| *s..*e).collect(); + let range_bytes = self.storage.read_ranges(path, range_reqs).await?; + let range_data: Vec<(u64, Bytes)> = + coalesced.iter().map(|(s, _)| *s).zip(range_bytes).collect(); // Assemble into sparse buffer let sparse_buffer = assemble_sparse_buffer(file_size as usize, range_data); diff --git a/fluree-db-iceberg/src/io/send_parquet.rs b/fluree-db-iceberg/src/io/send_parquet.rs index 74cf8a01f..b72acda40 100644 --- a/fluree-db-iceberg/src/io/send_parquet.rs +++ b/fluree-db-iceberg/src/io/send_parquet.rs @@ -950,12 +950,14 @@ impl<'a, S: SendIcebergStorage> SendParquetReader<'a, S> { "Range-reading Parquet file" ); - // Fetch all ranges - let mut range_data: Vec<(u64, Bytes)> = Vec::with_capacity(coalesced.len()); - for (start, end) in &coalesced { - let data = self.storage.read_range(path, *start..*end).await?; - range_data.push((*start, data)); - } + // Item 12 (B1-AppD): fetch the coalesced ranges via `read_ranges` — bounded + // concurrent GETs on S3 (order-preserving), sequential on other backends + // (incl. the local-file wrapper, where the default sequential impl already + // does the right thing). Bytes return in input order → zip to their starts. + let range_reqs: Vec> = coalesced.iter().map(|(s, e)| *s..*e).collect(); + let range_bytes = self.storage.read_ranges(path, range_reqs).await?; + let range_data: Vec<(u64, Bytes)> = + coalesced.iter().map(|(s, _)| *s).zip(range_bytes).collect(); // Assemble into sparse buffer let sparse_buffer = assemble_sparse_buffer(file_size as usize, range_data); diff --git a/fluree-db-iceberg/src/io/storage.rs b/fluree-db-iceberg/src/io/storage.rs index 42fa53f4f..fa87f327e 100644 --- a/fluree-db-iceberg/src/io/storage.rs +++ b/fluree-db-iceberg/src/io/storage.rs @@ -37,6 +37,18 @@ pub trait IcebergStorage: Debug { /// Used to determine where to read the Parquet footer from (last 8 bytes, /// then footer_length bytes before that). async fn file_size(&self, path: &str) -> Result; + + /// Read several byte ranges, returning the bytes in the SAME order as `ranges` + /// (item 12, B1-AppD). The default reads them sequentially; a backend with + /// concurrent range GETs (S3) overrides this with bounded parallelism. Always + /// order-preserving so the sparse-buffer assembler can pair bytes to ranges. + async fn read_ranges(&self, path: &str, ranges: Vec>) -> Result> { + let mut out = Vec::with_capacity(ranges.len()); + for range in ranges { + out.push(self.read_range(path, range).await?); + } + Ok(out) + } } /// Send-safe storage for AWS SDK integration. @@ -54,6 +66,17 @@ pub trait SendIcebergStorage: Debug + Send + Sync { /// Get the size of a file in bytes. async fn file_size(&self, path: &str) -> Result; + + /// Read several byte ranges in input order (item 12, B1-AppD). Default is + /// sequential; S3 overrides with bounded parallelism. See + /// [`IcebergStorage::read_ranges`]. + async fn read_ranges(&self, path: &str, ranges: Vec>) -> Result> { + let mut out = Vec::with_capacity(ranges.len()); + for range in ranges { + out.push(self.read_range(path, range).await?); + } + Ok(out) + } } /// S3 storage implementation using vended credentials. @@ -446,15 +469,33 @@ impl S3IcebergStorage { Ok(body.into_bytes()) } - /// Read multiple byte ranges concurrently with bounded parallelism. - pub async fn read_ranges(&self, path: &str, ranges: Vec>) -> Result> { + /// Read multiple byte ranges, returning bytes in the SAME order as `ranges` + /// (item 12, B1-AppD). Wired into the coalesced-fetch loops of the sparse + /// Parquet readers. With `FLUREE_ICEBERG_PARALLEL_RANGE_GETS` on (default) the + /// GETs run concurrently under a bounded semaphore, ORDER-PRESERVING via + /// `buffered` (NOT `buffer_unordered`, whose completion-order output cannot be + /// paired back to the input ranges); with the switch off it falls back to a + /// sequential loop, byte-identical to the pre-item-12 fetch. + pub(crate) async fn read_ranges_bounded( + &self, + path: &str, + ranges: Vec>, + ) -> Result> { use futures::stream::{self, StreamExt}; use std::sync::Arc; use tokio::sync::Semaphore; let (bucket, key) = Self::parse_s3_uri(path)?; - let semaphore = Arc::new(Semaphore::new(self.max_concurrent_reads)); + if !parallel_range_gets_enabled() { + let mut out = Vec::with_capacity(ranges.len()); + for range in ranges { + out.push(self.get_object_range(&bucket, &key, range).await?); + } + return Ok(out); + } + + let semaphore = Arc::new(Semaphore::new(self.max_concurrent_reads)); let results: Vec> = stream::iter(ranges) .map(|range| { let sem = Arc::clone(&semaphore); @@ -468,7 +509,9 @@ impl S3IcebergStorage { self.get_object_range(&bucket, &key, range).await } }) - .buffer_unordered(self.max_concurrent_reads) + // `buffered` (not `buffer_unordered`) yields IN INPUT ORDER, so the + // caller can zip the returned bytes back to their `ranges`. + .buffered(self.max_concurrent_reads) .collect() .await; @@ -476,6 +519,23 @@ impl S3IcebergStorage { } } +/// Item 12 kill switch (`FLUREE_ICEBERG_PARALLEL_RANGE_GETS`, default **on**). +/// When off, `read_ranges_bounded` reads the coalesced ranges sequentially — +/// byte-identical to the pre-item-12 loop. Read once (process-wide). +#[cfg(feature = "aws")] +fn parallel_range_gets_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init( + || match std::env::var("FLUREE_ICEBERG_PARALLEL_RANGE_GETS") { + Ok(v) => !matches!( + v.trim().to_ascii_lowercase().as_str(), + "0" | "false" | "off" | "no" + ), + Err(_) => true, + }, + ) +} + #[cfg(feature = "aws")] #[async_trait(?Send)] impl IcebergStorage for S3IcebergStorage { @@ -542,6 +602,13 @@ impl IcebergStorage for S3IcebergStorage { .map(|l| l as u64) .ok_or_else(|| IcebergError::storage("No content-length in HEAD response")) } + + /// Item 12 (B1-AppD): concurrent, ORDER-PRESERVING range GETs (delegates to + /// [`S3IcebergStorage::read_ranges_bounded`], which honors the + /// `FLUREE_ICEBERG_PARALLEL_RANGE_GETS` switch). + async fn read_ranges(&self, path: &str, ranges: Vec>) -> Result> { + self.read_ranges_bounded(path, ranges).await + } } #[cfg(feature = "aws")] @@ -610,6 +677,13 @@ impl SendIcebergStorage for S3IcebergStorage { .map(|l| l as u64) .ok_or_else(|| IcebergError::storage("No content-length in HEAD response")) } + + /// Item 12 (B1-AppD): concurrent, ORDER-PRESERVING range GETs (delegates to + /// [`S3IcebergStorage::read_ranges_bounded`], which honors the + /// `FLUREE_ICEBERG_PARALLEL_RANGE_GETS` switch). + async fn read_ranges(&self, path: &str, ranges: Vec>) -> Result> { + self.read_ranges_bounded(path, ranges).await + } } /// In-memory storage for testing. @@ -910,6 +984,26 @@ mod tests { assert!(empty.is_empty()); } + /// Item 12 (B1-AppD): `read_ranges` returns bytes in the SAME order as the + /// input ranges — the contract the sparse-buffer assembler relies on to pair + /// each range to its start offset. Exercises the trait's default (sequential) + /// impl; the S3 override preserves order too (via `buffered`, not + /// `buffer_unordered`). + #[tokio::test] + async fn read_ranges_returns_bytes_in_input_order() { + let mut storage = MemoryStorage::new(); + storage.add_file("f.txt", "0123456789ABCDEF"); + // Ranges deliberately NOT in ascending order. + let out = storage + .read_ranges("f.txt", vec![10..13, 0..3, 5..8]) + .await + .unwrap(); + assert_eq!(out.len(), 3); + assert_eq!(&out[0][..], b"ABC", "range 10..13"); + assert_eq!(&out[1][..], b"012", "range 0..3"); + assert_eq!(&out[2][..], b"567", "range 5..8"); + } + // ── Access-denied classification (feature-independent; no SDK types) ── #[test] diff --git a/fluree-db-iceberg/src/scan/predicate.rs b/fluree-db-iceberg/src/scan/predicate.rs index 76610fd17..6b98f5937 100644 --- a/fluree-db-iceberg/src/scan/predicate.rs +++ b/fluree-db-iceberg/src/scan/predicate.rs @@ -77,8 +77,13 @@ pub enum LiteralValue { Bytes(Vec), /// Date: days since 1970-01-01 Date(i32), - /// Timestamp: microseconds since epoch + /// Timestamp WITHOUT zone (`timestamp`): microseconds since epoch, wall-clock + /// frame. Only ever compared against a physically-`timestamp` column (item 10). Timestamp(i64), + /// Timestamp WITH zone (`timestamptz`): microseconds since epoch, UTC frame. + /// Only ever compared against a physically-`timestamptz` column (item 10) — the + /// frame (naive vs UTC) is matched at emit time so the micros are comparable. + TimestampTz(i64), /// Decimal: (unscaled_value, precision, scale) Decimal { unscaled: i128, @@ -100,6 +105,7 @@ impl LiteralValue { Self::Bytes(v) => TypedValue::Bytes(v.clone()), Self::Date(v) => TypedValue::Date(*v), Self::Timestamp(v) => TypedValue::Timestamp(*v), + Self::TimestampTz(v) => TypedValue::TimestampTz(*v), Self::Decimal { unscaled, precision, @@ -123,7 +129,8 @@ impl LiteralValue { TypedValue::String(v) => Self::String(v.clone()), TypedValue::Bytes(v) => Self::Bytes(v.clone()), TypedValue::Date(v) => Self::Date(*v), - TypedValue::Timestamp(v) | TypedValue::TimestampTz(v) => Self::Timestamp(*v), + TypedValue::Timestamp(v) => Self::Timestamp(*v), + TypedValue::TimestampTz(v) => Self::TimestampTz(*v), TypedValue::Uuid(v) => Self::Bytes(v.to_vec()), TypedValue::Decimal { unscaled, @@ -150,6 +157,7 @@ impl std::fmt::Display for LiteralValue { Self::Bytes(v) => write!(f, "bytes[{}]", v.len()), Self::Date(v) => write!(f, "date({v})"), Self::Timestamp(v) => write!(f, "ts({v})"), + Self::TimestampTz(v) => write!(f, "tstz({v})"), Self::Decimal { unscaled, scale, .. } => { diff --git a/fluree-db-iceberg/src/scan/pruning.rs b/fluree-db-iceberg/src/scan/pruning.rs index 62d4906dd..df88bc895 100644 --- a/fluree-db-iceberg/src/scan/pruning.rs +++ b/fluree-db-iceberg/src/scan/pruning.rs @@ -371,6 +371,13 @@ fn stat_bounds( }; (s.min_opt().and_then(to_str), s.max_opt().and_then(to_str)) } + // Item 10 (F-AUD-11): NO `TypedValue::Timestamp`/`TimestampTz` arm — a + // timestamp column is a Parquet INT64 whose logical UNIT (milli/micro/nano) + // is ambiguous from the row-group `Statistics` alone (A1 §4), so decoding the + // raw i64 stat as micros could over-prune. Timestamp pruning is therefore + // MANIFEST-LEVEL ONLY (Iceberg `value_codec` decodes the unit unambiguously + // from the schema type; see `can_contain_comparison`). Falling through to + // `(None, None)` keeps every row group for a timestamp predicate. _ => (None, None), } } @@ -507,11 +514,13 @@ fn evaluate_comparison( .get(i) .and_then(|v| *v) .is_some_and(|v| compare_op(v, *lit, op)), - (Column::Timestamp(vals) | Column::TimestampTz(vals), LiteralValue::Timestamp(lit)) => { - vals.get(i) - .and_then(|v| *v) - .is_some_and(|v| compare_op(v, *lit, op)) - } + ( + Column::Timestamp(vals) | Column::TimestampTz(vals), + LiteralValue::Timestamp(lit) | LiteralValue::TimestampTz(lit), + ) => vals + .get(i) + .and_then(|v| *v) + .is_some_and(|v| compare_op(v, *lit, op)), _ => false, // Type mismatch }; @@ -596,6 +605,69 @@ mod tests { } } + fn create_timestamptz_schema() -> Schema { + Schema { + schema_id: 0, + identifier_field_ids: vec![], + fields: vec![crate::metadata::SchemaField { + id: 1, + name: "ts".to_string(), + required: true, + field_type: serde_json::json!("timestamptz"), + doc: None, + }], + } + } + + #[test] + fn timestamptz_manifest_pruning_by_micros_bounds() { + // Item 10 (F-AUD-11): a `timestamptz` FILTER prunes at the MANIFEST level — + // Iceberg decodes the file's micros bounds unambiguously from the schema + // type. File holds ts micros in [1000, 2000]. + let schema = create_timestamptz_schema(); + let data_file = create_test_data_file(1000, 2000); + let cmp = |op, micros| Expression::Comparison { + field_id: 1, + column: "ts".to_string(), + op, + value: LiteralValue::TimestampTz(micros), + }; + // ts > 2000 (at/above upper) → prune; ts > 1500 (in range) → keep. + assert!(!can_contain_file( + &cmp(ComparisonOp::Gt, 2000), + &data_file, + &schema + )); + assert!(can_contain_file( + &cmp(ComparisonOp::Gt, 1500), + &data_file, + &schema + )); + // ts < 1000 (at/below lower) → prune; ts < 1500 (in range) → keep. + assert!(!can_contain_file( + &cmp(ComparisonOp::Lt, 1000), + &data_file, + &schema + )); + assert!(can_contain_file( + &cmp(ComparisonOp::Lt, 1500), + &data_file, + &schema + )); + + // Defense in depth: a `Timestamp` (naive-frame) literal against this + // `timestamptz` column is a TypedValue mismatch → the compare is + // incomparable → the file is KEPT (never over-pruned). The build-time + // frame match prevents this pairing from ever being emitted. + let ntz_lit = Expression::Comparison { + field_id: 1, + column: "ts".to_string(), + op: ComparisonOp::Gt, + value: LiteralValue::Timestamp(9_000), + }; + assert!(can_contain_file(&ntz_lit, &data_file, &schema)); + } + #[test] fn test_can_contain_file_eq() { let schema = create_test_schema(); @@ -800,6 +872,38 @@ mod tests { assert!(row_group_can_contain(&ge, meta.row_group(0), &unmapped)); } + #[test] + fn row_group_pruning_in_set_keeps_iff_any_member_in_bounds() { + // Item 7 (F-AUD-5): `Expression::In` keeps a row group iff ANY member could + // lie within its column bounds — the same superset guarantee the file-level + // prune gives. rg0 = [0..4], rg1 = [100..104]. + use parquet::file::reader::{FileReader, SerializedFileReader}; + + let reader = SerializedFileReader::new(two_row_group_parquet()).unwrap(); + let meta = reader.metadata(); + let field_to_col = HashMap::from([(1i32, 0usize)]); + let in_set = |vs: &[i64]| Expression::In { + field_id: 1, + column: "v".to_string(), + values: vs.iter().map(|v| LiteralValue::Int64(*v)).collect(), + }; + + // {2, 200}: 2 ∈ rg0 → keep rg0; neither ∈ [100,104] → prune rg1. + let a = in_set(&[2, 200]); + assert!(row_group_can_contain(&a, meta.row_group(0), &field_to_col)); + assert!(!row_group_can_contain(&a, meta.row_group(1), &field_to_col)); + + // {2, 102}: a member lands in each row group → keep both. + let b = in_set(&[2, 102]); + assert!(row_group_can_contain(&b, meta.row_group(0), &field_to_col)); + assert!(row_group_can_contain(&b, meta.row_group(1), &field_to_col)); + + // {500, 600}: no member in either range → prune both. + let c = in_set(&[500, 600]); + assert!(!row_group_can_contain(&c, meta.row_group(0), &field_to_col)); + assert!(!row_group_can_contain(&c, meta.row_group(1), &field_to_col)); + } + /// Two row groups with disjoint UTF-8 string ranges, for `ByteArray`-stats /// pruning: rg0 = [apple, cherry], rg1 = [mango, peach]. fn two_row_group_string_parquet() -> bytes::Bytes { diff --git a/fluree-db-iceberg/src/scan/topk.rs b/fluree-db-iceberg/src/scan/topk.rs index 94cc968d6..558ad20ec 100644 --- a/fluree-db-iceberg/src/scan/topk.rs +++ b/fluree-db-iceberg/src/scan/topk.rs @@ -1,11 +1,13 @@ -//! Scan-side top-k bound engine (PR-5). +//! Scan-side top-k bound engine (PR-5; ASC added in item 8, F-AUD-6). //! -//! For a single-column DESCENDING `ORDER BY … LIMIT k` pushed to the scan, the -//! reader visits files in `upper_bound(sort_col)`-DESC order and maintains the -//! running k-th bound (the smallest of the k largest sort values seen so far). -//! Once the heap is full, any *unread* file whose `upper_bound` is strictly below -//! that bound cannot contain a top-k row, so — because files are visited in -//! bound-DESC order — the reader stops. +//! For a single-column `ORDER BY … LIMIT k` pushed to the scan, the reader visits +//! files in best-first bound order (DESC: `upper_bound(sort_col)` descending; ASC: +//! `lower_bound(sort_col)` ascending) and maintains the running k-th bound (the +//! worst of the k best sort values seen so far — smallest of the k largest for +//! DESC, largest of the k smallest for ASC). Once the heap is full, any *unread* +//! file whose best-possible bound is strictly worse than the k-th bound cannot +//! contain a top-k row, so — because files are visited best-first — the reader +//! stops. //! //! # Soundness (strict-superset, like [`crate::scan::pruning`]) //! @@ -19,30 +21,54 @@ //! bound is read (a tie could belong in the result; the sort above resolves the //! exact order). The engine therefore over-keeps, never over-prunes. //! -//! DESC only. ASC is declined upstream (SPARQL orders unbound values *first* in -//! ASC, so a NULL-bearing file can never be pruned — not a clean mirror). +//! Both directions are supported, gated at admission: +//! - **DESC** is admitted for any scalar column. SPARQL orders unbound (NULL) +//! values LAST under DESC, so they never form the descending bound and a +//! NULL-bearing file is handled by the `is_full`/no-bound-last rules. +//! - **ASC** (item 8, F-AUD-6) is admitted ONLY when the sort column is REQUIRED +//! (non-nullable per the Iceberg schema) — the provider enforces this gate. A +//! required column has no NULL rows, so the SPARQL "unbound first" ordering +//! cannot place an unread NULL ahead of the top-k, and the ASC mirror (read by +//! `lower_bound` ASC, stop when the next `lower_bound` is strictly ABOVE the +//! k-th bound) is sound. Ignoring the directive is always correct — the +//! `SortOperator` above applies the exact order + LIMIT. use crate::io::batch::{Column, ColumnBatch}; use crate::manifest::value_codec::TypedValue; use crate::manifest::{decode_by_type_string, DataFile}; -use std::cmp::{Ordering, Reverse}; +use std::cmp::Ordering; use std::collections::BinaryHeap; -/// Directive handed to the scan for a single-column DESC top-k pushdown. +/// Directive handed to the scan for a single-column top-k pushdown. #[derive(Debug, Clone, PartialEq, Eq)] pub struct TopKConfig { - /// Iceberg field id of the primary DESC sort column. + /// Iceberg field id of the primary sort column. pub sort_field_id: i32, /// How many top rows the bound must retain — the query's `LIMIT + OFFSET`. pub k: usize, + /// `true` for `ASC` (read by `lower_bound`), `false` for `DESC` (by + /// `upper_bound`). ASC is only ever set for a REQUIRED column (provider gate). + pub ascending: bool, } -/// Total-order wrapper over a same-typed `TypedValue` for heap ordering. In -/// practice one column has one type; `partial_cmp` is total except for float -/// NaN, which we fold to `Equal` (a NaN sort key only ever *weakens* pruning — -/// the file-bound compare is NaN-safe and keeps — never breaks correctness). +/// Total-order wrapper over a same-typed `TypedValue`, oriented **worst-first**: +/// the heap root is the value the bound should evict — the current k-th bound. +/// For DESC (retain the k LARGEST) the worst is the smallest; for ASC (retain the +/// k SMALLEST) the worst is the largest, so the value order is reversed. In +/// practice one column has one type; `partial_cmp` is total except for float NaN, +/// which folds to `Equal` (a NaN sort key only ever *weakens* pruning — the +/// file-bound compare is NaN-safe and keeps — never breaks correctness). #[derive(Debug, Clone)] -struct OrdKey(TypedValue); +struct OrdKey { + v: TypedValue, + ascending: bool, +} + +impl OrdKey { + fn new(v: TypedValue, ascending: bool) -> Self { + Self { v, ascending } + } +} impl PartialEq for OrdKey { fn eq(&self, other: &Self) -> bool { @@ -57,7 +83,15 @@ impl PartialOrd for OrdKey { } impl Ord for OrdKey { fn cmp(&self, other: &Self) -> Ordering { - self.0.partial_cmp(&other.0).unwrap_or(Ordering::Equal) + let base = self.v.partial_cmp(&other.v).unwrap_or(Ordering::Equal); + // Worst-first: DESC evicts the smallest value (→ smallest is the "greatest" + // key → reverse); ASC evicts the largest value (→ largest is "greatest" → + // as-is). + if self.ascending { + base + } else { + base.reverse() + } } } @@ -65,15 +99,17 @@ impl Ord for OrdKey { #[derive(Debug)] pub struct TopKBound { k: usize, - /// Min-heap of the k largest values seen — the root is the current k-th - /// (smallest retained) value once full. - heap: BinaryHeap>, + ascending: bool, + /// Max-heap of `OrdKey`s in worst-first order — the root is the worst retained + /// value (the current k-th bound once full). + heap: BinaryHeap, } impl TopKBound { - pub fn new(k: usize) -> Self { + pub fn new(k: usize, ascending: bool) -> Self { Self { k, + ascending, heap: BinaryHeap::with_capacity(k.saturating_add(1)), } } @@ -83,15 +119,18 @@ impl TopKBound { if self.k == 0 { return; } + let key = OrdKey::new(v, self.ascending); if self.heap.len() < self.k { - self.heap.push(Reverse(OrdKey(v))); + self.heap.push(key); return; } - // Replace the current k-th (smallest retained) if v is strictly larger. - if let Some(Reverse(min)) = self.heap.peek() { - if OrdKey(v.clone()).cmp(min) == Ordering::Greater { + // Replace the worst retained (heap root) if the new value is strictly + // BETTER — i.e. a "smaller" worst-first key (larger for DESC, smaller for + // ASC). A tie never replaces. + if let Some(worst) = self.heap.peek() { + if key.cmp(worst) == Ordering::Less { self.heap.pop(); - self.heap.push(Reverse(OrdKey(v))); + self.heap.push(key); } } } @@ -108,52 +147,69 @@ impl TopKBound { self.heap.len() >= self.k && self.k > 0 } - /// The k-th bound (smallest of the retained top-k), or `None` until full. + /// The k-th bound (worst of the retained top-k), or `None` until full. pub fn kth(&self) -> Option<&TypedValue> { if self.is_full() { - self.heap.peek().map(|Reverse(OrdKey(v))| v) + self.heap.peek().map(|k| &k.v) } else { None } } - /// Whether the scan may stop before reading the next file, whose (highest - /// remaining) `upper_bound` is `next_upper`. Stop iff the heap is full and - /// `next_upper` is strictly below the k-th bound. A `None` next bound (all-NULL - /// column / missing stats) never stops (must read); a tie (`==`) never stops - /// (over-keep); a NaN compare never stops (`lt` → None → false). - pub fn can_stop(&self, next_upper: Option<&TypedValue>) -> bool { - match (self.kth(), next_upper) { - (Some(kth), Some(u)) => u.lt(kth).unwrap_or(false), + /// Whether the scan may stop before reading the next file, whose best-possible + /// sort value in the read direction (`upper_bound` for DESC, `lower_bound` for + /// ASC) is `next_bound`. Stop iff the heap is full and `next_bound` is strictly + /// WORSE than the k-th bound (below it for DESC, above it for ASC). A `None` + /// next bound (all-NULL column / missing stats) never stops (must read); a tie + /// (`==`) never stops (over-keep); a NaN compare never stops. + pub fn can_stop(&self, next_bound: Option<&TypedValue>) -> bool { + match (self.kth(), next_bound) { + (Some(kth), Some(nb)) => { + OrdKey::new(nb.clone(), self.ascending) + .cmp(&OrdKey::new(kth.clone(), self.ascending)) + == Ordering::Greater + } _ => false, } } } -/// Plan the DESC top-k read order over a set of data files: pairs of -/// `(original_index, decoded upper_bound of the sort column)` sorted so the -/// highest `upper_bound` is read first and **files with no `upper_bound` (an -/// all-NULL column or missing stats) come LAST** — they can never stop the scan -/// (see [`TopKBound::can_stop`]) and must be read. The read loop visits the -/// returned order and, after each file, consults the NEXT pair's bound to decide -/// whether to stop. +/// Plan the top-k read order over a set of data files: pairs of `(original_index, +/// decoded best-possible sort bound)` sorted so the best-possible file is read +/// first — highest `upper_bound` first for DESC, lowest `lower_bound` first for +/// ASC — and **files with no bound (an all-NULL column or missing stats) come +/// LAST**: they can never stop the scan (see [`TopKBound::can_stop`]) and must be +/// read. The read loop visits the returned order and, after each file, consults +/// the NEXT pair's bound to decide whether to stop. pub fn plan_topk_read<'a>( data_files: impl Iterator, sort_field_id: i32, sort_type: Option<&str>, + ascending: bool, ) -> Vec<(usize, Option)> { let mut order: Vec<(usize, Option)> = data_files .enumerate() .map(|(i, df)| { - let bound = df - .upper_bound(sort_field_id) - .and_then(|bytes| decode_by_type_string(bytes, sort_type).ok()); + let bytes = if ascending { + df.lower_bound(sort_field_id) + } else { + df.upper_bound(sort_field_id) + }; + let bound = bytes.and_then(|b| decode_by_type_string(b, sort_type).ok()); (i, bound) }) .collect(); order.sort_by(|(_, a), (_, b)| match (a, b) { - // DESC by value; a stable sort preserves the manifest order within ties. - (Some(x), Some(y)) => y.partial_cmp(x).unwrap_or(Ordering::Equal), + // Best-first: ASC smallest lower_bound first, DESC largest upper_bound + // first. A stable sort preserves the manifest order within ties. + (Some(x), Some(y)) => { + let base = x.partial_cmp(y).unwrap_or(Ordering::Equal); + if ascending { + base + } else { + base.reverse() + } + } (Some(_), None) => Ordering::Less, // bound-present before no-bound (None, Some(_)) => Ordering::Greater, (None, None) => Ordering::Equal, @@ -230,7 +286,7 @@ mod tests { #[test] fn bound_forms_only_when_full() { - let mut b = TopKBound::new(3); + let mut b = TopKBound::new(3, false); assert!(!b.is_full()); assert_eq!(b.kth(), None); b.observe(f(10.0)); @@ -248,7 +304,7 @@ mod tests { #[test] fn larger_values_evict_the_kth() { - let mut b = TopKBound::new(3); + let mut b = TopKBound::new(3, false); b.observe_all([f(10.0), f(20.0), f(30.0)]); assert_eq!(b.kth(), Some(&f(10.0))); b.observe(f(25.0)); // evicts 10 → top-3 {20,25,30}, k-th = 20 @@ -261,7 +317,7 @@ mod tests { #[test] fn can_stop_semantics() { - let mut b = TopKBound::new(3); + let mut b = TopKBound::new(3, false); // Not full → never stop, whatever the next bound. assert!(!b.can_stop(Some(&f(1.0)))); b.observe_all([f(10.0), f(20.0), f(30.0)]); // k-th = 10 @@ -277,7 +333,7 @@ mod tests { #[test] fn k_zero_is_inert() { - let mut b = TopKBound::new(0); + let mut b = TopKBound::new(0, false); b.observe(f(10.0)); assert!(!b.is_full()); assert_eq!(b.kth(), None); @@ -288,7 +344,7 @@ mod tests { fn nan_never_over_prunes() { // A NaN sort value folded in must not produce a bound that prunes a file // holding real values (the file-bound compare stays NaN-safe → keep). - let mut b = TopKBound::new(2); + let mut b = TopKBound::new(2, false); b.observe(f(f64::NAN)); b.observe(f(5.0)); // Full (2 values), but the k-th may be NaN; a NaN k-th makes `u.lt(NaN)` @@ -333,7 +389,7 @@ mod tests { df_with_upper(1, None, 5), // 2 — all-null column, no upper_bound df_with_upper(1, Some(20.0), 0), // 3 ]; - let order = plan_topk_read(dfs.iter(), 1, Some("double")); + let order = plan_topk_read(dfs.iter(), 1, Some("double"), false); let idxs: Vec = order.iter().map(|(i, _)| *i).collect(); // DESC by upper_bound: 30(1), 20(3), 10(0); the no-bound file (2) LAST. assert_eq!(idxs, vec![1, 3, 0, 2]); @@ -376,8 +432,8 @@ mod tests { df_with_upper(1, Some(3000.0), 0), df_with_upper(1, None, 2), ]; - let order = plan_topk_read(dfs.iter(), 1, Some("double")); - let mut bound = TopKBound::new(2); + let order = plan_topk_read(dfs.iter(), 1, Some("double"), false); + let mut bound = TopKBound::new(2, false); let mut read = 0usize; for pos in 0..order.len() { let (orig, _) = order[pos]; @@ -407,8 +463,8 @@ mod tests { df_with_upper(1, Some(3.0), 0), df_with_upper(1, None, 1), // all-null ]; - let order = plan_topk_read(dfs.iter(), 1, Some("double")); - let mut bound = TopKBound::new(5); // k=5 > 2 non-null values + let order = plan_topk_read(dfs.iter(), 1, Some("double"), false); + let mut bound = TopKBound::new(5, false); // k=5 > 2 non-null values let mut read = 0usize; for pos in 0..order.len() { let (orig, _) = order[pos]; @@ -426,7 +482,7 @@ mod tests { #[test] fn works_for_int_and_string_keys() { - let mut bi = TopKBound::new(2); + let mut bi = TopKBound::new(2, false); bi.observe_all([ TypedValue::Int64(3), TypedValue::Int64(9), @@ -436,7 +492,7 @@ mod tests { assert!(bi.can_stop(Some(&TypedValue::Int64(4)))); assert!(!bi.can_stop(Some(&TypedValue::Int64(5)))); - let mut bs = TopKBound::new(2); + let mut bs = TopKBound::new(2, false); bs.observe_all([ TypedValue::String("apple".into()), TypedValue::String("mango".into()), @@ -446,4 +502,100 @@ mod tests { assert_eq!(bs.kth(), Some(&TypedValue::String("cherry".into()))); assert!(bs.can_stop(Some(&TypedValue::String("banana".into())))); } + + // ---- Item 8 (F-AUD-6): ASC top-k (the DESC mirror) ---- + + fn df_with_lower(field_id: i32, lower: Option, null_count: i64) -> DataFile { + use std::collections::HashMap; + let lower_bounds = lower.map(|l| { + let mut m = HashMap::new(); + m.insert(field_id, l.to_le_bytes().to_vec()); + m + }); + let null_value_counts = { + let mut m = HashMap::new(); + m.insert(field_id, null_count); + Some(m) + }; + DataFile { + file_path: "t.parquet".to_string(), + file_format: crate::manifest::FileFormat::Parquet, + record_count: 10, + file_size_in_bytes: 1, + partition: crate::manifest::PartitionData::default(), + column_sizes: None, + value_counts: None, + null_value_counts, + nan_value_counts: None, + lower_bounds, + upper_bounds: None, + split_offsets: None, + sort_order_id: None, + } + } + + #[test] + fn asc_bound_retains_smallest_and_stops_above_kth() { + // ASC retains the k SMALLEST; the k-th bound is the LARGEST retained. + let mut b = TopKBound::new(3, true); + b.observe_all([f(10.0), f(20.0), f(30.0)]); + assert_eq!(b.kth(), Some(&f(30.0)), "k-th = largest of the bottom-3"); + b.observe(f(5.0)); // evicts 30 → {5,10,20}, k-th = 20 + assert_eq!(b.kth(), Some(&f(20.0))); + b.observe(f(25.0)); // above k-th (worse for ASC) → ignored + assert_eq!(b.kth(), Some(&f(20.0))); + // Next file's lowest value strictly ABOVE the k-th → STOP; a tie / below + // never stops. + assert!(b.can_stop(Some(&f(20.001)))); + assert!(!b.can_stop(Some(&f(20.0)))); + assert!(!b.can_stop(Some(&f(1.0)))); + assert!(!b.can_stop(None)); + } + + #[test] + fn asc_plan_orders_by_lower_bound_ascending_no_bound_last() { + let dfs = [ + df_with_lower(1, Some(10.0), 0), // 0 + df_with_lower(1, Some(30.0), 0), // 1 + df_with_lower(1, None, 0), // 2 — no lower_bound (missing stats) + df_with_lower(1, Some(20.0), 0), // 3 + ]; + let order = plan_topk_read(dfs.iter(), 1, Some("double"), true); + let idxs: Vec = order.iter().map(|(i, _)| *i).collect(); + // ASC by lower_bound: 10(0), 20(3), 30(1); the no-bound file (2) LAST. + assert_eq!(idxs, vec![0, 3, 1, 2]); + assert_eq!(order.last().unwrap().1, None); + } + + #[test] + fn asc_read_loop_prunes_high_files_after_heap_fills() { + // Files ordered by ascending lower_bound; each of the first two holds one + // of the two smallest values (plus a high decoy). Once the bottom-2 are + // seen, the higher file is provably not in the ASC top-2 and is pruned. + let vals = [ + vec![Some(1.0), Some(5000.0)], // 0: holds the min (+ high decoy) + vec![Some(2.0), Some(6000.0)], // 1: holds the 2nd (+ high decoy) + vec![Some(3000.0)], // 2: above the bottom-2 once full + ]; + let dfs = [ + df_with_lower(1, Some(1.0), 0), + df_with_lower(1, Some(2.0), 0), + df_with_lower(1, Some(3000.0), 0), + ]; + let order = plan_topk_read(dfs.iter(), 1, Some("double"), true); + let mut bound = TopKBound::new(2, true); + let mut read = 0usize; + for pos in 0..order.len() { + let (orig, _) = order[pos]; + read += 1; + bound.observe_all(vals[orig].iter().flatten().map(|&x| f(x))); + if let Some((_, next_lower)) = order.get(pos + 1) { + if bound.can_stop(next_lower.as_ref()) { + break; + } + } + } + assert_eq!(read, 2, "read only the 2 files holding the ASC top-2"); + assert_eq!(bound.kth(), Some(&f(2.0)), "k-th = larger of {{1,2}}"); + } } diff --git a/fluree-db-query/src/aggregate.rs b/fluree-db-query/src/aggregate.rs index 0d8703acf..a0e67fd75 100644 --- a/fluree-db-query/src/aggregate.rs +++ b/fluree-db-query/src/aggregate.rs @@ -151,6 +151,16 @@ impl AggregateOperator { #[async_trait] impl Operator for AggregateOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — an aggregate consumes ALL input to + /// compute its result (a single output row folds every input row). Explicit + /// (was a silent trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "AGGREGATE row-budget swallowed (unsound to forward: folds all input)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/dataset_operator.rs b/fluree-db-query/src/dataset_operator.rs index 447894223..f3edd444a 100644 --- a/fluree-db-query/src/dataset_operator.rs +++ b/fluree-db-query/src/dataset_operator.rs @@ -194,7 +194,7 @@ pub struct DatasetOperator { /// T1.3: an `ORDER BY … LIMIT` top-k directive, applied per member like /// `row_budget`. Per-member top-k is sound — the outer sort merges the members' /// partial top-k into the global one. - topk: Option<(VarId, usize)>, + topk: Option<(VarId, usize, bool)>, } impl DatasetOperator { @@ -221,8 +221,8 @@ impl DatasetOperator { if let Some(budget) = self.row_budget { member.set_row_budget(budget); } - if let Some((sort_var, k)) = self.topk { - member.set_topk(sort_var, k); + if let Some((sort_var, k, ascending)) = self.topk { + member.set_topk(sort_var, k, ascending); } } @@ -381,13 +381,13 @@ impl Operator for DatasetOperator { } } - fn set_topk(&mut self, sort_var: VarId, k: usize) { + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { // T1.3: record ORDER BY … LIMIT top-k; applied per member like the row // budget. Per-member top-k is sound — the outer sort merges the members' // partial top-k into the global one (same reasoning as `GraphOperator`'s // per-partition top-k). Switch-gated. if crate::r2rml::dataset_budget_enabled() { - self.topk = Some((sort_var, k)); + self.topk = Some((sort_var, k, ascending)); } } @@ -626,7 +626,7 @@ mod tests { /// freshly built member. struct DirectiveRecorder { budget: Arc>>, - topk: Arc>>, + topk: Arc>>, schema: Arc<[VarId]>, state: OperatorState, } @@ -650,8 +650,8 @@ mod tests { fn set_row_budget(&mut self, budget: usize) { *self.budget.lock().unwrap() = Some(budget); } - fn set_topk(&mut self, sort_var: VarId, k: usize) { - *self.topk.lock().unwrap() = Some((sort_var, k)); + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { + *self.topk.lock().unwrap() = Some((sort_var, k, ascending)); } } @@ -659,7 +659,7 @@ mod tests { /// handles, so a test can read what the built member(s) received. struct RecorderBuilder { budget: Arc>>, - topk: Arc>>, + topk: Arc>>, schema: Arc<[VarId]>, } @@ -679,7 +679,7 @@ mod tests { type RecorderHandles = ( Arc>>, - Arc>>, + Arc>>, ); fn recorder_dataset() -> (DatasetOperator, RecorderHandles) { @@ -721,9 +721,9 @@ mod tests { #[tokio::test] async fn dataset_forwards_topk_to_member() { let (mut op, (_budget, topk)) = recorder_dataset(); - op.set_topk(VarId(3), 5); + op.set_topk(VarId(3), 5, false); open_and_drain(&mut op).await; - assert_eq!(*topk.lock().unwrap(), Some((VarId(3), 5))); + assert_eq!(*topk.lock().unwrap(), Some((VarId(3), 5, false))); } #[tokio::test] diff --git a/fluree-db-query/src/distinct.rs b/fluree-db-query/src/distinct.rs index 2e7bc83d5..2c475751e 100644 --- a/fluree-db-query/src/distinct.rs +++ b/fluree-db-query/src/distinct.rs @@ -87,6 +87,17 @@ impl DistinctOperator { #[async_trait] impl Operator for DistinctOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — DISTINCT may collapse arbitrarily + /// many input rows into one, so producing `k` distinct rows can require + /// unboundedly many input rows. Explicit (was a silent trait-default no-op) so + /// the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "DISTINCT row-budget swallowed (unsound to forward: dedup collapses rows)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/execute/operator_tree.rs b/fluree-db-query/src/execute/operator_tree.rs index e50cb293b..0c665a478 100644 --- a/fluree-db-query/src/execute/operator_tree.rs +++ b/fluree-db-query/src/execute/operator_tree.rs @@ -3380,18 +3380,27 @@ pub(crate) fn apply_solution_modifiers( (Some(limit), None) => limit, _ => 0, }; - // PR-5: offer the primary DESC sort key + k to the row-preserving scan - // below, so a single-column `ORDER BY DESC() LIMIT k` over an - // R2RML scan can read only the files that can hold the top-k. Offered - // only for a DESCENDING primary key (ASC declines — SPARQL orders - // unbound first, so a null-bearing file can't be pruned); the scan - // honors it ONLY if the key resolves to a single scalar scan column, - // else no-op. Pure optimization — this `SortOperator` remains the - // authority for the exact (compound) order + LIMIT. + // PR-5 / item 8 (F-AUD-6): offer the primary sort key + k to the + // row-preserving scan below, so a single-column `ORDER BY + // LIMIT k` over an R2RML scan can read only the files that can hold the + // top-k. DESC is offered for any scalar column; ASC is offered too but + // the scan honors it ONLY when the column is REQUIRED (non-nullable) — + // SPARQL orders unbound values FIRST under ASC, so a nullable column + // could hide an unread top-k row (the provider re-checks). The scan + // honors either direction ONLY if the key resolves to a single scalar + // scan column, else no-op. Pure optimization — this `SortOperator` + // remains the authority for the exact (compound) order + LIMIT. if can_topk { if let Some(primary) = ordering.first() { - if primary.direction == crate::sort::SortDirection::Descending { - operator.set_topk(primary.var, k); + use crate::sort::SortDirection; + match primary.direction { + SortDirection::Descending => operator.set_topk(primary.var, k, false), + // Gated by FLUREE_R2RML_TOPK_ASC: OFF is byte-identical to + // the pre-item-8 DESC-only behavior. + SortDirection::Ascending if crate::r2rml::topk_asc_enabled() => { + operator.set_topk(primary.var, k, true); + } + SortDirection::Ascending => {} } } } @@ -3464,7 +3473,7 @@ mod tests { struct RecordingTopkOp { schema: Vec, - recorded: Arc>>, + recorded: Arc>>, } #[async_trait::async_trait] impl Operator for RecordingTopkOp { @@ -3478,8 +3487,8 @@ mod tests { Ok(None) } fn close(&mut self) {} - fn set_topk(&mut self, sort_var: VarId, k: usize) { - *self.recorded.lock().unwrap() = Some((sort_var, k)); + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { + *self.recorded.lock().unwrap() = Some((sort_var, k, ascending)); } } @@ -3506,8 +3515,68 @@ mod tests { .expect("apply_solution_modifiers"); assert_eq!( *recorded.lock().unwrap(), - Some((VarId(1), 15)), - "k must be LIMIT(10) + OFFSET(5) = 15, so the scan retains the offset rows the sort then skips" + Some((VarId(1), 15, false)), + "k must be LIMIT(10) + OFFSET(5) = 15, so the scan retains the offset rows the sort then skips; DESC ⇒ ascending=false" + ); + } + + /// Item 8 (F-AUD-6): an ASC `ORDER BY … LIMIT` offers a top-k directive too, + /// carrying `ascending = true` (the scan/provider then gates on the column + /// being required). Mirrors the DESC test above. + #[test] + fn topk_directive_ascending_carries_direction() { + use crate::binding::Batch; + use crate::context::ExecutionContext; + use crate::error::Result as QResult; + use crate::operator::{BoxedOperator, Operator}; + use std::sync::{Arc, Mutex}; + + struct RecordingTopkOp { + schema: Vec, + recorded: Arc>>, + } + #[async_trait::async_trait] + impl Operator for RecordingTopkOp { + fn schema(&self) -> &[VarId] { + &self.schema + } + async fn open(&mut self, _ctx: &ExecutionContext<'_>) -> QResult<()> { + Ok(()) + } + async fn next_batch(&mut self, _ctx: &ExecutionContext<'_>) -> QResult> { + Ok(None) + } + fn close(&mut self) {} + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { + *self.recorded.lock().unwrap() = Some((sort_var, k, ascending)); + } + } + + let recorded = Arc::new(Mutex::new(None)); + let op: BoxedOperator = Box::new(RecordingTopkOp { + schema: vec![VarId(0), VarId(1)], + recorded: Arc::clone(&recorded), + }); + let planning = crate::temporal_mode::PlanningContext::current(); + // ORDER BY ASC(?1) LIMIT 10. + let _tree = apply_solution_modifiers( + op, + None, + &[], + &[SortSpec::asc(VarId(1))], + None, + false, + None, + Some(10), + false, + None, + &planning, + ) + .expect("apply_solution_modifiers"); + assert_eq!( + *recorded.lock().unwrap(), + Some((VarId(1), 10, true)), + "ASC ⇒ ascending=true (switch default-on); provider re-gates on required column" ); } diff --git a/fluree-db-query/src/filter.rs b/fluree-db-query/src/filter.rs index c0d29bef8..3592e9d58 100644 --- a/fluree-db-query/src/filter.rs +++ b/fluree-db-query/src/filter.rs @@ -685,6 +685,16 @@ impl FilterOperator { #[async_trait] impl Operator for FilterOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — a FILTER may reject arbitrarily many + /// input rows, so producing `k` output can require unboundedly many input rows. + /// Explicit (was a silent trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "FILTER row-budget swallowed (unsound to forward: a filter rejects rows)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/graph.rs b/fluree-db-query/src/graph.rs index 5a2b0b096..9c137d191 100644 --- a/fluree-db-query/src/graph.rs +++ b/fluree-db-query/src/graph.rs @@ -109,7 +109,7 @@ pub struct GraphOperator { /// top-k row from partition p is among p's k largest, so the global top-k is a /// subset of the union of the per-partition results (the authoritative sort /// above re-selects the exact k). - topk: Option<(VarId, usize)>, + topk: Option<(VarId, usize, bool)>, /// Plan-time decision: seed the enumerated graph variable into the inner /// subplan. True only when the inner patterns bind the graph var in EVERY /// solution (required top-level triple / property path / slice-free @@ -356,8 +356,8 @@ impl GraphOperator { if let Some(budget) = self.row_budget { inner.set_row_budget(budget); } - if let Some((sort_var, k)) = self.topk { - inner.set_topk(sort_var, k); + if let Some((sort_var, k, ascending)) = self.topk { + inner.set_topk(sort_var, k, ascending); } inner.open(&graph_ctx).await?; @@ -535,8 +535,8 @@ impl GraphOperator { if let Some(budget) = self.row_budget { inner.set_row_budget(budget); } - if let Some((sort_var, k)) = self.topk { - inner.set_topk(sort_var, k); + if let Some((sort_var, k, ascending)) = self.topk { + inner.set_topk(sort_var, k, ascending); } inner.open(&graph_ctx).await?; @@ -644,11 +644,11 @@ impl Operator for GraphOperator { self.row_budget = Some(budget); } - fn set_topk(&mut self, sort_var: VarId, k: usize) { + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { // Record the top-k directive; threaded into the per-parent inner subplan // (like `row_budget`). NOT forwarded to `self.child` (the parent seed is // not the scan). Per-partition top-k is sound (see the field doc). - self.topk = Some((sort_var, k)); + self.topk = Some((sort_var, k, ascending)); } async fn open(&mut self, ctx: &ExecutionContext<'_>) -> Result<()> { diff --git a/fluree-db-query/src/group_aggregate.rs b/fluree-db-query/src/group_aggregate.rs index f2a1cd06c..78bac29bf 100644 --- a/fluree-db-query/src/group_aggregate.rs +++ b/fluree-db-query/src/group_aggregate.rs @@ -785,6 +785,17 @@ impl GroupAggregateOperator { #[async_trait] impl Operator for GroupAggregateOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — a grouped aggregate must consume ALL + /// input before any group is closed, so a finite budget cannot bound it (the + /// last input row may open a new group or change an existing one). Explicit + /// (was a silent trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "GROUP-AGGREGATE row-budget swallowed (unsound to forward: groups need all input)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/having.rs b/fluree-db-query/src/having.rs index 9a919a344..1fa38949d 100644 --- a/fluree-db-query/src/having.rs +++ b/fluree-db-query/src/having.rs @@ -76,6 +76,17 @@ impl HavingOperator { #[async_trait] impl Operator for HavingOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — HAVING filters whole GROUPS, so + /// producing `k` surviving groups needs the complete grouped input (an + /// arbitrary number of groups may fail the predicate). Explicit (was a silent + /// trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "HAVING row-budget swallowed (unsound to forward: filters whole groups)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/minus.rs b/fluree-db-query/src/minus.rs index 1fefab170..f0f23f390 100644 --- a/fluree-db-query/src/minus.rs +++ b/fluree-db-query/src/minus.rs @@ -315,6 +315,20 @@ fn rows_match( #[async_trait] impl Operator for MinusOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — MINUS is an anti-join whose output + /// is a SUBSET of the primary (each primary row emits 0 or 1), so a finite + /// budget on the primary can under-produce (the excluded rows don't count + /// toward `k`). This is the SAME reasoning the inner nested-loop join uses to + /// absorb rather than forward (`join.rs`). NOTE: this DEVIATES from the item's + /// brief ("MINUS forwards to primary"), on soundness grounds — see the audit + /// implementation log. Explicit (was a silent trait-default no-op). + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "MINUS row-budget swallowed (unsound: anti-join output ⊆ primary, multiplicity ≤1)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/operator.rs b/fluree-db-query/src/operator.rs index 6e9b5ebd8..d4e3007bd 100644 --- a/fluree-db-query/src/operator.rs +++ b/fluree-db-query/src/operator.rs @@ -100,15 +100,17 @@ pub trait Operator: Send + Sync { /// never inside fused per-row/per-group loops (hot-loop purity). fn set_row_budget(&mut self, _budget: usize) {} - /// Offer a scan-side top-k directive (PR-5): a single-column DESC `ORDER BY - /// LIMIT k` sits directly above a row-preserving operator chain - /// down to a single R2RML scan. Default no-op; only the R2RML scan honors it - /// (and only when `sort_var` resolves to a scalar scan column). Order- - /// preserving pass-through operators forward it to their child so it reaches - /// the scan. It is a pure optimization — a scan may then read only the files - /// that can hold the top-k — and never changes results (the `Sort` above is - /// authoritative). Set **before** `open()`. - fn set_topk(&mut self, _sort_var: VarId, _k: usize) {} + /// Offer a scan-side top-k directive (PR-5; ASC added in item 8, F-AUD-6): a + /// single-column `ORDER BY LIMIT k` sits directly above a + /// row-preserving operator chain down to a single R2RML scan. `ascending` is + /// the sort direction (`false` = DESC). Default no-op; only the R2RML scan + /// honors it (and only when `sort_var` resolves to a scalar scan column — for + /// ASC, also a required/non-nullable column). Order-preserving pass-through + /// operators forward it to their child so it reaches the scan. It is a pure + /// optimization — a scan may then read only the files that can hold the top-k — + /// and never changes results (the `Sort` above is authoritative). Set + /// **before** `open()`. + fn set_topk(&mut self, _sort_var: VarId, _k: usize, _ascending: bool) {} // ------------------------------------------------------------------ // EXPLAIN introspection (never called on the hot path) diff --git a/fluree-db-query/src/optional.rs b/fluree-db-query/src/optional.rs index 1c67f5af8..c3cf1298d 100644 --- a/fluree-db-query/src/optional.rs +++ b/fluree-db-query/src/optional.rs @@ -1941,6 +1941,23 @@ impl Operator for OptionalOperator { effective_schema(&self.out_schema, &self.combined_schema) } + /// Item 11 (F-AUD-7): forward a top-of-tree `LIMIT` budget to the REQUIRED + /// (outer) side only. A left-outer-join emits ≥1 output row per required row + /// (a padded null row when the optional side has no match) and preserves + /// required-row order, so the first `k` output rows come from the first ≤`k` + /// required rows — bounding the required side to `k` is sound (the `LIMIT` + /// above truncates any surplus). The optional (inner) side is deliberately NOT + /// budgeted: it must still produce every match for a given required row. + /// Gated by `FLUREE_R2RML_BUDGET_OPTIONAL`; OFF swallows the budget (the + /// pre-item-11 full outer scan — byte-identical results). + fn set_row_budget(&mut self, budget: usize) { + if crate::r2rml::optional_budget_enabled() { + self.required.set_row_budget(budget); + } else { + tracing::debug!(budget, "OPTIONAL row-budget forwarding disabled by switch"); + } + } + async fn open(&mut self, ctx: &ExecutionContext<'_>) -> Result<()> { if !self.state.can_open() { if self.state.is_closed() { @@ -2381,6 +2398,51 @@ mod tests { assert_eq!(op.schema()[2], VarId(2)); // ?email (optional-only) } + /// Item 11 (F-AUD-7): a top-of-tree `LIMIT` budget is forwarded to the REQUIRED + /// (outer) side of an OPTIONAL — the sound direction (each required row emits + /// ≥1 output), so the outer scan stops early instead of full-scanning (probe-04). + #[test] + fn optional_forwards_row_budget_to_required_side() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let recorded = Arc::new(AtomicUsize::new(0)); + struct RecordingOp { + recorded: Arc, + } + #[async_trait] + impl Operator for RecordingOp { + fn schema(&self) -> &[VarId] { + &[] + } + async fn open(&mut self, _: &ExecutionContext<'_>) -> Result<()> { + Ok(()) + } + async fn next_batch(&mut self, _: &ExecutionContext<'_>) -> Result> { + Ok(None) + } + fn close(&mut self) {} + fn set_row_budget(&mut self, budget: usize) { + self.recorded.store(budget, Ordering::SeqCst); + } + } + + let required_schema: Arc<[VarId]> = Arc::from(vec![VarId(0), VarId(1)].into_boxed_slice()); + let mut op = OptionalOperator::new( + Box::new(RecordingOp { + recorded: Arc::clone(&recorded), + }), + required_schema, + make_optional_pattern(), + crate::temporal_mode::PlanningContext::current(), + ); + op.set_row_budget(50); + assert_eq!( + recorded.load(Ordering::SeqCst), + 50, + "the budget must reach the required (outer) side (switch default-on)" + ); + } + #[test] fn test_pattern_optional_builder() { let required_schema: Arc<[VarId]> = Arc::from(vec![VarId(0), VarId(1)].into_boxed_slice()); diff --git a/fluree-db-query/src/project.rs b/fluree-db-query/src/project.rs index 2ac6a2034..fd4d8ff29 100644 --- a/fluree-db-query/src/project.rs +++ b/fluree-db-query/src/project.rs @@ -59,10 +59,10 @@ impl Operator for ProjectOperator { self.child.set_row_budget(budget); } - fn set_topk(&mut self, sort_var: crate::var_registry::VarId, k: usize) { + fn set_topk(&mut self, sort_var: crate::var_registry::VarId, k: usize, ascending: bool) { // Projection is row- and order-preserving, so a top-k directive passes // straight through to the scan below (mirrors `set_row_budget`). - self.child.set_topk(sort_var, k); + self.child.set_topk(sort_var, k, ascending); } async fn next_batch(&mut self, ctx: &ExecutionContext<'_>) -> Result> { diff --git a/fluree-db-query/src/r2rml/fused_aggregate.rs b/fluree-db-query/src/r2rml/fused_aggregate.rs index 8a02f5abe..4fafb9591 100644 --- a/fluree-db-query/src/r2rml/fused_aggregate.rs +++ b/fluree-db-query/src/r2rml/fused_aggregate.rs @@ -79,6 +79,72 @@ fn numeric_kind(datatype: Option<&str>) -> Option { } } +/// Whether a declared object-map datatype is admissible for a fused MIN/MAX fold: +/// a numeric (`xsd:integer`/`long`/`int`/`decimal`/`double`/`float`) or temporal +/// (`xsd:date`/`dateTime`) type. Everything else — string, boolean, IRI-/langtag- +/// typed, or an **un-annotated** column (which the R2RML natural mapping treats as +/// `xsd:string`, so its MIN/MAX would be lexical) — declines, keeping the generic +/// pipeline's collation/term-type semantics. Fold parity itself comes from +/// materializing the same term + `compare_bindings`; this gate only scopes the +/// mechanism to the types the audit item covers (F-AUD-8). +fn minmax_admissible_datatype(datatype: Option<&str>) -> bool { + use fluree_vocab::xsd; + matches!( + datatype, + Some(dt) if dt == xsd::INTEGER + || dt == xsd::LONG + || dt == xsd::INT + || dt == xsd::DECIMAL + || dt == xsd::DOUBLE + || dt == xsd::FLOAT + || dt == xsd::DATE + || dt == xsd::DATE_TIME + ) +} + +/// Whether a MIN/MAX fold should replace its running extreme with a candidate +/// whose `compare_bindings(candidate, current)` is `ord`. This mirrors the +/// generic `agg_min`/`agg_max` (`min_by`/`max_by`) tie-breaking EXACTLY: +/// - MIN keeps the FIRST minimum (`min_by` returns the first of equal-minimums), +/// so it replaces only on a strictly-less candidate; +/// - MAX keeps the LAST maximum (`max_by` returns the last of equal-maximums), +/// so it replaces on greater-OR-EQUAL — the later of two equal elements wins. +/// The `Equal` case is load-bearing: two values that compare equal can still +/// RENDER differently (double `+0.0` vs `-0.0` → "0" vs "-0"; or a decimal at two +/// scales — `1.50` vs `1.5`), so picking the wrong one breaks byte-parity with the +/// materialized aggregate even though the values are "equal". Candidates are +/// materialized before the compare, so replacing on `Equal` costs no extra work. +fn minmax_should_replace(is_max: bool, ord: std::cmp::Ordering) -> bool { + if is_max { + ord != std::cmp::Ordering::Less + } else { + ord == std::cmp::Ordering::Less + } +} + +/// Whether the bare-COUNT manifest shortcut is eligible for a resolved fused plan: +/// exactly one `CountRows` fold, no GROUP BY, no FILTER, and no folded +/// constant-object constraints. The Iceberg manifest `record_count` sum cannot see +/// per-row FILTER/constraint matches or per-group partitions, so anything else must +/// fall through to the scan-fold (which applies them). This is the D-c5 soundness +/// line for item 9b in particular: a constraint-bearing COUNT (e.g. `isCurrent +/// true`) MUST decline the delete-blind shortcut and count matching rows in the +/// fold instead. Extracted as a pure predicate so the decline invariant is +/// DIRECTLY unit-tested, not only verified by inspection (R-1522 verified it that +/// way). `filter_present` is passed as a bool because a `FilterPlan` needs a live +/// `LedgerSnapshot` to build — the shortcut only ever cares about its presence. +fn count_shortcut_eligible( + filter_present: bool, + group_cols: &[GroupCol], + fact_constraints: &[ResolvedConstraint], + folds: &[Fold], +) -> bool { + !filter_present + && group_cols.is_empty() + && fact_constraints.is_empty() + && matches!(folds, [Fold::CountRows]) +} + /// How to read a numeric column value as an exact decimal during native /// expression evaluation. #[derive(Clone, Copy)] @@ -379,14 +445,19 @@ pub fn detect_fused_r2rml_aggregate(query: &Query) -> Option // Every aggregate must be a column fold this operator supports. let mut aggregates = Vec::with_capacity(aggregation.aggregates.len()); for spec in aggregation.aggregates.iter() { - // Only multiset (non-DISTINCT) COUNT/SUM/AVG fold from columns; the fused - // path has no dedup, so DISTINCT (Set) must fall back to the normal - // pipeline. `CountDistinct` is already a separate, unmatched variant. + // Only multiset (non-DISTINCT) COUNT/SUM/AVG and MIN/MAX fold from columns; + // the fused path has no dedup, so DISTINCT (Set) must fall back to the + // normal pipeline. `CountDistinct` is already a separate, unmatched variant. + // MIN/MAX carry no DISTINCT flag (dedup is a no-op for them). Whether a + // MIN/MAX aggregate variable actually resolves to a foldable numeric/temporal + // column is decided at `open` (`resolve_agg_folds`), which declines string / + // language- / IRI-typed / un-annotated columns. let foldable = match &spec.function { AggregateFn::CountAll | AggregateFn::Count(_) => true, AggregateFn::Sum(_, sem) | AggregateFn::Avg(_, sem) => { matches!(sem, InputSemantics::List) } + AggregateFn::Min(_) | AggregateFn::Max(_) => true, _ => false, }; if !foldable { @@ -437,6 +508,14 @@ enum Fold { /// `SUM(expr)` / `AVG(expr)` over a native decimal arithmetic expression; /// `index` points into `Resolved::expr_folds`. NumericExpr { index: usize, is_avg: bool }, + /// `MIN(?col)` / `MAX(?col)` over a numeric or date/timestamp scalar column; + /// `index` points into `Resolved::minmax_folds`. Unlike the COUNT/SUM/AVG + /// folds this one materializes the candidate object term (via the same + /// `materialize_object_from_batch` + `LiteralEncoder` path the FILTER fold + /// uses) and keeps the running extreme by `compare_bindings` — byte-parity + /// with the generic `agg_min`/`agg_max`, but streaming (O(1) memory) instead + /// of buffering every value and skipping the subject/BindingRow build. + MinMax { index: usize }, } /// Running accumulator for one [`Fold`], mutated per batch in `next_batch`. @@ -465,12 +544,22 @@ enum Acc { count: u64, is_avg: bool, }, + /// Running MIN/MAX extreme, held as the materialized winning `Binding` + /// (`Unbound` until the first non-null row). The extreme is updated by + /// `compare_bindings` in the fold loop (the `Fold::MinMax` arm), so this is the + /// exact `Binding` the generic `agg_min`/`agg_max` would return. + MinMax { + best: Binding, + }, } impl Acc { fn for_fold(fold: &Fold) -> Self { match fold { Fold::CountRows | Fold::CountColumn(_) => Acc::Count(0), + Fold::MinMax { .. } => Acc::MinMax { + best: Binding::Unbound, + }, Fold::NumericExpr { is_avg, .. } => Acc::Expr { sum: 0, scale: 0, @@ -533,6 +622,9 @@ impl Acc { /// Materialize the final result binding for this accumulator. fn finalize(self) -> Binding { match self { + // The extreme is already the materialized winning term (or `Unbound` + // for an empty group), byte-identical to `agg_min`/`agg_max`. + Acc::MinMax { best } => best, Acc::Count(n) => Binding::lit(FlakeValue::Long(n as i64), Sid::xsd_integer()), Acc::Exact { sum, @@ -803,6 +895,14 @@ struct Resolved { /// for a join with no fact-side flag. Dim-side constraints are applied earlier, /// during the FK→GKey map build, so they are not carried here. fact_constraints: Vec, + /// MIN/MAX fold plans, indexed by `Fold::MinMax.index`. Empty unless the query + /// carries a MIN/MAX aggregate. The aggregates always fold from the FACT scan + /// (single-table or join), so these read the fact batch. + minmax_folds: Vec, + /// Shared literal encoder for materializing MIN/MAX candidate terms into + /// `Binding`s (datatype Sids pre-resolved from the fact TriplesMap). `None` + /// when there are no MIN/MAX folds. + minmax_encoder: Option, } /// PR-6 fact⋈dim group-key resolver, built once at `open` by scanning the small @@ -826,6 +926,19 @@ struct ExprFold { var_cols: Vec<(VarId, String, DecKind)>, } +/// A `MIN(?col)` / `MAX(?col)` fold plan. The object map materializes the +/// candidate term per non-null row via the same `materialize_object_from_batch` +/// path the FILTER fold uses (so the term — value + datatype/lang/term-type — is +/// byte-identical to the generic scan's), and the running extreme is kept by +/// `compare_bindings`. The scan column is projected at resolve time; the fold +/// materializes through `object_map`, which carries the column reference +/// internally. Admitted only for numeric/temporal plain-literal columns +/// (`minmax_admissible_datatype` + a `TermType::Literal`, no-lang gate). +struct MinMaxFold { + object_map: ObjectMap, + is_max: bool, +} + /// Per-row FILTER evaluation plan. The filter expression is evaluated through the /// engine's own evaluator (`PreparedBoolExpression`) against a `BindingRow` built /// from the referenced object columns, so semantics are identical to the normal @@ -1129,11 +1242,12 @@ impl Operator for FusedR2rmlAggregateOperator { // whole fused path (a disabled switch fails detection, so this is never // reached). The emitted binding is byte-identical to the scan+fold result // (`Acc::Count(n).finalize()`). - if resolved.filter.is_none() - && resolved.group_cols.is_empty() - && resolved.fact_constraints.is_empty() - && matches!(resolved.folds.as_slice(), [Fold::CountRows]) - { + if count_shortcut_eligible( + resolved.filter.is_some(), + &resolved.group_cols, + &resolved.fact_constraints, + &resolved.folds, + ) { let gs = resolved.pattern.graph_source_id.clone(); let table = resolved.table_name.clone(); let non_null_cols = resolved.count_non_null_cols.clone(); @@ -1185,7 +1299,9 @@ impl Operator for FusedR2rmlAggregateOperator { let fold_cols: Vec> = folds .iter() .map(|f| match f { - Fold::CountRows | Fold::NumericExpr { .. } => None, + // MinMax materializes via its object map (handled inline below), + // not a bare column read. + Fold::CountRows | Fold::NumericExpr { .. } | Fold::MinMax { .. } => None, Fold::CountColumn(c) | Fold::Numeric { column: c, .. } => { batch.column_by_name(c) } @@ -1321,6 +1437,34 @@ impl Operator for FusedR2rmlAggregateOperator { _ => true, } } + Fold::MinMax { index } => { + let mmf = &resolved.minmax_folds[*index]; + // Materialize just the candidate object term (no subject, + // no BindingRow) exactly as the generic scan would, then + // keep the running extreme by `compare_bindings` — so the + // final `Binding` is byte-identical to `agg_min`/`agg_max`. + // A null/absent value materializes `None` and contributes + // nothing, matching those aggregates dropping `Unbound`. + if let (Some(encoder), Ok(Some(term))) = ( + resolved.minmax_encoder.as_ref(), + materialize_object_from_batch(&mmf.object_map, &batch, row), + ) { + let cand = encoder.encode(&term); + if let Acc::MinMax { best } = &mut accs[i] { + let replace = match &*best { + Binding::Unbound => true, + cur => minmax_should_replace( + mmf.is_max, + crate::sort::compare_bindings(&cand, cur), + ), + }; + if replace { + *best = cand; + } + } + } + true + } _ => accs[i].update_row(fold, fold_cols[i], row), }; if !ok { @@ -1376,22 +1520,6 @@ impl Operator for FusedR2rmlAggregateOperator { } } -/// C5 slice-1 MANDATORY soundness guard: whether ANY participating R2RML pattern -/// (the aggregated fact AND every joined dim) carries a `star_constraints` entry — -/// a constant-object member the rewrite folded into the pattern (e.g. -/// `?c ex:isCurrent true` → `star_constraints=[(IS_CURRENT, true)]`, `rewrite.rs:344`). -/// The fused fold reads only the GROUP-BY/aggregate/validity columns + the SPARQL -/// FILTER; it has NO star_constraints handling (`grep star_constraints -/// fused_aggregate.rs` = 0), while the normal scan DOES apply them -/// (`operator.rs:524/1719/…`). So a fused plan over a constrained pattern would -/// silently IGNORE the constraint and OVER-COUNT. Declining here keeps the fold -/// sound; slice 1.5 teaches the fold to apply the constraint and lifts this for -/// that shape. This is also a *current* correctness fix: any COUNT-over-a-flagged- -/// star that reaches the fused path today over-counts. -fn fold_over_star_constraints(pats: &[&R2rmlPattern]) -> bool { - pats.iter().any(|p| !p.star_constraints.is_empty()) -} - /// E2: a folded constant-object constraint (`star_constraints`) resolved to a /// per-row scalar-column check. The predicate's column `PredicateObjectMap` is /// materialized per row and compared to `constant` with the normal scan's @@ -1535,12 +1663,19 @@ impl FusedR2rmlAggregateOperator { _ => return Ok(None), // join sub-switch off, or non-R2rml pattern }; - // C5 slice-1 MANDATORY guard: a folded constant-object constraint - // (star_constraints, e.g. `?c ex:isCurrent true`) on the single-table - // pattern is NOT applied by the fold and would OVER-COUNT. Decline. - if fold_over_star_constraints(&[&pattern]) { - return Ok(None); - } + // C5 slice-1.5 (item 9b, the q038 filtered-COUNT class): a folded + // constant-object constraint (`star_constraints`, e.g. `?s edw:isCurrent + // true`) on the single-table pattern is NO LONGER a blanket decline — the + // fold now APPLIES it (resolved below, once the TriplesMap is known). Slice-1 + // declined here because the fold ignored the constraint and would OVER-COUNT; + // that hazard is retired by resolving the constraint to a per-row scalar-column + // check via the SAME `resolve_star_constraint_checks` / `row_satisfies_constraints` + // machinery the join path (E2) already uses, so a constrained COUNT excludes + // the non-matching rows exactly as the materialized answer does. A constraint + // that does not resolve to a scalar column still declines (below), and the + // COUNT(*) manifest shortcut stays declined for a constraint-bearing plan (it + // checks `fact_constraints.is_empty()`), because `record_count` cannot see + // per-row constraint matches. // The graph is genuinely R2RML-backed here; without the mapping fall back // to the normal path (which surfaces any real load error). @@ -1555,7 +1690,19 @@ impl FusedR2rmlAggregateOperator { return Ok(None); }; + // Resolve the fact-side folded constant-object constraints to per-row + // scalar-column checks. Declines (`None`) if any constraint's predicate is a + // RefObjectMap object or is absent — those keep the materialize path (no + // silent over-count). Empty for an unconstrained COUNT/SUM/AVG/MIN/MAX plan. + let Some(fact_constraints) = Self::resolve_star_constraint_checks(&pattern, tm) else { + return Ok(None); + }; + let mut projection: Vec = Vec::new(); + // Scan the constraint columns so the fold can enforce them per row. + for c in &fact_constraints { + projection.push(c.column().to_string()); + } // Resolve GROUP BY key columns (string / integer in slice 3). The output // key binding's datatype Sid is encoded from the snapshot so it matches @@ -1616,10 +1763,17 @@ impl FusedR2rmlAggregateOperator { } // Resolve the aggregate output folds against the (single) scanned TM. - let (folds, expr_folds) = match self.resolve_agg_folds(&pattern, tm, &mut projection) { - Some(x) => x, - None => return Ok(None), - }; + let mut minmax_folds: Vec = Vec::new(); + let (folds, expr_folds) = + match self.resolve_agg_folds(&pattern, tm, &mut projection, &mut minmax_folds) { + Some(x) => x, + None => return Ok(None), + }; + // MIN/MAX materializes candidate terms via this encoder (built once, from + // the scanned TriplesMap's datatype annotations — the same datatype Sids the + // generic scan uses). + let minmax_encoder = + (!minmax_folds.is_empty()).then(|| LiteralEncoder::build(tm, ctx.active_snapshot)); // Row-validity columns. A row participates only if the subject template // columns and every predicate's object column are non-null — mirroring @@ -1702,7 +1856,9 @@ impl FusedR2rmlAggregateOperator { group_resolver: None, // Single-table fold: every key reads inline from the fact batch. group_key_plan: (0..self.group_by.len()).map(|_| KeySource::Fact).collect(), - fact_constraints: Vec::new(), + fact_constraints, + minmax_folds, + minmax_encoder, })) } @@ -1717,6 +1873,7 @@ impl FusedR2rmlAggregateOperator { pattern: &R2rmlPattern, tm: &TriplesMap, projection: &mut Vec, + minmax_folds: &mut Vec, ) -> Option<(Vec, Vec)> { let bind_lookup: std::collections::HashMap = self.agg_binds.iter().map(|(v, e)| (*v, e)).collect(); @@ -1782,6 +1939,40 @@ impl FusedR2rmlAggregateOperator { }); } } + AggregateFn::Min(v) | AggregateFn::Max(v) => { + // MIN/MAX over a bare scalar column only. A desugared expression + // MIN/MAX (an agg_bind) is not folded — decline to the generic + // path (the exact term of an arithmetic result is out of scope). + if bind_lookup.contains_key(v) { + return None; + } + let is_max = matches!(func, AggregateFn::Max(_)); + let (col, datatype) = Self::scalar_column_for_var(pattern, tm, *v)?; + // Scope gate: numeric / date / timestamp only. String (collation), + // boolean, and un-annotated (→ xsd:string) columns decline. + if !minmax_admissible_datatype(datatype.as_deref()) { + return None; + } + // Plain-literal term only: a language-tagged or IRI-/blank-typed + // object map materializes a term whose lang/term-type the fold's + // compare would order differently than the generic path intends — + // decline (decline-only, never wrong). + let object_map = Self::object_map_for_var(pattern, tm, *v)?; + if !matches!( + &object_map, + ObjectMap::Column { + language: None, + term_type: TermType::Literal, + .. + } + ) { + return None; + } + projection.push(col); + let index = minmax_folds.len(); + minmax_folds.push(MinMaxFold { object_map, is_max }); + folds.push(Fold::MinMax { index }); + } _ => return None, } } @@ -2095,10 +2286,16 @@ impl FusedR2rmlAggregateOperator { // Aggregates fold from the FACT scan. let mut projection: Vec = Vec::new(); - let Some((folds, expr_folds)) = self.resolve_agg_folds(fact_p, fact_tm, &mut projection) + let mut minmax_folds: Vec = Vec::new(); + let Some((folds, expr_folds)) = + self.resolve_agg_folds(fact_p, fact_tm, &mut projection, &mut minmax_folds) else { return Ok(None); }; + // MIN/MAX candidate terms materialize from the FACT scan, so the encoder is + // built from the fact TriplesMap (same datatype Sids the generic path uses). + let minmax_encoder = + (!minmax_folds.is_empty()).then(|| LiteralEncoder::build(fact_tm, ctx.active_snapshot)); // Fact-side row-validity: the subject template columns, the first hop's FK // child columns (a null FK ⇒ no ref triple ⇒ row drops), and every scalar @@ -2319,6 +2516,8 @@ impl FusedR2rmlAggregateOperator { group_resolver: Some(GroupKeyResolver { fact_fk_cols, map }), group_key_plan, fact_constraints: fact_checks, + minmax_folds, + minmax_encoder, })) } } @@ -2328,35 +2527,51 @@ mod tests { use super::*; use crate::ir::grouping::AggregateSpec; - /// C5 slice-1 over-count TRIPWIRE: a participating R2RML pattern carrying a + /// C5 slice-1.5 (item 9b, the q038 class): a single-table COUNT carrying a /// folded constant-object constraint (`star_constraints`, e.g. - /// `?c ex:isCurrent true`) must DECLINE the fuse — the fused fold has no - /// star_constraints handling and would over-count. Red under a hypothetical - /// unguarded fuse, green with `fold_over_star_constraints`; also the guard - /// against slice 1.5 silently lifting it. + /// `?s edw:isCurrent true`) is NO LONGER a blanket decline — the fold resolves + /// the constraint to a scalar-column check and APPLIES it per row (the same + /// `resolve_star_constraint_checks` / `row_satisfies_constraints` machinery the + /// join path uses), so the constrained count excludes the non-matching rows + /// exactly as the materialized answer does. The over-count hazard the slice-1 + /// decline guarded is now closed by APPLICATION (this test) + the fused-vs-native + /// corpus parity (q022/q038/q061 + a multi-constraint member) rather than by + /// declining. A constraint the fold cannot enforce (a RefObjectMap object) still + /// declines — see `e2_resolve_star_constraint_checks_scalar_vs_ref`. #[test] - fn fold_over_star_constraints_declines_a_constrained_pattern() { + fn slice_1_5_admits_and_applies_a_single_table_flag_constraint() { use crate::r2rml::{ObjectConstant, ScanValue}; - let plain = R2rmlPattern::new("gs:main", VarId(1), Some(VarId(2))) - .with_predicate("http://ex/gender"); - assert!( - !fold_over_star_constraints(&[&plain]), - "no constraint → foldable" - ); - - let mut constrained = plain.clone(); + use fluree_db_r2rml::mapping::{ObjectMap, PredicateMap, PredicateObjectMap}; + let flag = "http://ex/isCurrent"; + let tm = + TriplesMap::new("#Cust", "DIM_CUSTOMER").with_predicate_object(PredicateObjectMap { + predicate_map: PredicateMap::constant(flag), + object_map: ObjectMap::column("IS_CURRENT"), + }); + // The q038 shape: a scalar-column `isCurrent true` flag folded onto the star. + let mut constrained = R2rmlPattern::new("gs:main", VarId(1), None); constrained.star_constraints = vec![( - "IS_CURRENT".to_string(), + flag.to_string(), ObjectConstant::Scalar(ScanValue::Bool(true)), )]; - assert!( - fold_over_star_constraints(&[&constrained]), - "a folded `isCurrent true` constraint must decline the fuse (else over-count)" + // Slice-1.5: it now RESOLVES (admits) with exactly one per-row check applied + // in the fold — no longer a decline, no longer a silent over-count. + let checks = FusedR2rmlAggregateOperator::resolve_star_constraint_checks(&constrained, &tm); + assert_eq!( + checks.map(|v| v.len()), + Some(1), + "the q038 `isCurrent true` flag resolves to one applied scalar check" + ); + // A non-empty `fact_constraints` set keeps the COUNT(*) manifest shortcut + // declined in `next_batch` (it requires `fact_constraints.is_empty()`), so a + // constrained count is never answered from the delete-blind `record_count`. + let plain = R2rmlPattern::new("gs:main", VarId(1), None); + assert_eq!( + FusedR2rmlAggregateOperator::resolve_star_constraint_checks(&plain, &tm) + .map(|v| v.len()), + Some(0), + "an unconstrained star resolves to zero checks (shortcut stays eligible)" ); - // Fires if ANY participating map is constrained (the aggregated fact OR a - // joined dim) — the broadened guard. - assert!(fold_over_star_constraints(&[&plain, &constrained])); - assert!(!fold_over_star_constraints(&[&plain, &plain])); } /// C5 O2 mixed-dataset guard (the `dataset_is_single_source` core): the fused @@ -2538,6 +2753,232 @@ mod tests { assert!(FusedR2rmlAggregateOperator::row_satisfies_constraints(&[], &batch, 1).unwrap()); } + /// Item 9b MULTI-constraint boundary (the D-c5 lesson): with TWO folded + /// constant-object constraints a row is kept iff it satisfies BOTH (AND + /// semantics), so a constrained COUNT excludes any row failing EITHER — no + /// over-count. This is the shape of corpus member q077 (isCurrent true AND + /// segment=Enterprise); the fused single-table path routes through this same + /// `row_satisfies_constraints`, so the member's native oracle and this test pin + /// the same guarantee at two levels. + #[test] + fn multi_constraint_requires_all_to_match() { + use crate::r2rml::ScanValue; + use fluree_db_r2rml::mapping::{ObjectMap, PredicateMap, PredicateObjectMap}; + use fluree_db_tabular::{BatchSchema, FieldInfo, FieldType}; + let flag = ObjectConstant::Scalar(ScanValue::Bool(true)); + let seg = ObjectConstant::Scalar(ScanValue::Str("Enterprise".to_string())); + let checks = [ + ResolvedConstraint { + canon: decimal_canonical_of(&flag), + pom: PredicateObjectMap { + predicate_map: PredicateMap::constant("http://ex/isCurrent"), + object_map: ObjectMap::column("IS_CURRENT"), + }, + constant: flag, + }, + ResolvedConstraint { + canon: decimal_canonical_of(&seg), + pom: PredicateObjectMap { + predicate_map: PredicateMap::constant("http://ex/segment"), + object_map: ObjectMap::column("SEGMENT"), + }, + constant: seg, + }, + ]; + let schema = Arc::new(BatchSchema::new(vec![ + FieldInfo { + name: "IS_CURRENT".to_string(), + field_type: FieldType::Boolean, + nullable: true, + field_id: 1, + }, + FieldInfo { + name: "SEGMENT".to_string(), + field_type: FieldType::String, + nullable: true, + field_id: 2, + }, + ])); + // rows: (both match), (flag matches, segment wrong), (flag wrong, segment + // matches), (segment null). + let batch = ColumnBatch::new( + schema, + vec![ + Column::Boolean(vec![Some(true), Some(true), Some(false), Some(true)]), + Column::String(vec![ + Some("Enterprise".to_string()), + Some("Consumer".to_string()), + Some("Enterprise".to_string()), + None, + ]), + ], + ) + .unwrap(); + let ok = |row| { + FusedR2rmlAggregateOperator::row_satisfies_constraints(&checks, &batch, row).unwrap() + }; + assert!(ok(0), "both constraints satisfied → kept"); + assert!(!ok(1), "segment mismatch → dropped (no over-count)"); + assert!(!ok(2), "flag mismatch → dropped (no over-count)"); + assert!(!ok(3), "null segment → dropped (existence filter)"); + } + + /// Item 9b decline invariant (D-c5): the bare-COUNT manifest shortcut fires ONLY + /// for a plain COUNT — it DECLINES for a filtered, grouped, constant-object- + /// constrained, or non-`CountRows` plan, because the manifest `record_count` sum + /// cannot see per-row matches or per-group partitions. A false positive here is + /// exactly the over-count R-1522 named as the unacceptable outcome; this pins the + /// predicate directly (it was previously only inspection-verified). + #[test] + fn count_shortcut_declines_constraints_filter_group_and_non_count() { + use crate::r2rml::ScanValue; + use fluree_db_r2rml::mapping::{ObjectMap, PredicateMap, PredicateObjectMap}; + // Eligible: a single CountRows fold, no filter / group / constraints. + assert!(count_shortcut_eligible(false, &[], &[], &[Fold::CountRows])); + // Declines: a FILTER is present. + assert!(!count_shortcut_eligible(true, &[], &[], &[Fold::CountRows])); + // Declines: a GROUP BY key is present. + let gcol = GroupCol { + column: "SEG".into(), + kind: GKind::String, + dt_sid: Sid::new(2, "string"), + }; + assert!(!count_shortcut_eligible( + false, + &[gcol], + &[], + &[Fold::CountRows] + )); + // Declines: a folded constant-object constraint is present (the q038/9b line). + let constant = ObjectConstant::Scalar(ScanValue::Bool(true)); + let constraint = ResolvedConstraint { + canon: decimal_canonical_of(&constant), + pom: PredicateObjectMap { + predicate_map: PredicateMap::constant("http://ex/isCurrent"), + object_map: ObjectMap::column("IS_CURRENT"), + }, + constant, + }; + assert!( + !count_shortcut_eligible(false, &[], &[constraint], &[Fold::CountRows]), + "a constraint-bearing COUNT must decline the delete-blind shortcut" + ); + // Declines: not a bare CountRows fold (COUNT(col) / SUM / MIN / two folds / none). + assert!(!count_shortcut_eligible( + false, + &[], + &[], + &[Fold::CountColumn("C".into())] + )); + assert!(!count_shortcut_eligible( + false, + &[], + &[], + &[Fold::CountRows, Fold::CountRows] + )); + assert!(!count_shortcut_eligible(false, &[], &[], &[])); + } + + /// Item 9 MIN/MAX admission scope: only numeric and date/timestamp datatypes + /// fold; string (collation), boolean, custom, and un-annotated (→ xsd:string) + /// columns decline to the generic pipeline. Parity of the fold itself comes from + /// materializing the same term + `compare_bindings`; this gate only scopes the + /// mechanism to the audit item's covered types (F-AUD-8). + #[test] + fn minmax_admissible_datatype_scope() { + use fluree_vocab::xsd; + for dt in [ + xsd::INTEGER, + xsd::LONG, + xsd::INT, + xsd::DECIMAL, + xsd::DOUBLE, + xsd::FLOAT, + xsd::DATE, + xsd::DATE_TIME, + ] { + assert!(minmax_admissible_datatype(Some(dt)), "{dt} should fold"); + } + for dt in [xsd::STRING, xsd::BOOLEAN, "http://ex/custom"] { + assert!( + !minmax_admissible_datatype(Some(dt)), + "{dt} should decline (collation / non-orderable / out of scope)" + ); + } + assert!( + !minmax_admissible_datatype(None), + "an un-annotated column (→ xsd:string) declines" + ); + } + + /// Item 9 MIN/MAX tie-break parity (R-1522): the fold must resolve an + /// equal-BUT-differently-rendered extreme the SAME way the generic + /// `agg_min`/`agg_max` do — MIN keeps the FIRST of the ties, MAX keeps the LAST + /// (`min_by`/`max_by` semantics). The reachable concrete case is a double column + /// carrying both `+0.0` and `-0.0`: they compare Equal but render "0" vs "-0", + /// so picking the wrong one on a tie breaks byte-parity. (A single decimal column + /// has a fixed scale, so it can't hold two renderings of one value — but the same + /// `Equal`-by-value rule covers `1.50` vs `1.5` if such a pair ever reached the + /// fold; the pure-logic assertions below pin that rule directly.) + #[test] + fn minmax_tie_break_matches_generic_agg() { + use crate::sort::compare_bindings; + use std::cmp::Ordering; + // The replace predicate == min_by (first) / max_by (last) tie-breaking. + assert!( + !minmax_should_replace(false, Ordering::Equal), + "MIN keeps the FIRST on a tie" + ); + assert!( + minmax_should_replace(true, Ordering::Equal), + "MAX keeps the LAST on a tie" + ); + assert!(minmax_should_replace(false, Ordering::Less)); + assert!(!minmax_should_replace(false, Ordering::Greater)); + assert!(minmax_should_replace(true, Ordering::Greater)); + assert!(!minmax_should_replace(true, Ordering::Less)); + + // Concrete reachable case: ±0.0 compare Equal but render differently. + let pos = Binding::lit(FlakeValue::Double(0.0), Sid::xsd_double()); + let neg = Binding::lit(FlakeValue::Double(-0.0), Sid::xsd_double()); + assert_eq!( + compare_bindings(&pos, &neg), + Ordering::Equal, + "+0.0 and -0.0 compare Equal" + ); + let bits = |b: &Binding| match b { + Binding::Lit { + val: FlakeValue::Double(d), + .. + } => d.to_bits(), + _ => panic!("expected a double lit"), + }; + // Fold over [+0.0, -0.0] in scan order: MAX must end on -0.0 (the LAST, + // matching agg_max); MIN must end on +0.0 (the FIRST, matching agg_min). + // Under the old first-on-ties MAX this would keep +0.0 and diverge. + for (is_max, generic) in [ + ( + true, + AggregateFn::Max(VarId(0)).apply(&Binding::Grouped(vec![pos.clone(), neg.clone()])), + ), + ( + false, + AggregateFn::Min(VarId(0)).apply(&Binding::Grouped(vec![pos.clone(), neg.clone()])), + ), + ] { + let mut best = pos.clone(); + if minmax_should_replace(is_max, compare_bindings(&neg, &best)) { + best = neg.clone(); + } + assert_eq!( + bits(&best), + bits(&generic), + "fused {} tie-break must match the generic aggregate", + if is_max { "MAX" } else { "MIN" } + ); + } + } + /// Q2 lang/IRI admission gate: a fused group key — the single-table key OR the /// join path's terminal-dim key, both of which call this SAME shared predicate /// before applying the `xsd:string` default — must be a PLAIN LITERAL. A diff --git a/fluree-db-query/src/r2rml/mod.rs b/fluree-db-query/src/r2rml/mod.rs index 336c87a59..746624a0c 100644 --- a/fluree-db-query/src/r2rml/mod.rs +++ b/fluree-db-query/src/r2rml/mod.rs @@ -105,3 +105,69 @@ pub(crate) fn iceberg_numeric_stats_enabled() -> bool { static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); *ENABLED.get_or_init(|| env_switch_enabled("FLUREE_ICEBERG_NUMERIC_STATS")) } + +/// Item 10 (F-AUD-11): whether `xsd:dateTime` FILTER predicates may be pushed to +/// the Iceberg scan for MANIFEST-level file pruning (mirrors +/// [`iceberg_numeric_stats_enabled`]). Default on; +/// `FLUREE_ICEBERG_TIMESTAMP_STATS=0|false|off|no` reverts to leaving them with the +/// in-engine FILTER only (no timestamp `ScanValue` is produced, so the iceberg-side +/// timestamp arms stay inert). Gating at the single push site (`to_scan_value`) +/// keeps the widening inert when off. Cached in a `OnceLock` — set at process +/// startup, not per query. +pub(crate) fn iceberg_timestamp_stats_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| env_switch_enabled("FLUREE_ICEBERG_TIMESTAMP_STATS")) +} + +/// Item 7 (F-AUD-5): whether a bounded `FILTER … IN (…)` or single-var `VALUES` +/// set over a scalar-column object var is lowered to an Iceberg `Expression::In` +/// for file / row-group pruning (the backend already evaluates `In`; nothing +/// emitted it). Default on; `FLUREE_R2RML_IN_PUSHDOWN=0|false|off|no` reverts to +/// leaving the set to the in-engine FILTER / VALUES join (a full FACT scan). The +/// push is a strict superset (keeps a file iff any member could be in range), so +/// OFF is byte-identical in results, only slower. Read once (process-wide). +pub(crate) fn in_pushdown_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| env_switch_enabled("FLUREE_R2RML_IN_PUSHDOWN")) +} + +/// Item 11 (F-AUD-7): whether `OPTIONAL` (left-outer-join) forwards a top-of-tree +/// `LIMIT` row budget to its REQUIRED (outer) side. Sound because each required +/// row yields ≥1 output row (matched or padded-with-null), so bounding the +/// required side to `k` still produces ≥`k` output for the `LIMIT` to truncate — +/// the inner/optional side is NOT budgeted (it must produce every match for a +/// given required row). This closes probe-04's 68,828× read amplification. Default +/// on; `FLUREE_R2RML_BUDGET_OPTIONAL=0|false|off|no` reverts to swallowing the +/// budget (byte-identical results, just the full outer scan). Read once. +pub(crate) fn optional_budget_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| env_switch_enabled("FLUREE_R2RML_BUDGET_OPTIONAL")) +} + +/// Item 8 (F-AUD-6): whether an ASCENDING single-column `ORDER BY … LIMIT k` over +/// an R2RML scan may offer a scan-side top-k directive. DESC top-k (PR-5) is +/// always offered; ASC is admitted only for a REQUIRED (non-nullable) column (the +/// provider re-checks — SPARQL orders unbound values first under ASC). Default on; +/// `FLUREE_R2RML_TOPK_ASC=0|false|off|no` reverts to the pre-item-8 DESC-only +/// behavior (byte-identical: an ASC sort then full-scans + sorts). Read once. +pub(crate) fn topk_asc_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| env_switch_enabled("FLUREE_R2RML_TOPK_ASC")) +} + +/// Item 7 (F-AUD-5): the maximum set size lowered to `Expression::In`. A larger +/// set declines the pushdown (stays with the in-engine FILTER / VALUES join) so +/// manifest evaluation stays cheap — every member is bound-checked against every +/// candidate file, i.e. O(files × members). Default 64; override with +/// `FLUREE_R2RML_IN_PUSHDOWN_MAX=` (a non-numeric or zero value falls back to +/// the default). Read once (process-wide). +pub(crate) fn in_pushdown_max() -> usize { + static MAX: std::sync::OnceLock = std::sync::OnceLock::new(); + *MAX.get_or_init(|| { + std::env::var("FLUREE_R2RML_IN_PUSHDOWN_MAX") + .ok() + .and_then(|v| v.trim().parse::().ok()) + .filter(|n| *n > 0) + .unwrap_or(64) + }) +} diff --git a/fluree-db-query/src/r2rml/operator.rs b/fluree-db-query/src/r2rml/operator.rs index 377298b6e..3d389743d 100644 --- a/fluree-db-query/src/r2rml/operator.rs +++ b/fluree-db-query/src/r2rml/operator.rs @@ -424,13 +424,13 @@ pub struct R2rmlScanOperator { row_budget: Option, /// Output rows emitted so far, counted against `row_budget`. emitted: usize, - /// Scan-side top-k directive (PR-5): `(primary DESC sort var, LIMIT+OFFSET)`, - /// set by a `SortOperator` for a `ORDER BY DESC() LIMIT k` directly - /// above this scan. Resolved to the sort column against the mapping at scan - /// time ([`Self::resolve_topk_directive`]); `None` = no pushdown (full scan + - /// the authoritative sort above). Only ever consulted for the main table scan, - /// never a parent/dimension lookup. - topk: Option<(VarId, usize)>, + /// Scan-side top-k directive (PR-5; ASC added in item 8): `(primary sort var, + /// LIMIT+OFFSET, ascending)`, set by a `SortOperator` for an `ORDER BY + /// LIMIT k` directly above this scan. Resolved to the sort column + /// against the mapping at scan time ([`Self::resolve_topk_directive`]); `None` + /// = no pushdown (full scan + the authoritative sort above). Only ever + /// consulted for the main table scan, never a parent/dimension lookup. + topk: Option<(VarId, usize, bool)>, /// A scan-local FILTER the planner folded into this scan (see /// [`R2rmlPattern::consumed_filter`]). Applied to each output batch with the /// same evaluator the dropped `FilterOperator` would use, so results are @@ -622,7 +622,7 @@ impl R2rmlScanOperator { /// column; the `SortOperator` above still applies the exact compound order + /// LIMIT, so a `None` here is only a missed optimization, never wrong. fn resolve_topk_directive(&self, triples_map: &TriplesMap) -> Option { - let (sort_var, k) = self.topk?; + let (sort_var, k, ascending) = self.topk?; // SOUNDNESS (heap feed): decline when a residual filter the operator // enforces after the scan is present — the heap would see pre-filter rows. if topk_residual_filter_present(&self.pattern) { @@ -655,6 +655,7 @@ impl R2rmlScanOperator { Some(crate::r2rml::ScanTopK { sort_column: col.to_string(), k, + ascending, }) } @@ -1921,12 +1922,16 @@ fn coerce_scalar_for_pushdown( // Already a pushable, type-matched value; the residual uses the same // semantics for these variants. ScanValue::Int(_) | ScanValue::Bool(_) | ScanValue::Date(_) => Some(value.clone()), - // Double/Decimal/TemplateKey never wrap a Scalar object constant (a numeric - // object routes to ObjectConstant::Double/Decimal, operator-enforced only), - // so these are unreachable here; push as-is defensively — never wrong. - ScanValue::Double(_) | ScanValue::Decimal { .. } | ScanValue::TemplateKey(_) => { - Some(value.clone()) - } + // Double/Decimal/TemplateKey/Timestamp never wrap a Scalar object constant + // (a numeric/temporal object routes elsewhere, operator-enforced only), so + // these are unreachable here; push as-is defensively — never wrong. + ScanValue::Double(_) + | ScanValue::Decimal { .. } + | ScanValue::TemplateKey(_) + | ScanValue::Timestamp { .. } => Some(value.clone()), + // A `Set` only ever arrives via the FILTER-IN / VALUES set-pushdown path, + // never wrapped in a `Scalar` object constant — decline to coerce it. + ScanValue::Set(_) => None, } } @@ -2111,6 +2116,12 @@ pub(crate) fn rdf_term_eq_object_constant_cached( // A TemplateKey is only ever a reversed subject-key filter, never // an object constant, so it never matches an object term. ScanValue::TemplateKey(_) => false, + // A Set is only ever a FILTER-IN / VALUES scan filter, never an + // object constant, so it never matches an object term. + ScanValue::Set(_) => false, + // A Timestamp is only ever a FILTER pushdown value (dateTime object + // constants are not lowered), never an object constant here. + ScanValue::Timestamp { .. } => false, } } } @@ -2634,13 +2645,15 @@ impl Operator for R2rmlScanOperator { } } - fn set_topk(&mut self, sort_var: VarId, k: usize) { - // Record the DESC top-k directive; it is resolved to a scan column against - // the mapping at scan time and honored only for the main table scan. Like + fn set_topk(&mut self, sort_var: VarId, k: usize, ascending: bool) { + // Record the top-k directive; it is resolved to a scan column against the + // mapping at scan time and honored only for the main table scan. Like // `row_budget`, do NOT forward to the child — an inner correlated scan must // still produce every row the join needs; only a topmost scan is eligible. + // An ASC directive is admitted only when the sort column is REQUIRED (the + // provider re-checks nullability at scan time). if topk_pushdown_enabled() { - self.topk = Some((sort_var, k)); + self.topk = Some((sort_var, k, ascending)); } } @@ -4434,7 +4447,7 @@ mod tests { let mut op = R2rmlScanOperator::new(Box::new(EmptyOperator::new()), pattern); op.mapping = Some(Arc::clone(mapping)); if topk { - op.topk = Some((VarId(1), 10)); + op.topk = Some((VarId(1), 10, false)); } for _ in 0..3 { futures::executor::block_on(op.build_progress(&ctx, Batch::single_empty())) diff --git a/fluree-db-query/src/r2rml/provider.rs b/fluree-db-query/src/r2rml/provider.rs index 92e53d053..31aff2bf4 100644 --- a/fluree-db-query/src/r2rml/provider.rs +++ b/fluree-db-query/src/r2rml/provider.rs @@ -31,6 +31,13 @@ pub enum ScanCmpOp { LtEq, Gt, GtEq, + /// Set membership (`?x IN (c1..cN)`), lowered from a `FILTER … IN` or a + /// single-var `VALUES` block. The filter's `value` is a [`ScanValue::Set`]; + /// the provider builds an Iceberg `Expression::In`, which keeps a file iff + /// ANY member could lie within the file's column bounds (a superset — the + /// in-engine FILTER / VALUES join still enforces exact membership). Never + /// paired with a scalar `value`; never mapped to a scalar `ComparisonOp`. + In, } /// A literal value for a pushed-down scan filter. @@ -67,6 +74,26 @@ pub enum ScanValue { /// skipped for field types not yet supported. The R2RML operator still enforces /// the subject equality, so a skipped or imperfect push is never wrong. TemplateKey(String), + /// A bounded set of scalar values for a [`ScanCmpOp::In`] membership filter + /// (from a `FILTER … IN` or single-var `VALUES`). Every member is one of the + /// scalar variants above — never a nested `Set`, never a `TemplateKey`. Only + /// ever produced by the set-lowering emit path; it is never wrapped in an + /// [`ObjectConstant`] (a constant object is a single scalar term), so the + /// scalar-only match sites treat it defensively as "not a scalar constant". + Set(Vec), + /// An `xsd:dateTime` value as microseconds since the Unix epoch, carrying + /// whether the source literal was timezone-AWARE (item 10). `tz = true` (an + /// explicit offset, e.g. `…Z`/`+05:00`) means the micros are in the UTC frame + /// and only push against a physically-`timestamptz` column; `tz = false` (a + /// naive literal) means wall-clock micros and only push against a physically + /// `timestamp` column. The frame is matched at pushdown-build time so the + /// micros are directly comparable to the Iceberg manifest bounds; a mismatch + /// declines the push (the in-engine FILTER stays the authority). Pruning is + /// MANIFEST-level only — see the row-group note in `fluree-db-iceberg`. + Timestamp { + micros: i64, + tz: bool, + }, } /// A constant object in a triple pattern (`?s `), enforced by the @@ -103,22 +130,28 @@ pub struct ScanFilter { pub value: ScanValue, } -/// A scan-side top-k directive for a single-column **DESCENDING** `ORDER BY … -/// LIMIT k` directly above a single-table R2RML scan (PR-5). The scan reads files -/// in `upper_bound(sort_column)`-DESC order, keeps a running k-th bound, and stops +/// A scan-side top-k directive for a single-column `ORDER BY … LIMIT k` directly +/// above a single-table R2RML scan (PR-5; ASC added in item 8, F-AUD-6). The scan +/// reads files best-first (DESC: `upper_bound(sort_column)` descending; ASC: +/// `lower_bound(sort_column)` ascending), keeps a running k-th bound, and stops /// once no unread file can beat it — reading far fewer than the whole table. /// /// A pure perf optimization: the scan still streams a strict SUPERSET of the true /// top-k (it only skips files that provably cannot contribute), and the /// authoritative `SortOperator` above applies the exact (compound) order + LIMIT. /// Ignored by the provider unless `sort_column` resolves to a pushable scalar -/// column of the scanned table. +/// column of the scanned table — and, for `ascending`, unless that column is +/// REQUIRED (non-nullable) in the Iceberg schema, since SPARQL orders unbound +/// values FIRST under ASC and a nullable column could hide an unread top-k row. #[derive(Debug, Clone, PartialEq)] pub struct ScanTopK { - /// The primary DESC sort column (an R2RML-mapped table column name). + /// The primary sort column (an R2RML-mapped table column name). pub sort_column: String, /// How many top rows the bound must retain — the query's `LIMIT + OFFSET`. pub k: usize, + /// `true` for an `ASC` sort (admitted only for a required column; the provider + /// re-checks nullability), `false` for `DESC`. + pub ascending: bool, } /// Provider for compiled R2RML mappings. diff --git a/fluree-db-query/src/r2rml/rewrite.rs b/fluree-db-query/src/r2rml/rewrite.rs index 47246a9d6..8a34d62b8 100644 --- a/fluree-db-query/src/r2rml/rewrite.rs +++ b/fluree-db-query/src/r2rml/rewrite.rs @@ -24,6 +24,7 @@ //! - Subject-bound patterns (`ex:subject ex:name ?o`) are not optimized //! - Filter patterns are preserved and applied post-R2RML scan +use crate::binding::Binding; use crate::ir::adapters::ScanPushdown; use crate::ir::triple::{Ref, Term, TriplePattern}; use crate::ir::{Expression, Function, Pattern, R2rmlPattern}; @@ -431,13 +432,16 @@ pub fn rewrite_patterns_for_r2rml( } } - // Attach pushable FILTER comparisons to the R2RML pattern that produces - // each compared variable, for Iceberg file pruning. The FILTER pattern is - // left in place (residual), so this only ever skips data files. + // Attach pushable FILTER comparisons — and bounded FILTER-IN / single-var + // VALUES sets (item 7, F-AUD-5) — to the R2RML pattern that produces each + // compared variable, for Iceberg file pruning. The FILTER / VALUES pattern is + // left in place (residual / join), so this only ever skips data files. let mut pushdowns: Vec<(VarId, ScanCmpOp, ScanValue)> = Vec::new(); for p in &result_patterns { - if let Pattern::Filter(expr) = p { - collect_pushdowns(expr, &mut pushdowns); + match p { + Pattern::Filter(expr) => collect_pushdowns(expr, &mut pushdowns), + Pattern::Values { vars, rows } => collect_values_pushdown(vars, rows, &mut pushdowns), + _ => {} } } if !pushdowns.is_empty() { @@ -585,6 +589,13 @@ fn collect_pushdowns(expr: &Expression, out: &mut Vec<(VarId, ScanCmpOp, ScanVal } return; } + // Item 7 (F-AUD-5): a bounded `FILTER ?var IN (c1..cN)` lowers to a set + // membership pushdown. (`NOT IN` lowers to `Function::NotIn`, which is not + // collected — `Expression::NotIn` cannot prune via min/max bounds.) + if matches!(func, Function::In) { + collect_in_pushdown(args, out); + return; + } if args.len() != 2 { return; } @@ -603,6 +614,76 @@ fn collect_pushdowns(expr: &Expression, out: &mut Vec<(VarId, ScanCmpOp, ScanVal } } +/// Collect a bounded `?var IN (c1..cN)` (the [`Function::In`] shape from +/// [`eval_in`](crate::eval): `args[0]` is the tested var, `args[1..]` are the +/// list members) as a [`ScanCmpOp::In`] set pushdown, when In-pushdown is enabled, +/// the tested position is a bare variable, and EVERY member is a constant that +/// converts to a prunable [`ScanValue`]. A single non-constant or non-convertible +/// member declines the WHOLE set: a partial `IN` would be unsound — the scan could +/// prune a file that a dropped member's rows live in, and those rows never reach +/// the residual FILTER. Sets larger than the cap decline (kept cheap). The value +/// types are the same `to_scan_value` types the scalar pushdown emits, so the +/// per-member soundness (loose value match, numeric gating) is identical. +fn collect_in_pushdown(args: &[Expression], out: &mut Vec<(VarId, ScanCmpOp, ScanValue)>) { + if !super::in_pushdown_enabled() { + return; + } + let Some(Expression::Var(var)) = args.first() else { + return; + }; + let members = &args[1..]; + if members.is_empty() || members.len() > super::in_pushdown_max() { + return; + } + let mut values = Vec::with_capacity(members.len()); + for m in members { + let Expression::Const(c) = m else { + return; + }; + let Some(sv) = to_scan_value(c) else { + return; + }; + values.push(sv); + } + out.push((*var, ScanCmpOp::In, ScanValue::Set(values))); +} + +/// Item 7 (F-AUD-5): collect a single-var `VALUES ?v { c1 c2 … }` as a +/// [`ScanCmpOp::In`] set pushdown. Only single-var VALUES lower here — a +/// multi-column VALUES binds correlated tuples, and independent per-column INs +/// would lose the correlation (still sound for pruning, but out of scope). Every +/// row must bind exactly ONE scalar literal that converts to a prunable +/// [`ScanValue`]; an `UNDEF` (`Binding::Unbound`), an IRI/ref binding, or a +/// non-convertible literal declines the WHOLE set — an `UNDEF` means "any value", +/// so an `IN` prune would wrongly drop rows. The VALUES pattern stays in the plan +/// (the join enforces exact membership), so this only skips data files. +fn collect_values_pushdown( + vars: &[VarId], + rows: &[Vec], + out: &mut Vec<(VarId, ScanCmpOp, ScanValue)>, +) { + if !super::in_pushdown_enabled() { + return; + } + let [var] = vars else { + return; + }; + if rows.is_empty() || rows.len() > super::in_pushdown_max() { + return; + } + let mut values = Vec::with_capacity(rows.len()); + for row in rows { + let [Binding::Lit { val, .. }] = row.as_slice() else { + return; + }; + let Some(sv) = to_scan_value(val) else { + return; + }; + values.push(sv); + } + out.push((*var, ScanCmpOp::In, ScanValue::Set(values))); +} + /// Map a comparison `Function` to a pushable `ScanCmpOp`, reversing operand /// order when the constant was on the left. Returns None for non-prunable ops. fn cmp_op(func: &Function, reversed: bool) -> Option { @@ -690,6 +771,16 @@ fn to_scan_value(value: &FlakeValue) -> Option { FlakeValue::Decimal(d) if crate::r2rml::iceberg_numeric_stats_enabled() => { scan_value_from_bigdecimal(d) } + // Item 10 (F-AUD-11): an xsd:dateTime pushes as micros-since-epoch, carrying + // whether the source was tz-AWARE (an explicit offset ⇒ UTC frame) so the + // provider can frame-match it to a `timestamp` vs `timestamptz` column. + // Gated by FLUREE_ICEBERG_TIMESTAMP_STATS. + FlakeValue::DateTime(dt) if crate::r2rml::iceberg_timestamp_stats_enabled() => { + Some(ScanValue::Timestamp { + micros: dt.epoch_micros(), + tz: dt.tz_offset().is_some(), + }) + } _ => None, } } @@ -2564,4 +2655,133 @@ mod tests { }) ); } + + // ---- Item 7 (F-AUD-5): FILTER-IN / VALUES set lowering ---- + + fn str_const(s: &str) -> Expression { + Expression::Const(FlakeValue::String(s.to_string())) + } + + fn set_len(out: &[(VarId, ScanCmpOp, ScanValue)]) -> usize { + assert_eq!(out.len(), 1, "expected exactly one set pushdown"); + let (_, op, val) = &out[0]; + assert_eq!(*op, ScanCmpOp::In); + match val { + ScanValue::Set(vs) => vs.len(), + other => panic!("expected ScanValue::Set, got {other:?}"), + } + } + + #[test] + fn filter_in_collects_bounded_scalar_set() { + let args = vec![Expression::Var(VarId(0)), str_const("a"), str_const("b")]; + let mut out = Vec::new(); + collect_in_pushdown(&args, &mut out); + assert_eq!(out[0].0, VarId(0)); + assert_eq!(set_len(&out), 2); + } + + #[test] + fn filter_in_declines_when_a_member_is_not_a_pushable_const() { + // A non-constant member (a bound var) declines the WHOLE set — a partial + // IN could prune a file the dropped member's rows live in. + let args = vec![ + Expression::Var(VarId(0)), + str_const("a"), + Expression::Var(VarId(1)), + ]; + let mut out = Vec::new(); + collect_in_pushdown(&args, &mut out); + assert!(out.is_empty()); + } + + #[test] + fn filter_in_declines_over_cap() { + // Default cap is 64; 65 members decline (kept cheap — O(files × members)). + let mut args = vec![Expression::Var(VarId(0))]; + for i in 0..65 { + args.push(Expression::Const(FlakeValue::Long(i))); + } + let mut out = Vec::new(); + collect_in_pushdown(&args, &mut out); + assert!(out.is_empty()); + } + + #[test] + fn filter_in_declines_empty_set() { + let args = vec![Expression::Var(VarId(0))]; + let mut out = Vec::new(); + collect_in_pushdown(&args, &mut out); + assert!(out.is_empty()); + } + + fn lit(s: &str) -> Binding { + Binding::lit(FlakeValue::String(s.to_string()), Sid::new(XSD, "string")) + } + + #[test] + fn values_single_var_collects_scalar_set() { + let rows = vec![vec![lit("x")], vec![lit("y")], vec![lit("z")]]; + let mut out = Vec::new(); + collect_values_pushdown(&[VarId(0)], &rows, &mut out); + assert_eq!(out[0].0, VarId(0)); + assert_eq!(set_len(&out), 3); + } + + #[test] + fn values_declines_on_undef_row() { + // UNDEF means "any value"; an IN prune would wrongly drop rows → decline. + let rows = vec![vec![lit("x")], vec![Binding::Unbound]]; + let mut out = Vec::new(); + collect_values_pushdown(&[VarId(0)], &rows, &mut out); + assert!(out.is_empty()); + } + + #[test] + fn values_declines_multi_var() { + // A multi-column VALUES binds correlated tuples; out of scope (single-var + // only). An independent per-column IN would lose the correlation. + let rows = vec![vec![lit("x"), lit("y")]]; + let mut out = Vec::new(); + collect_values_pushdown(&[VarId(0), VarId(1)], &rows, &mut out); + assert!(out.is_empty()); + } + + #[test] + fn values_declines_on_iri_binding() { + // An IRI/ref member is not a scalar-column value (the FK-IRI reversal is a + // separate, deeper mechanism) → decline. + let rows = vec![vec![Binding::Iri("http://x/1".into())]]; + let mut out = Vec::new(); + collect_values_pushdown(&[VarId(0)], &rows, &mut out); + assert!(out.is_empty()); + } + + // ---- Item 10 (F-AUD-11): xsd:dateTime → frame-aware Timestamp ---- + + #[test] + fn datetime_emits_frame_aware_timestamp() { + use fluree_db_core::DateTime; + let mk = + |s: &str| to_scan_value(&FlakeValue::DateTime(Box::new(DateTime::parse(s).unwrap()))); + match (mk("2024-06-01T00:00:00Z"), mk("2024-06-01T00:00:00")) { + ( + Some(ScanValue::Timestamp { + tz: tz_a, + micros: m_a, + }), + Some(ScanValue::Timestamp { + tz: tz_n, + micros: m_n, + }), + ) => { + assert!(tz_a, "explicit Z ⇒ tz-aware (UTC frame)"); + assert!(!tz_n, "no offset ⇒ naive (wall-clock frame)"); + // A naive dateTime is treated as UTC, so the same wall-clock yields + // the same micros — only the frame flag differs. + assert_eq!(m_a, m_n); + } + other => panic!("expected two Timestamps, got {other:?}"), + } + } } diff --git a/fluree-db-query/src/sort.rs b/fluree-db-query/src/sort.rs index 67412ce29..402759be2 100644 --- a/fluree-db-query/src/sort.rs +++ b/fluree-db-query/src/sort.rs @@ -574,6 +574,17 @@ impl SortOperator { #[async_trait] impl Operator for SortOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — a full sort must consume ALL input + /// to order it; the scan-side top-k for `ORDER BY … LIMIT` travels a separate + /// channel (`set_topk`, PR-5/item 8), not the row budget. Explicit (was a + /// silent trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "SORT row-budget swallowed (a full sort needs all input; top-k uses set_topk)" + ); + } + fn plan_children(&self) -> Vec> { vec![crate::plan_node::PlanChild::child(self.child.as_ref())] } diff --git a/fluree-db-query/src/subquery.rs b/fluree-db-query/src/subquery.rs index 19a1ea69d..e7bcda96b 100644 --- a/fluree-db-query/src/subquery.rs +++ b/fluree-db-query/src/subquery.rs @@ -268,6 +268,17 @@ impl SubqueryOperator { #[async_trait] impl Operator for SubqueryOperator { + /// Item 11 (F-AUD-7): DECLINE forwarding — a subquery computes its own result + /// set independently of the outer `LIMIT` (its cardinality, grouping, and + /// ordering are self-contained), so the outer budget cannot bound its input. + /// Explicit (was a silent trait-default no-op) so the swallow is observable. + fn set_row_budget(&mut self, budget: usize) { + tracing::debug!( + budget, + "SUBQUERY row-budget swallowed (unsound to forward: independent inner cardinality)" + ); + } + fn plan_details(&self) -> serde_json::Map { let mut m = serde_json::Map::new(); m.insert("join-mode".into(), self.join_mode.into());