Skip to content

feat(sql): pushdown lane M3 — subject policies, expressions, sub-selects, unions, HAVING - #1788

Open
bplatz wants to merge 12 commits into
mainfrom
feature/sql-pushdown-lane-m3
Open

feat(sql): pushdown lane M3 — subject policies, expressions, sub-selects, unions, HAVING#1788
bplatz wants to merge 12 commits into
mainfrom
feature/sql-pushdown-lane-m3

Conversation

@bplatz

@bplatz bplatz commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Third chunk of the SQL pushdown lane. #1785 has merged and the branch carries main (d6a482d); the nine commits before the merge are self-contained and read in order.

The merge ports the subject-skeleton rule from #1785's review into the union-entity resolution (parts_of), extends collect_col_eqs into derived tables and UNION ALL branches so the column-class vet reaches every rendered join, and makes the fake's newer predicate arms three-valued.

After the merge: the block-size probe is bounded at the cache cap plus one (SELECT COUNT(*) over the fetch statement as a derived table with LIMIT), closing #1784, and the decline for subject templates of one prefix but different skeletons is pinned.

What the lane now pushes

  • Subject-targeted view policies as key predicates (policy_subjectsVerdict::BySubject), alongside the class-targeted ones from feat(sql): pushdown lane M1/M2 — MIN/MAX, BIND, widening filters, partitioning, caps, block cache, class policies #1785. The Iceberg policy parity suite is replayed through a SQL source, so both SQL lanes meet the same oracle Iceberg does.
  • BIND expressions: +, -, * over exact numeric columns, and CONCAT, STRLEN, SUBSTR (from a positive constant position) and STR over plain string columns, as Expr in the tabular plan. A FILTER comparing such an expression with a literal pushes as the expression (also when written out without a BIND), an ORDER BY … LIMIT over it as a top-k. The bound value is still built in the engine.
  • LCASE/UCASE equality widens rather than pushes exactly: every dialect's case mapping matches SPARQL's on printable ASCII and nowhere else for certain, so the statement keeps rows whose folded value matches or that hold any other character (a regex, NOT REGEXP or GLOB per dialect), and the engine decides the rest.
  • Sub-selects as derived tables joined on their projected keys; grouped ones carry their aggregates as outputs the engine decodes like the grouped lane's, DISTINCT and ORDER BY … LIMIT push inside. There is no other lane for a sub-select over a graph source (the engine's subquery operator has no native index), so one the lane does not take still refuses the query as before; the suite gained a native-ledger twin of the fixture as the oracle for these.
  • An entity several triples maps provide (a predicate two maps mint, on the same subject or on different ones) is one derived table: every resolution lowered on its own, UNION ALLed under shared columns with a branch tag, so each row's terms decode through its own branch's maps. RelNode::UnionAll in the IR. Filters, seeds and a top-k push on the union's columns where every branch agrees on the key shape.
  • A UNION's branches share one UNION ALL statement where the database can type it (NULL-padded slots; SQLite types a compound column from the first branch alone, so a new capability keeps it on one statement per branch when padding would be needed). A top-k pushes onto the union when every branch orders on the same required column.
  • HAVING over COUNT and exact SUM comparisons goes with the grouped statement (rendered by repeating the aggregate expression; Postgres and Trino do not resolve a select alias there), and a top-k may follow it. AVG, MIN/MAX and key comparisons stay in the engine.
  • A foreign key into a union entity joins the parent's columns, which every branch exposes.

Behavior changes outside the lane

Two, both toward the spec, found while pinning the union cases against the per-scan lane:

  • A triple two maps mint alike is held once. An RDF graph is a set, but the per-scan lane returned such a triple once per map on an unfused star and once per covering map on a fused one. Maps minting a predicate from the same table, subject template and object map now count as one provider in both lanes (TriplesMap::mints_alike). Maps deriving the same value differently still come back once each.
  • Star fusion no longer drops rows. The per-scan lane fused a star as soon as one map covered every member and ignored a map providing only some of them on the same subject. It fuses only when every partial provider's subject is provably disjoint from the covering maps' (the EDW single-scan guards keep their scans).

Also fixed on the way: the aggregate lane's generic fallback projected only the SELECT variables, so a HAVING over a lifted aggregate saw it unbound and dropped every group when the lane declined at open; a sub-select column's type was trusted as exact unprobed.

Verification

Every admitted shape is pinned as its exact statement on the fake endpoint and checked against the per-scan lane (or the native twin where none exists), then replayed live on SQLite, Postgres and MySQL. Each feature was revert-checked: its path disabled, its pins watched fail. Clippy and the query, SQL, tabular, R2RML, graph-source and Iceberg policy suites are green locally; no workflow runs until the chain is retargeted to main.

Known harness issue, pre-existing: it_sql_graph_source::live_bridge_round_trip fails when the lane's live SQLite suite ran first on the same bridge (a pooled connection left in a read transaction sees the old people schema).

Follow-up: #1780
Fixes #1784

An f:onSubject policy made the whole block decline ("policy not static").
The gate now decides each (map, predicate) verdict per targeted subject
against the no-subject baseline, and the lane reverses every subject that
differs through the subject template into a predicate on the key columns:
NOT (id IN (1, 9)), or id IN (2, 3) under a deny default. A subject the
template cannot mint names no row and adds nothing; a constant subject
decides its map outright.

Row-dependent verdicts (by subject or by column-derived class) now also
apply to an optional entity, as conditions in its LEFT JOIN, so a hidden
row leaves the optional variables unbound. A subject policy beside a
class policy over a column-derived type still declines: that verdict is
joint per row. So does a row-dependent verdict on an optional member of
the entity it hides, which would need the column nulled, not the row
dropped.
A BIND kept the block on one statement but ran in the engine, so a FILTER
or an ORDER BY … LIMIT over the bound value could not push. The plan now
carries a scalar expression (columns, numeric literals, +, - and *), a
comparison of one with a literal, and an expression order key; the
renderer parenthesizes every operation so precedence is the plan's, and
types expression literals by their own kind. The lowering turns a BIND of
those operations over native numeric columns into an expression, answers
a filter over the bound variable with it (exact: the same promotion in
both worlds), and offers it as a top-k key when every column it reads is
required. The bound value is still computed in the engine, so its
datatype stays SPARQL's. Division stays in the engine (SPARQL divides
integers into a decimal, SQL into an integer), as does anything over a
string.

The fake endpoint parses and evaluates the parenthesized form and sorts
by expression keys. Pinned on the fake and live on SQLite, Postgres and
MySQL.
Every parity shape in it_iceberg_policy now also runs against a SQL
source over a fake endpoint holding the same five rows, so the pushdown
lane and the per-scan lane it declines to are held to the native twin:
property, class and subject targeting, wildcard scans, counts, f:query
failing closed, column-derived classes, dataset mode and top-k under a
subject deny. Gated on the sql feature.
…ment

A sub-SELECT inside a GRAPH block over a SQL source was refused outright:
the engine's subquery operator has no native index to run against, and
the lane did not admit it. The lane now lowers the sub-select's block on
its own (sharing the alias counter and the policy verdicts), groups it
through the same builder the grouped lane uses when it has a GROUP BY,
and joins it as a derived table on the projected variables' key columns.
Its aggregate outputs decode as the grouped lane's do; DISTINCT and
ORDER BY … LIMIT push inside it. Admitted: SPARQL's sub-SELECT, or a
JSON-LD subquery without a LIMIT per-row seeding could change; grouping
without HAVING or aggregate BINDs; no OFFSET, nested sub-select, BIND or
residual filter; no inner variable hidden from the enclosing block that
the block also uses. A sub-select the lane does not take still refuses
the query as before, never an empty answer.

Along the way: DISTINCT now pushes on MySQL when no projected column is
a string; the schema probe descends into sub-selects; the grouped plan
builder is a function of the lowered block rather than the operator.

There is no per-scan oracle for these shapes, so the lane suite gains a
native-ledger twin of the shop fixture and checks them against it, on
the fake and in the live replay on SQLite, Postgres and MySQL.
An entity several triples maps can provide — a predicate two maps mint,
on the same subject (a vertical partition carrying a duplicate) or on
different ones (people and companies both with names) — declined as
"entity spans several triples maps" or "predicate provided by several
triples maps". The lowering now enumerates every resolution (one choice
of providing map per member, the chosen maps minting the same subject; a
class member follows a chosen part declaring it) and, when there are
several, lowers each on its own and joins their `UNION ALL` once as a
derived table. Each branch's rows carry a tag naming the branch, so a
variable's term decodes through that branch's maps while the rest of
the block sees one relation.

Tabular IR: `RelNode::UnionAll { alias, branches }` and an integer
`OutputExpr::Tag`. The renderer types the union's columns from the first
branch and refuses misaligned branches; branches go bare, since SQLite
rejects a parenthesized compound member. The fake endpoint concatenates
`UNION ALL` branch rows and parses integer select items.

A variable keeps its key shape over the union's columns only where every
branch agrees on it, so filters, seeds and a top-k on a union variable
push; a slot every branch requires is required of the union. Derived
and union output types now feed the lowering's column typing, closing a
gap where a sub-select column was trusted as exact unprobed. Declines:
more than eight resolutions, branches of differing column types, a
foreign key into a union entity, an aggregate over one, a union inside a
sub-select.

Rows follow the per-scan lane: a triple two maps mint comes back once per
map. Pinned on the fake and replayed live on SQLite, Postgres and MySQL,
with a second subject template minting the shared predicate.
A `UNION` block ran one statement per branch combination: the branches
may bind a variable from columns of different types, and each carries
its own residual filters and materializer. They now share one statement
where the database can type it. `UnionLayout` assigns output slots by
(variable, column position, column type): branches binding a variable
on same-typed columns share a slot, a differently typed binding gets its
own, and a branch not binding it projects `NULL` there. Every row
carries a branch tag; the operator splits each page by it and runs each
branch's own materializer, join plan and residuals over its rows, so the
engine-side semantics are unchanged.

Padding a slot with NULL relies on the database typing the union's
column from the branches projecting a value there, which SQLite does not
(a compound's column takes the first branch's expression type), so a new
`union_null_is_typed` capability keeps SQLite on one statement per
branch whenever padding would be needed. Branches seeded differently by
the outer query, or disagreeing on whether their LIMIT is exact, stay
separate. A top-k pushes onto the union when every branch orders on the
same required column; otherwise the branches keep their own statements
and each its LIMIT.

Pinned on the fake (grouped statement, grouped top-k, a VALUES key set
joined inside every branch, the per-branch fallback for a top-k one
branch cannot order) and replayed live on SQLite, Postgres and MySQL.
BIND expressions pushed only arithmetic. `CONCAT`, `STRLEN`, `SUBSTR`
from a positive constant position and `STR` over plain string columns
and constants now lower to the dialect's own functions (`||` or MySQL's
`CONCAT()`, `LENGTH` or `CHAR_LENGTH`, `SUBSTR`), and a FILTER comparing
such an expression with a string literal pushes where the dialect
compares bytes, an ORDER BY over it where it orders code points. A
FILTER that writes the expression out instead of binding it pushes the
same way. Language-tagged strings, division and a SUBSTR from a
computed or non-positive position stay in the engine, where SQL and
SPARQL disagree.

`LCASE(?v) = "lit"` and `UCASE(?v) = "lit"` widen rather than push
exactly: every dialect's case mapping agrees with SPARQL's on printable
ASCII and not beyond it (SPARQL maps `ß` to `SS` and a ligature to two
letters; the databases do not), so the statement keeps the rows whose
folded value matches or that hold any other character, rendered per
dialect as a regex, `NOT REGEXP` or `GLOB`, and the engine still runs
the filter over what comes back.

The fixtures that used `STRLEN(?n) > 2` as the canonical unpushable
filter now use a `$`-anchored REGEX. Pinned on the fake and replayed
live on SQLite, Postgres and MySQL.
The grouped lane left every HAVING to the engine, and so could not push
a top-k under one. A HAVING made of AND/OR/NOT over comparisons of a
COUNT, or a SUM of integers or decimals, with a constant now goes with
the statement as `RelPlan.having`, rendered by repeating the output's
aggregate expression (Postgres and Trino do not resolve a select alias
there). The engine's HAVING still runs above it, and a top-k follows a
pushed HAVING. One over an AVG, which the engine divides, over a
MIN/MAX or over a group key stays in the engine and keeps the LIMIT
with it. A sub-select's HAVING pushes inside its derived table the same
way; one the lane cannot push still declines.

Admitting a HAVING whose lifted aggregate the projection drops exposed
a fallback defect: the generic pipeline behind the lane projected only
the SELECT variables, so when the lane declined at open the HAVING
above saw that aggregate unbound and every group was dropped. The
fallback now projects every group key and aggregate, as the lane's own
operator does.

Pinned on the fake and replayed live on SQLite, Postgres and MySQL.
A foreign key pointing at a union entity declined. The lowering now
learns, before an entity is lowered, which foreign keys may point at
it, and a union exposes the parent's join columns as slots of its own:
a branch on the parent's row carries them directly, a branch on the
parent's subject over another table takes the parent's row as a part
joined on the subject key, and when the key is certain to be placed a
branch minting another subject, which could never meet it, is dropped.

Pinning this exposed that the two lanes disagreed on how often a triple
two maps mint comes back, and that both were wrong: an RDF graph holds
a triple once, but the per-scan lane returned it once per map on an
unfused star and once per covering map on a fused one, and the pushdown
lane once per resolution. Two maps that mint a predicate alike, from
the same table, subject template and object map, now count as one
provider in both lanes (`TriplesMap::mints_alike`); maps deriving the
same value differently still come back once each. The per-scan lane's
star fusion also fused as soon as one map covered every member and lost
the rows of a map providing only some of them on the same subject; it
now fuses only when every partial provider's subject is provably
disjoint from the covering maps'.

The fixture gains an alias map minting `ex:label` from another table,
a distinct provider proper. Pinned on the fake and replayed live on
SQLite, Postgres and MySQL.
@bplatz bplatz added enhancement New feature or request area:query Query execution, planning, fast paths, overlay, result formatting labels Sep 3, 2026
@bplatz
bplatz requested review from aaj3f and zonotope September 3, 2026 23:31
Base automatically changed from feature/sql-pushdown-lane-m1 to main September 4, 2026 03:05
…lane-m3

Carries #1785 and the review round on it. Beyond the textual conflicts:
the subject-skeleton rule is ported into `parts_of`, so a union-entity
resolution over templates that provably never meet is skipped and one the
lane cannot relate declines; `collect_col_eqs` descends into derived tables
and UNION ALL branches so the column-class vet reaches every join the
statement renders; the fake's new predicate arms (`AsciiOnly`, `ExprCmp`)
are three-valued like the rest; the SUM-over-nullable top-k rule reads the
aggregates `group_plan` now receives directly.
…fferent skeletons

`order/{id}` and `order/{order_ref}/note` share a literal prefix, so the
lane cannot prove them apart, and differ in skeleton, so it cannot join
them either: an `order_ref` of `10/note` would mint an order's IRI. The
resolution declines to the engine. With the port reverted to skipping the
resolution, the block ran on the lane as empty.
Once the outer side outgrew one key set, `count_block` sent a full
`SELECT COUNT(*)` over the branch, an unbounded scan whose answer past the
cap is only "too large": on the 1M-row Postgres probe it added 18 ms to a
25 ms query and the block stayed seeded anyway.

The count now runs over the branch's own fetch statement as a derived
table with `LIMIT cap + 1`, so the probe scans no further than a fetch
would and a block past the cap costs the cap. The derived-table node and
its rendering already exist for sub-selects, and the fake endpoint reads
one; the shape is checked against Postgres 16 and MySQL 8 directly.

Fixes #1784

@aaj3f aaj3f left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bplatz having seen the branch PRs for what this builds on, I don't have a great deal to conceptually emphasize / agree with / contend with. This all makes sense insofar as the problems being solved / improved against the spec the base branches/PRs establish. Claude helped find a few places where the implementation doesn't fully satisfy the design rules of this and the base branches, so I'll just let Claude's own findings carry through below:


The first PR in the series to get the real CI matrix, @bplatz, and it's green across the board including the three-bridge live differential — and the shapes are the ones I'd have chosen: RelNode::UnionAll with a branch tag so each row decodes through its own maps, sub-selects as derived tables sharing the alias counter and the policy verdicts, expressions in the IR with the bound value still built in the engine, HAVING rendered by repeating the aggregate expression, subject policies reversed into key predicates exactly like #1785's class policies, and "held once" expressed as a mapping-level predicate rather than a runtime dedupe. The bounded COUNT probe is exactly what #1784 asked for, and a lot of the new surface held up under adversarial probing: the sub-select refusal path is loud, no policy bypass through derived tables or unions, the subject reversal handles the empty-allow-list and unmintable-subject corners, the LCASE/UCASE widening is sound by construction, and the engine's HAVING really does re-run above the pushed one (a mutation moved the SQL pins without moving the rows).

What I have to block on is six admitted shapes where the lane's rows differ from the oracle, each reproduced on the fake or in a scratch unit test and re-traced by me to the line, plus one perf regression on the Iceberg path. On the lane: a pushed HAVING over a nullable SUM drops the all-NULL group the engine keeps as 0 (HAVING(SUM(?d) >= 0): lane [], per-scan customer/1 s=0; the top-k rule beside it already has the nullability check, lower_having doesn't, and it also pushes inside sub-selects where no engine HAVING follows); a sub-select projecting an OPTIONAL variable is re-bound nullable: false and joined with = (native twin 4 rows, lane 2); and the FK-into-union filter drops a branch whose template shares the prefix but not the skeleton instead of declining it (order/{ref} vs order/{ref}-note: per-scan 4 rows, lane 3 — the existing /note fixture can't see it because / renders as %2F). On the per-scan side: the new dedupe ignores type_var, so ?s a ex:Store . ?s a ?t over two same-row maps with different class sets returns {Store, Shop} or {Store} depending on HashMap order (7 runs, 5 vs 2); the relaxed aggregate admission returns from the fast-path tail with no DistinctOperator, so SELECT DISTINCT over a grouped statement is lost (four n=1 rows where the oracle gives one; base declined the shape); and the fusion rule has no "minted alike" clause even though the same commit introduces mints_alike for the operator, so a Customer + CustomerCountry pair on one table and template — a star that was one scan at base — now runs as two scans of the same table plus an engine hash join with identical rows. That last one is the CRITICAL-perf item: the affected idiom (one map per class over one table sharing label-type predicates) is common on Iceberg, the plan it now takes is the 26–1000× one from #1777's own table, and the fix is one clause using same_source_row + mints_alike. One note from my own read, pre-existing rather than new: mints_alike compares only the first object map for a predicate, which mirrors the per-scan projection path (columns_for_predicate reads only the first POM per predicate), so a mapping with two rr:predicateObjectMap blocks for one predicate was already emitting only the first — worth saying so on object_map_for, not a change for this PR.

Fold-ins alongside, all inline: per-dialect typing of expression literals (SQLite computes ?t * 0.1 in REAL — = 9.95 loses order/10, reproduced; Postgres types 1E-1 as numeric, so a double filter pushes as exact where the engine says no — read, not run), the fake's two-valued Having::Cmp that masks the </NOT forms of the HAVING finding, unit and Iceberg-path pins for both per-scan changes (today each is caught by exactly one SQL-lane case through set_fast_paths_disabled, and iceberg.md doesn't mention either), the UNION ALL key set sent once per branch against a budget sized for one, and a subject cap on static_verdicts so a thousand-subject policy neither costs a thousand plan-time evaluations nor becomes a hard error at the provider. Optional: the shared people table between the two live suites only passes because cargo test runs binaries in name order (your "known harness issue" — per-suite table names fix it), JSON-LD twins for the sub-select and UNION ALL paths, and the _ => false arm for column/constant subjects in the fusion rule.

Adherence to repo commitments:

  • Patterns/abstractions: ✔ extends the lane's own IR (UnionAll, Derived, Expr, having), the policy gate (BySubject) and the mapping (mints_alike) rather than parallel constructs; ⚠️ the fusion rule and the operator dedupe disagree about "alike", and nullability doesn't survive the derived-table boundary.
  • Performance (speed first, memory second): ✖ CRITICAL on the Iceberg per-scan lane — a previously single-scan star splits for same-template partial providers (rewrite.rs:455); otherwise no per-row cost (dedupe once per open, fusion once per rewrite), native untouched, lane row path unchanged; UNION ALL key-set duplication and the uncapped subject IN are budget items; the bounded probe removes the +72% from #1785.
  • Testing: ⚠️ 17 lane + 13 policy-replay + 66 R2RML + 6 SQL-source cases green, real oracle, CI on the head green; ✖ six admitted shapes diverge from the oracle; the per-scan changes have no unit or Iceberg-path pin; the fake's HAVING is two-valued.
  • Conventions: ✔ twelve self-contained commits with real rationale, docs updated for the lane; ⚠️ iceberg.md untouched despite Iceberg-visible changes; one stale rustdoc (subquery_is_admissible still says "grouped without HAVING").

Verified locally at branch HEAD 7ba967805: it_sql_pushdown_lane 17/17, it_iceberg_policy 13/13, it_graph_source_r2rml 66/66 (+2 ignored), it_sql_graph_source 6/6 (--features sql,native,iceberg; live bridges skipped), fluree-db-query r2rml 163, fluree-db-r2rml 127; cargo fmt --all -- --check clean; clippy clean on the changed crates; throwaway probes for every divergence above (deleted); mutations (lower_having op flip → SQL pins red, rows unchanged; rewrite.rs → base and mints_alike → false → one SQL-lane case red each, no unit or Iceberg pin) restored; worktree clean.

Just be sure to get the three lane declines, the two per-scan correctness items and the fusion clause in before this merges — each is a few lines with the pin named inline — and I'd rather see the fold-ins land here than in the backlog. Happy to talk through any of them.

return None;
};
let output = match &decodes[group_by.len() + i] {
Decode::Count { name } => name.clone(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (correctness — the pushed HAVING is a strict subset for a nullable SUM). The admission here (Decode::Numeric { avg: false, kind: Integer | Decimal } a few lines down) has no nullability check, unlike the top-k rule beside it (:494-500) that keeps a SUM over a nullable member in the engine. SQL SUM of an all-NULL group is NULL, NULL >= 0 is UNKNOWN, and the database drops the group before the engine's HAVING ever sees it; SPARQL's SUM of the empty multiset is 0 (NumericAcc::finalize_sum, §18.5.1.3).

Reproduced on the fake (?d = orders.discount, nullable; customer 1 has no discount rows): SELECT ?c (SUM(?d) AS ?s) WHERE { ?o ex:customer ?c OPTIONAL { ?o ex:discount ?d } } GROUP BY ?c HAVING(SUM(?d) >= 0) — lane [], per-scan customer/1 s=0; same for = 0. The < 100 and !(… > 5) forms agree on the fake only because its Having::Cmp is two-valued (separate note) — Postgres/MySQL/SQLite return UNKNOWN there too. It also reaches sub-selects via group_plan (lower.rs:1207), where there is no engine HAVING above at all.

Fix: in cmp, return None when the summed variable is nullable — the same lowered.vars.get(v).is_none_or(|src| src.nullable) the top-k branch uses; Decode::Count stays (COUNT of NULLs is 0 in both worlds). The >= 0 shape fails on the fake today, so a case in aggregate_cases() has teeth without a live bridge.

VarSource {
term,
key,
nullable: false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (correctness — a sub-select projecting an OPTIONAL variable is joined with =). Every projected variable of a sub-select is re-bound as VarSource { …, nullable: false } (here and at :1420), discarding what the inner lowering knew — the inner ?k below is a LEFT JOIN column with no IS NOT NULL. bind_var (:2089) then unifies it with an outer binding through a ColEq edge instead of declining as it does for every other optional variable.

Reproduced against the native twin (customer 2 has no country): SELECT ?c ?k ?x WHERE { ?x ex:country ?k { SELECT ?c ?k WHERE { ?c ex:name ?n OPTIONAL { ?c ex:country ?k } } } } — twin 4 rows (c1,UK,x1) (c2,UK,x1) (c2,US,x3) (c3,US,x3), lane 2 rows; the statement joins … ON "t0"."country" = "d0"."c1". The control with no outer binding of ?k returns k= for customer 2 on both sides, so the nullable projection is admitted and reachable. (A VALUES seed on ?k, FILTER(!BOUND(?k)) and an outer ORDER BY ?k LIMIT 1 were all refused loudly — correct.)

Fix: carry lowered.vars[&v].nullable into the pushed VarSource on both paths (grouped keys included); bind_var then declines the unification and order_columns/seeds already skip nullable sources. Pin the shape in LANE_ONLY_CASES against the twin.

let Some(parent) = mapping.get(&inc.parent) else {
return Ok(Err(Decline("ref object map parent missing")));
};
alternatives.retain_mut(|parts| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (contract — a branch the lane cannot relate is dropped, not declined). The #1785 skeleton rule as ported into parts_of has three outcomes: provably disjoint → skip, same skeleton → join, otherwise → decline — and 052b21cc0 pins that decline. This retain_mut collapses the third outcome into a drop: a resolution is kept only if some part is same_row or same_subject with the FK's parent map, and anything else is silently removed as "could never meet".

Reproduced on a private fake source: notes(order_ref) rows 10, 10-note, 12; maps Note = order/{order_ref} and OrderNote2 = order/{order_ref}-note, both minting ex:memo; Pointer with ex:about as a RefObjectMap → Note on ref = order_ref. SELECT ?n ?o ?m WHERE { ?n ex:about ?o . ?o ex:memo ?m } — per-scan 4 rows including n=pointer/2 o=order/10-note m=gift wrap (OrderNote2 over the order_ref = "10" row mints exactly the IRI pointer/2 targets); lane 3 rows, a one-branch union. The existing /note fixture cannot catch this because / in a value renders %2F; any unreserved separator can collide.

Fix: drop a resolution only when every part is subjects_disjoint(tm, parent); a part that is neither same_subject nor disjoint must Decline("ref object map into a union entity over templates the lane cannot relate"). Pin with a -note template.

} else {
let mut kept: Vec<&TriplesMap> = Vec::with_capacity(triples_maps.len());
for tm in triples_maps {
let alike = kept.iter().any(|k| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (correctness on the per-scan lane — nondeterministic ?type loss). pattern_predicates() (:550-562) never includes rdf:type, and this closure never looks at pattern.type_var or compares class sets — only class_filter. A type_var scan emits one row per class of the kept map (:2620-2628), and CompiledR2rmlMapping.triples_maps is a HashMap, so which map survives is per-process random.

Shape: the try_fuse_wildcard_class two-scan path (rewrite.rs:1395-1412) puts class_filter on the standalone ?s a ?t scan. Over the one-table-per-class idiom — #StoreA dw.store store/{store_key} rr:class ex:Store, ex:Shop; ex:name←store_name and #StoreB dw.store store/{store_key} rr:class ex:Store; ex:name←store_nameSELECT ?s ?t { GRAPH <gs> { ?s a ex:Store . ?s a ?t . ?s ?p ?o } } returned ?t ∈ {Store, Shop} at base every time; at HEAD a scratch test in it_graph_source_r2rml run 7× gave {Shop, Store} 5× and {Store} 2×. Affects Iceberg sources.

Fix: add && (self.pattern.type_var.is_none() || k.classes() == tm.classes()) to the closure and pin it with two maps of differing class sets over one table.


/// The object map of the first predicate-object map naming `predicate`
/// as a constant.
pub fn object_map_for(&self, predicate: &str) -> Option<&super::term_map::ObjectMap> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional (pre-existing limitation, note only). mints_alike compares only the first predicate-object map naming the predicate. The loader emits one POM per rr:predicateObjectMap block, so a map can carry several for one predicate — but the per-scan lane's projection pushdown already reads only the first (columns_for_predicatefind_predicate_object_map, :118-160), so a second POM for the same predicate was never materialized under a predicate filter at base either.

Not a new drop, so nothing to change here; worth a comment on object_map_for saying it mirrors that rule, and a note somewhere that multiple POMs per predicate are unsupported on the projection path — a mapping author would expect both to emit. (Correcting my first read of this, which called it a new drop.)

impl Having {
fn eval(&self, members: &[Tuple], r: &Resolver<'_>, rels: &[Rel]) -> Result<bool, String> {
Ok(match self {
Having::Cmp(e, op, lit) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should-fix (the fake's HAVING is two-valued on NULL). Having::Cmp compares through cmp_values (:1174), whose (Value::Null, _) => Ordering::Less (:1197) makes NULL < 100 true and NOT (NULL > 5) true. Pred::ExprCmp (:1011-1015) correctly returns None for a NULL operand, and the merge commit says the new arms are three-valued, but this one isn't — it is what masks the < and NOT forms of the nullable-SUM HAVING finding. if v.is_null() { return Ok(false) } before the comparison makes those rows fail on the fake too.

let plans = branches
.iter()
.map(|&b| RelPlan {
root: self.seeded_root(b, keyset.clone()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should-fix (a UNION ALL statement repeats the whole key set in every branch). union_plan renders self.seeded_root(b, keyset.clone()) once per branch — visible in the pinned SQL as (VALUES (10), (12)) twice — but the chunk size is derived from keyset_max_rows / half statement_max_bytes for one copy (:1121-1122), so an N-branch union at a full 2,000-row chunk is N× the budgeted bytes and the database materialises the key set N times. Not wrong; it defeats the budget the cap exists for. Chunk at keyset_max_rows / branches when grouped, or hoist the key set to one JOIN outside the union on the shared slot (every grouped branch seeds on the same shape — that is the grouping precondition).

(true, false) => Verdict::Deny,
(false, _) => Verdict::ByClass { classes, otherwise },
let mut by_subject = Vec::new();
for subject in &subjects {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should-fix (scale, fold in now). static_verdicts evaluates allows() once per targeted subject per (map, predicate) — O(maps × predicates × subjects) async policy evaluations at plan time — and subject_pred (lower.rs:1725) folds every mintable subject into one IN (…) bounded only by statement_max_bytes at the provider, by which point the lane has committed, so an oversize statement is a hard query error rather than a decline. A policy naming thousands of subjects is exactly the per-user row-grant shape. A subject cap (e.g. keyset_max_rows) above which the verdict is None → "policy not static" → per-scan lane keeps both bounded. (Nothing per row: the only new allocation is the subjects.clone() here, once per block.)

@@ -687,9 +779,16 @@ impl SqlBlockSource {
QueryError::InvalidQuery("R2RML table provider not configured".into())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise. The bounded probe is built from plan_for_cache(branch) — the branch's own root with its predicates and joins, distinct preserved, no ORDER BY, no LIMIT — then limit = cap + 1 under COUNT(*) as a derived table, so a huge block costs the cap and a failing count leaves the branch seeded. Pinned text at lane test 2119 (… LIMIT 100001) AS "p") and 2144 under FLUREE_SQL_PUSHDOWN_CACHE_ROWS=2. LIMIT inside a derived table is accepted by Postgres, MySQL 8 (the restriction is on IN (SELECT … LIMIT)) and SQLite. That closes #1784 the way the #1785 review hoped it would; worth adding the probe shape to live_cases() so the three bridges replay it too.

@@ -0,0 +1,209 @@
//! One statement for the branches of a `UNION`: the branch plans

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise. UnionLayout assigning slots by (variable, column position, column type) — same-typed bindings share a slot, a differently typed one gets its own, a non-binding branch projects NULL — plus the integer branch tag so each row decodes through its own branch's maps is the right way to keep one statement without ever letting the database coerce across branches; union_null_is_typed false only for SQLite matches how SQLite types a compound column. Bag multiplicity across branches is preserved (tag split, no dedupe), and the eight-resolution cap keeps enumeration bounded. Policy verdicts reach derived tables and union branches through nested()/rejoin(), so neither is a policy bypass — I looked for one and didn't find it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:query Query execution, planning, fast paths, overlay, result formatting enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL pushdown lane: bound the block-size probe instead of a full COUNT(*)

2 participants