fix(query): keep a VALUES written after an OPTIONAL below it - #1701
Conversation
`reorder_patterns` classifies `Pattern::Values` as an exact-cardinality source and lets it win the seed race, so a VALUES written after an OPTIONAL in the same group was hoisted ahead of it. That rewrites `Join(LeftJoin(P, O), V)` as `LeftJoin(Join(P, V), O)`, which is only the same query when V binds nothing O introduces. When it does, V seeds the variable before the left join runs, the left join matches the seeded value and — dropping nothing — lets every driving row out carrying it. The result is not merely too many rows: a row whose OPTIONAL bound `?b` to `ns:X0` was reported as `?b = ns:B`, a value the data never had. Add the missing arm to the order-sensitivity guard that already defers MINUS/EXISTS/NOT EXISTS and correlated subqueries. `values_needs_optional_barrier` fires only when a preceding OPTIONAL *introduces* one of the VALUES variables; a variable already bound ahead of that OPTIONAL is only being restricted, which commutes across the left join, so those VALUES keep their seed and the exact-cardinality seed race is untouched. A barriered VALUES is also dropped from `seed_anchor_vars`, since it seeds nothing. The barrier keeps the JOIN semantics of SPARQL 1.1 §18.2.4 — it is not a filter. A solution whose OPTIONAL left the variable UNBOUND is compatible with every VALUES row, so it survives and adopts the binding. W3C `sparql11/bindings/values07` pins that outcome; it passed already because it uses the post-query `} VALUES ...` spelling, which plans correctly. `DeferredPattern` gains a `nestable` flag so the barriered VALUES cannot be folded back above the OPTIONAL by `try_nest_deferred` when a preceding UNION/GRAPH/SERVICE happens to be placed early. Every existing deferral sets it true, so those paths are unchanged.
`Pattern::produced_vars` is a MAY-bind set: `Union` unions its branches,
`Graph` recurses straight through a nested `Optional`, and `Subquery` is
just the SELECT list. Using it to decide "this variable is already bound
ahead of the OPTIONAL, so the OPTIONAL only restricts it" turned the
barrier's carve-out into a suppressor: a variable bound in only ONE union
branch was recorded as required-bound, the barrier stopped firing for a
later sibling OPTIONAL that genuinely introduces it, and the fabricated
binding came back one UNION away from the shape the barrier was written
for.
Lift `must_bind_vars` as the single definition of "bound in every
solution this pattern emits" — empty for OPTIONAL and the pure row
filters, branch INTERSECTION for UNION, recursive through
GRAPH/SERVICE/DefaultGraphSource, and `select` ∩ body-must-bind for a
subquery. `subquery_correlation_vars` needed exactly this rule and stated
it as an inline `matches!` allow-list that was flat and treated UNION as
wholly absent; it now shares the helper, so the two relations over the
same concept cannot drift.
Detection is now a separate notion from accumulation.
`left_join_introduced_vars` recurses through preceding containers, so an
OPTIONAL nested in a GRAPH/SERVICE/UNION raises the barrier too — the
gap previously documented as a scope note. It deliberately does NOT
include a UNION that merely binds a variable on one branch: `Join` is
commutative, so a VALUES hoisted past a UNION still meets the other
branch's rows unbound and they still adopt the value. Branch
intersection separates the cases exactly, so `{?f} UNION {?f}` keeps its
seed while `{?f} UNION {?e}` raises the barrier.
The barrier is also positional now. `required_vars` is a
variable-readiness test, so a sibling UNION branch binding the same
variable satisfied it and drained the VALUES before the OPTIONAL had been
placed at all — the plan was still wrong with the must-bind fix alone.
`after_indices` names the blocking patterns and `drain_ready_deferred`
holds the VALUES until they are placed.
Tests: UNION-in-front at both the planner and SPARQL levels (both go red
under a may-bind mutation), an OPTIONAL nested in a preceding GRAPH, a
subquery exposing an OPTIONAL-bound var, and a pin that a UNION with no
left join keeps its seed. `trailing_values_is_a_join_not_a_filter` now
asserts both answer sets — `assert_ne!` alone was satisfied by the wrong
VALUES answer too, so it passed with the fix reverted.
bplatz
left a comment
bplatz
left a comment
There was a problem hiding this comment.
Approving. The diagnosis is right, the instrument is right, and the second commit catches a real bug the first one had.
I verified the defect independently on pristine main (fe3c198c8) rather than taking either the issue or the PR at face value. On seed_values_dataset: in-group VALUES 5 rows including ["ex:alice","ex:alice"], post-query 4, FILTER 2. ex:alice's only ex:friend is ex:brian, so that row reports a binding the data never had, and two spellings of one query disagree. Real bug, correctly fixed — the branch returns 4. The must-bind carve-out also checks out algebraically: when V's vars are must-bound by P, the same P rows survive the restriction on either side of the left join, so keeping the seed is safe.
One thing worth correcting on #1690
The filed repro doesn't actually demonstrate the bug. I modelled its shape exactly — one subject has the OPTIONAL predicate, every other driving row leaves the variable unbound (ex:cool is only on ex:nikola):
main, in-group VALUES: 5 rows
main, post-query VALUES: 5 rows <- identical
Buggy and correct paths agree. In #1690's numbers: 1 bound-compatible row + 1499 unbound-and-adopting rows = 1500, which is what the engine returned. Its "correct answer is 1" is the FILTER answer, and trailing_values_is_a_join_not_a_filter in this PR is the argument for why that isn't the equivalent rewrite.
So I'd put it differently than "the original report framed it as a cross-product: it is worse than that" — the original fixture was coincidentally clean, and the defect only surfaces once the OPTIONAL binds a different value. That makes this PR's fixture choice the load-bearing part rather than an incidental one, and it's worth saying on the issue so the repro doesn't teach the wrong diagnostic.
#1713 is wider than filed, and sharper
Same defect through a second door, confirmed pre-existing on main:
?s schema:name ?name .
VALUES (?s ?f) { (ex:alice UNDEF) (ex:cam UNDEF) (ex:liam UNDEF) (ex:brian UNDEF) (ex:nikola UNDEF) }
OPTIONAL { ?s ex:friend ?f }8 rows, correct multiplicity, ?f Null on every one. The OPTIONAL matched the triples and then didn't write the binding, because ?f was already in the row as Unbound. That's stronger than #1713's current "the left join is a no-op" — worth adding there.
Before merge
Only the first inline comment. The other two are a comment edit and a typo.
| } | ||
| // Triples, property paths, BIND/UNWIND/VALUES, search adapters: every | ||
| // solution they emit carries their produced vars. | ||
| other => other.produced_vars().into_iter().collect(), |
There was a problem hiding this comment.
Pattern::Values lands here, and it is may-bind: lower_values_pattern (fluree-db-sparql/src/lower/pattern.rs:225) emits Binding::Unbound for UNDEF, so an UNDEF column doesn't bind its variable in every row. That leaves the one may-bind variant this arm still misclassifies as the pattern kind the barrier is actually about.
Reproduces the original fabrication on this branch:
?s schema:name ?name .
VALUES (?s ?f) { (ex:alice UNDEF) (ex:cam UNDEF) (ex:liam UNDEF) (ex:brian UNDEF) (ex:nikola UNDEF) }
OPTIONAL { ?s ex:friend ?f }
VALUES ?f { ex:alice }5 rows including ["ex:alice","ex:alice"]; the reference answer is 4. The leading table records ?f as required-bound, the barrier doesn't fire, and the trailing VALUES seeds. This is exactly the suppressor class cde7d129a was written to close — and UNDEF-in-VALUES is the parameterized-query idiom #1690 names as the motivating usage, so it isn't a synthetic corner.
Honest caveat on the fix: I patched this arm to exclude columns containing an Unbound cell and the plan corrects, but the answer becomes 8 rows still carrying the fabricated one, because #1713 bites the same query. So it's a plan-shape fix only — the same posture you already took for the UNION shape, and the same treatment fits: an arm here plus a plan-shape test.
If you'd rather not widen, the alternative is fine too: say plainly in the doc comment above that Values (UNDEF) and Bind (see the other comment) are known may-bind exceptions this function deliberately counts. What I'd avoid is leaving the contract stated as unconditional, since the whole value of the helper is that a future reader trusts it.
| /// Shared with [`subquery_correlation_vars`], which needs the identical rule to | ||
| /// decide whether a shared SELECT-list variable is a join key or a correlation | ||
| /// input — it used to state it as an inline `matches!` allow-list. One relation | ||
| /// over one concept, so the two cannot drift. |
There was a problem hiding this comment.
This claim is a bit stronger than the code supports, and in a direction that could bite later.
Pattern::Bind is may-bind too — bind.rs:10, evaluation errors produce Binding::Unbound — and a BIND-then-OPTIONAL lead reproduces the fabrication the same way a UNION lead does. But subquery_correlation_vars deliberately counts BIND, and its own comment says why (a var the subquery produces via a WITH-pipeline binder must not be mistaken for an external correlation). So for Bind the two call sites genuinely want different answers.
Not something to fix here — the current behaviour is right at both sites in practice. The risk is that "one relation over one concept, so the two cannot drift" reads as an invariant, and someone later tightens must_bind_vars for Bind on that basis and quietly breaks subquery correlation. A sentence noting that Bind is a deliberate exception, and which site depends on it, would inoculate that.
| /// question: it unions a `UNION`'s branches (`ir/pattern.rs`) and recurses | ||
| /// straight through a nested `Optional`. Reading it as "already bound" is what | ||
| /// let a variable bound in only ONE union branch suppress | ||
| /// [`values_needs_optional_barrier`] for a later sibling OPTIONAL that genuinely |
There was a problem hiding this comment.
values_needs_optional_barrier doesn't exist — the function is values_optional_barrier_indices. Same stale name at planner.rs:1524. This one is inside a /// block so it's a broken intra-doc link; CI runs clippy + nextest, not cargo doc, so nothing will catch it.
`must_bind_vars` fell through to `produced_vars` for `Pattern::Values`,
which is its whole variable list. Both surfaces lower `UNDEF` to
`Binding::Unbound` (`lower_values_pattern`, `lower_values_cell`), so an
all-UNDEF column binds its variable in no row at all while reading as
required-bound — the last may-bind variant the arm misclassified, and the
same suppressor class the barrier's must-bind rule was written to close.
A leading `VALUES (?s ?f) { (ex:alice UNDEF) … }` therefore recorded `?f`
as required-bound, the barrier didn't fire for a following
`OPTIONAL { ?s ex:friend ?f }`, and the trailing `VALUES ?f` went back to
seed position — the #1690 fabrication, reachable one UNDEF away from the
shape the barrier was written for. UNDEF is the parameterized-query idiom
#1690 names as its motivating usage, not a corner.
The arm counts only the columns with no `Unbound` cell. A zero-row table
stays vacuously must-bind on every column (it emits no solutions at all —
the missing-predicate sentinels in `parse/lower.rs` and
`empty_path_result` are both this shape), and Cypher's WITH-pipeline
binders never emit `Unbound`, so the narrowing cannot reach the
`self_produced` set `subquery_correlation_vars` builds for them.
Plan-shape only, and deliberately: with the arm the trailing VALUES lands
above the left join as it must, but the answer becomes 8 rows still
carrying the fabricated one, because #1713 bites the same query. Same
posture as the UNION shape — `an_undef_values_column_does_not_suppress_the_barrier`
asserts the physical plan and says why. `an_undef_column_does_not_suppress_the_optional_barrier`
pins the planner ordering; `a_bound_values_column_still_seeds_past_an_optional`
pins the over-fire direction (a fully-bound column must keep the seed).
Each reddens under exactly one mutation and neither under the other.
Also documents `Bind` as a known may-bind exception the function
deliberately counts, and which call site depends on it: it is may-bind for
the same reason (`bind.rs` yields `Binding::Unbound` on an evaluation
error), but `subquery_correlation_vars` needs it counted or a variable the
subquery produces through a WITH-pipeline binder is read as an external
correlation. The two sites genuinely want different answers there, so the
contract is stated with the exception rather than as unconditional.
Fixes the stale `values_needs_optional_barrier` intra-doc link (the
function is `values_optional_barrier_indices`) at both sites.
|
Thanks @bplatz — the UNDEF case is a real hole and I've widened rather than documented around it, and both of your issue corrections were right. The Two edge cases the arm handles deliberately, both of which are ways it could have regressed something: a zero-row table stays vacuously must-bind on every column — the missing-predicate sentinel in On The stale name is fixed in both places ( Each direction of the new arm is pinned by exactly one test, and I checked they don't overlap: deleting the Both of your issue corrections are posted. On #1690 — you're right that the filed repro doesn't demonstrate the bug, and I've said so there: where every non-matching driving row leaves the variable unbound, the fabricated and legitimately-adopting rows are indistinguishable by count, so buggy and correct paths agree at 5. I've also noted that its "correct answer is 1" is the FILTER answer, and reframed this PR's opening so the fixture choice reads as load-bearing rather than incidental. On #1713 — confirmed on the branch and it's sharper than either of us put it. Your query returns 8 rows with W3C 36/36 with registers untouched, and the usual |
2 participants
{ ?ev :entity1 ?a . OPTIONAL { ?ev :entity2 ?b } VALUES ?b { :B } }has been returning rows that report a binding the data never had for them. On a 6-row fixture where the OPTIONAL binds?btons:X0..X5/ns:B, the in-group VALUES form returned all six rows saying?b = ns:B— a fabricated value, not merely an extra row. Correct answer there is 1 row. Both surfaces were affected (the JSON-LD["values", …]after["optional", …]clobbered identically), which is the tell that the cause is in the shared planner rather than in SPARQL lowering.The fixture choice turns out to be load-bearing rather than incidental, so it's worth stating up front. The defect only surfaces once the OPTIONAL binds a different value than the VALUES supplies. On a shape where one subject has the OPTIONAL predicate and every other driving row leaves the variable unbound, the buggy and the correct paths agree exactly — the fabricated rows and the legitimately-adopting rows are indistinguishable by count. #1690's own filed repro is that shape, which is why its 1500 rows look like a cross-product and its "correct answer is 1" is really the FILTER answer (see below — FILTER is not the equivalent rewrite). The tests here deliberately use a fixture where the OPTIONAL binds something else, because that is the only arrangement that separates the two.
Mechanism
reorder_patternsinfluree-db-query/src/planner.rsclassifiesPattern::Valuesas an exact-cardinality source, and — viaseed_anchor_vars— it wins the seed race outright. EXPLAIN's logical plan listssource valuesfirst even for a VALUES written last.The order-sensitivity guard immediately above it (the
Minus | Exists | NotExistsarm, and the correlated-subquery deferral just below) exists precisely so the reorder cannot outrun what feeds a pattern. AValueswhose variable a precedingOptionalproduces was never covered by it. That is the whole gap.In algebra terms the hoist rewrites
and those are the same query only when
Vbinds nothing thatOintroduces. When it does,?bis already bound before the left join runs, the left join matches on the seeded value, and — being a left join — drops nothing, so every driving row exits carrying it. The physical plan shows it directly:OptionalOperator > ValuesOperatorbefore,ValuesOperator > OptionalOperatorafter.The fix
A small order-sensitivity barrier alongside the existing MINUS/EXISTS one.
values_optional_barrier_indices(planner.rs) answers "does a precedingOptionalintroduce one of this VALUES' variables"; when it does, the VALUES is deferred on the preceding patterns' produced vars — which include the OPTIONAL's own, perPattern::produced_vars— so it lands immediately after the OPTIONAL.Two deliberate narrowings, both to avoid paying for this where it isn't a hazard:
VALUES ?evover an?eva preceding triple binds keeps its seed. Without that carve-out every OPTIONAL mentioning the seed variable would cost the exact-cardinality seed, which is the 21k-row UNWIND cliffvalues_seed_beats_disconnected_class_anchorwas written to pin;values_on_a_required_var_still_seeds_past_an_optionalguards the carve-out from the other side.seed_anchor_vars. It isn't seeding anything, so letting it demote a disconnected class anchor would cost that anchor its seed with nothing taking its place.The carve-out has to be computed over must-bind, not may-bind
This is the load-bearing subtlety, and it's wrong in a way that looks right.
Pattern::produced_vars()is a may-bind set:Unionunions its branches,Graphrecurses straight through a nestedOptional,Subqueryis just the SELECT list, andValuesis its whole variable list regardless ofUNDEF. Accumulate "already bound ahead of the OPTIONAL" from that and the carve-out above stops being a narrowing and becomes a suppressor — a variable bound in only one branch/column is recorded as required-bound, the barrier stops firing for a later sibling OPTIONAL that genuinely introduces it, and the hoist comes straight back one UNION (or oneUNDEF) away from the shape the barrier was written for.So three things hold it together:
must_bind_varsis the single definition of "bound in every solution this pattern emits" — empty forOptionaland the pure row filters, branch intersection forUnion, recursive throughGraph/Service,select ∩ body-must-bindfor a subquery, and forValuesonly the columns with noBinding::Unboundcell. I didn't write it fresh:subquery_correlation_varsneeds exactly this rule and already stated it almost verbatim in its own comment, as an inlinematches!allow-list that was flat (no container recursion) and blunt about UNION (simply absent). Both call sites now share the helper. Worth flagging thatcollect_guaranteed_varsis not this despite the name — it's plainproduced_varswith the intersection done at its call site.left_join_introduced_varswalks preceding containers, so an OPTIONAL nested in aGRAPH/SERVICE/UNIONraises the barrier too. It deliberately does not include a UNION that merely binds a variable on one branch, becauseJoinis commutative: a VALUES hoisted past a UNION still meets the other branch's rows unbound and they still adopt the value. Branch intersection separates the two exactly —{?f} UNION {?f}keeps its seed (it really is must-bound),{?f} UNION {?e}raises the barrier.required_varsis a variable-readiness test, so a sibling UNION branch binding the same variable satisfies it and drains the VALUES before the OPTIONAL has been placed at all.after_indicesnames the blocking patterns anddrain_ready_deferredholds the VALUES until they're placed.UNDEFis the second may-bind spelling, and it's the motivating onePattern::Valuesis itself may-bind, and it was the last variant the arm misclassified. Both surfaces lowerUNDEFtoBinding::Unbound— SPARQL inlower_values_pattern, JSON-LD inlower_values_cell— so an all-UNDEFcolumn binds its variable in no row whileproduced_varsstill reports it. That reproduces the fabrication with no UNION anywhere:The leading table recorded
?fas required-bound, the barrier didn't fire, and the trailing VALUES seeded: 5 rows including["ex:alice","ex:alice"], whose onlyex:friendisex:brian. The reference answer is 4. This is not a synthetic corner — a placeholderUNDEFcolumn is exactly the parameterized-query idiom #1690 names as its motivating usage, which makes it the shape most likely to be hit in the wild.must_bind_varsnow counts only the columns with noUnboundcell, so?sstays required-bound and?fcorrectly does not. A zero-row table stays vacuously must-bind on every column — it emits no solutions at all, and the missing-predicate sentinels inparse/lower.rsandempty_path_resultare both that shape. Cypher's WITH-pipeline binders never emitUnbound, so the narrowing cannot reach theself_producedsetsubquery_correlation_varsbuilds for them.This one is a plan-shape fix only, and the test says so. With the arm the trailing VALUES lands above the left join as it must — and the answer becomes 8 rows still carrying the fabricated one, because #1713 bites the same query (below). Same posture as the UNION shape:
an_undef_values_column_does_not_suppress_the_barrierasserts the physical plan, not the rows.Bindis a deliberate exception, and the contract says soPattern::Bindis may-bind for the same reason —bind.rsyieldsBinding::Unboundwhen the expression errors — and a BIND-then-OPTIONAL lead can fabricate the same way a UNION lead does.must_bind_varscounts it anyway, becausesubquery_correlation_varsneeds it counted: a variable the subquery produces through a WITH-pipeline binder must not be read as an external correlation, or the subquery is deferred on a variable only it can bind. That is the one concept where the two call sites genuinely want different answers, and it's resolved in the correlation site's favour.So the doc comment states it as an exception rather than as an unconditional contract, and names which site depends on it. The helper's whole value is that a future reader trusts it, and "one relation over one concept" would otherwise read as an invariant — someone tightening
Bindhere on the strength of it would quietly break subquery correlation. It needs a fix at both sites or at neither.Two things here are load-bearing and shouldn't be simplified away
1. This isn't an open semantics question.
testsuite-sparql/rdf-tests/sparql/sparql11/bindings/values07is exactly this shape (?s ?p1 ?o1 . OPTIONAL { ?s foaf:knows ?o2 } } VALUES (?o2) { (:b) }), it isn't in the skip register, and its.srxpins the answer at 5 rows. It has been passing all along — because it uses the post-query spelling (} VALUES …), which we already plan correctly. So the correct behaviour existed in-engine on the sibling path the entire time; this PR just makes the in-group spelling agree with it, and there's now a test asserting the two spellings return the same rows on the same data.2.
FILTERis not the equivalent rewrite, and a "drop the unbound rows" fix would be wrong. Per SPARQL 1.1 §18.2.4 this staysJoin(…, ToMultiSet(data)), and Join keeps a solution when the two mappings are compatible — which, for a variable the OPTIONAL left UNBOUND, is trivially true. Those rows survive and adopt the VALUES binding.FILTER(?b = :B)evaluates to a type error on an unbound?band drops the row; VALUES adopts it. The distinction is invisible on a fixture where every row has the variable bound, which is exactly why an over-fix here would look green — so the tests pin all three outcomes on one fixture:?ftoex:cam,ex:liamex:aliceex:aliceex:brianex:brian,ex:nikolaex:friendat allex:alicetrailing_values_is_a_join_not_a_filterasserts both answer sets exactly rather than just asserting they differ. Anassert_ne!between the two is vacuous here — the pre-fix VALUES answer is also unequal to the FILTER answer, so it passes with the barrier fully reverted. Pinning both sets exactly goes red under that mutation.A second, pre-existing defect this deliberately doesn't fix
Two of the shapes above are planned correctly by this PR and still answer wrong, for a second and unrelated reason. The rows they return are the faithful §18.2.4 join of a wrong input.
The clearest statement of it needs no VALUES-after-OPTIONAL at all:
returns 8 rows — the correct multiplicity — with
?fnullon every single one. Delete the VALUES line and the same query returns those 8 rows with?fbound on 6 of them (ex:alice→ex:brian,ex:cam→ex:alice/ex:brian,ex:liam→ex:alice/ex:brian/ex:cam, andex:brian/ex:nikolagenuinely unbound). So the OPTIONAL did match the triples — the row count proves it — and then didn't write the binding, because?fwas already present in the row asUnbound. That is stronger than "the left join is a no-op": the join happened and the result was discarded.The UNION spelling is the same defect through a third door:
returns 10 rows — byte-identical to the same query with the
OPTIONALdeleted, where §18.2.4 says 13. I verified both on pristinea824bbfd1withplanner.rsandwhere_plan.rsreverted — no part of this PR present — and they reproduce identically. Neither can be this PR by construction: the second query contains noPattern::Valuesand no subquery, so neither the barrier normust_bind_varsis reached.This looks like the
nested-opt-1/nested-opt-2/join-scope-1correlated-OPTIONAL-independence cluster already registered as a known divergence. It's filed as #1713 and I'd rather not widen this PR into it. The consequence:a_union_binding_the_var_does_not_suppress_the_barrierandan_undef_values_column_does_not_suppress_the_barrierassert the plan shape, not the rows, and say why in the tests — pinning rows there would pin the other defect's output. The row-level §18.2.4 contract is carried by the tests whose inputs are correct.Tests
planner.rs—values_after_optional_is_not_hoisted_above_it;union_may_bind_does_not_suppress_the_optional_barrierandan_undef_column_does_not_suppress_the_optional_barrier(the two may-bind hazards above);pure_union_without_an_optional_keeps_the_values_seedanda_bound_values_column_still_seeds_past_an_optional(the over-fire direction —{?f} UNION {?f}and a fully-bound VALUES column must both keep the seed);optional_nested_in_a_preceding_graph_still_raises_the_barrier;values_before_optional_still_seeds;values_on_a_required_var_still_seeds_past_an_optional(the carve-out that protects the UNWIND-cliff seed);subquery_conditionally_bound_output_stays_below_the_subquery.where_plan.rs—values_after_optional_builds_above_the_optional, the physical-plan pin viaOperator::describe, so a future reorder change can't silently reintroduce this without a plan-shape failure.it_query_values.rs—trailing_values_over_an_optional_var_joins_instead_of_seeding(asserts the fabricated["ex:alice","ex:alice"]row is absent, then pins the exact 4-row answer),in_group_values_matches_the_post_query_spelling,trailing_values_is_a_join_not_a_filter,a_union_binding_the_var_does_not_suppress_the_barrier,an_undef_values_column_does_not_suppress_the_barrier,a_subquery_exposing_an_optional_bound_var_still_joins, and the JSON-LD twinjsonld_values_after_optional_joins_instead_of_seedingper the three-surface parity rule.One supporting change:
DeferredPatterngains anestableflag, anddrain_ready_deferredhonours it. Every existing deferral setsnestable: true, so those paths are byte-identical; only the barrier sets itfalse. I couldn't actually construct a case that reaches the state it guards — the barriered VALUES can't become ready until the OPTIONAL is placed, at which pointtry_nest_deferredrejects it anyway. It's kept as a cheap invariant guard in caserequired_varsis ever narrowed, and the doc comment says plainly that it's defensive and untested so nobody reads the missing test as missing coverage.Mutation results, since "the tests are green" isn't the claim worth making:
it_query_valuestestsmust_bind_vars→produced_vars(both may-bind bugs)union_may_bind_does_not_suppress_the_optional_barrier,an_undef_column_does_not_suppress_the_optional_barrier, and their twoit_query_valuestwinsValuesarm deleted (UNDEF alone)an_undef_column_does_not_suppress_the_optional_barrier+an_undef_values_column_does_not_suppress_the_barrier, and nothing elseValuescontributes no must-bind vars (over-fire)a_bound_values_column_still_seeds_past_an_optional, and nothing elseThe last two are the point of the pair: each direction of the
Valuesarm is pinned by exactly one test, and neither test goes red under the other's mutation. The carve-out and perf pins (values_before_optional_still_seeds,values_on_a_required_var_still_seeds_past_an_optional,pure_union_without_an_optional_keeps_the_values_seed,values_seed_beats_disconnected_class_anchor) stay green under all four, which is what they're for. Two tests are honestly vacuous w.r.t. the barrier and say so in their own comments: both subquery ones, because the correlated-subquery deferral already keeps the VALUES down for that shape — unsound on paper, not reachable in practice, andmust_bind_varscloses it regardless.W3C suite
I did run it —
testsuite-sparql,cargo test --test w3c_sparql, submodule initialised: 36/36 groups green, registers untouched. Nothing flipped in either direction.values07lives insparql11_bindingsand was already passing on the post-query spelling, so the fix neither greens a registered failure nor costs a passing test.Worth saying plainly rather than letting the number imply more than it does:
values07is the only file in the entire W3Cbindings/directory that contains anOPTIONAL, so the suite has never covered the in-group spelling at all. 36/36 means this broke nothing — not that the suite validates the fix. The integration tests are carrying that load on their own.The suite matters for a second reason here:
subquery_correlation_varsnow sharesmust_bind_vars, which both widens itsself_producedset (the old inline allow-list excludedGraph/Service/search adapters outright) and narrows it for anUNDEFcolumn.sparql11_subqueryandsparql10_query_eval_testsare the oracle for that, and both are green.Gates
cargo test -p fluree-db-query --lib— 1431 passed, 0 failed, 1 ignoredcargo test -p fluree-db-api --test grp_query_sparql— 356 passed, 0 failedcargo test -p fluree-db-api --test grp_query— 422 passed, 0 failed, 2 ignoredcargo test -p fluree-db-api --features native --test it_values_object_bounds— 9 passed, 0 failed (the fix: make IN/NOT IN match encoded resources, price the row lanes max_fuel could not see #1681 regression file this issue came out of)cargo test -p fluree-db-api --test it_query_explain— 13 passed, 0 failedtestsuite-sparqlcargo test --test w3c_sparql— 36 passed, 0 failedcargo clippy -p fluree-db-query -p fluree-db-api --all-targets --no-deps— clean, zero warningscargo fmt --all(root) andcargo fmt --all -- --checkintestsuite-sparql— both cleanFixes #1690