fix(sparql): CONSTRUCT set semantics (#54) + pre-ship review hardening#61
Merged
Conversation
) CONSTRUCT emitted one row per (solution × template-triple) pair, so a template producing the same triple across N solutions returned it N times. W3C SPARQL 1.1 §16.2 defines the result as an RDF *graph* — a set — which cannot hold a triple twice; the solution sequence's multiplicity does not survive into the output graph. The #17 differential oracle surfaced this on its first run (fixture 32: engine 3 rows vs W3C reference 1). Fix: row-level set-dedup at CONSTRUCT's return (dedup_construct_rows), applied on all three emission paths (constant fast-path ×2 + per-solution). Precise by construction — a template blank node mints a FRESH label per solution (§16.2.1), so its rows differ in their minted labels and correctly survive; only byte-identical rows (ground triples, within-solution repeats) collapse. Variable templates that bind distinct triples per solution are unaffected. - Corrected the pg_test that had codified the bug (construct_constant_template_three_solutions asserted 3 → construct_constant_template_dedups_to_set asserts 1) + a new test locking the subtle half (ground collapses, bnodes survive). - 100-construct-foundation invariant B golden 3 → 1, with the §16.2 set-semantics rationale corrected (it had repeated the same 'multiplicity matters' misreading). - Flipped fixture 32's oracle marker known-divergence(#54) → eligible; it now MATCHES spareval (oracle: 38 match, 0 diverge). 336/336 pgrx · 100/100 regression · 53/53 w3c + differential oracle · fmt + clippy clean. Closes #54
…rals, oracle harness Confirmed findings from the pre-v0.6.19 code review, fixed before the release tag: executor.rs (expression surface #50/#51): - math:pow now guards float8 OVERFLOW (y·ln|x| > 709.78 → UNBOUND), not just the 0^neg / neg^non-integer domain cases. pow(1e200,2) aborted the whole query, contradicting the tier's 'never a SQL abort' contract. [finding 2] - A constant INF / -INF / NaN xsd:double literal in the numeric lane mapped to an UNQUOTED (a column reference → query abort on a valid SPARQL literal). Now maps to Postgres' quoted 'Infinity' / '-Infinity' / 'NaN' numeric special-values; finite values unchanged. [finding 1] - math:exp guard widened 709 → 709.78 (ln(f64::MAX)); exp(709.5) is a representable double and was wrongly returning unbound. [finding 8] - dedup_construct_rows skips the serialize+hash pass for <= 1 row. [9] tests/oracle (differential harness #17): - engine() now returns the psql exit status; run() treats a non-zero exit (SQL error under ON_ERROR_STOP=1) as a hard fixture FAIL — previously an erroring zero-result fixture matched an empty golden and read as green, defeating the harness. [finding 5] - zero_elapsed_ms replaces EVERY occurrence (was first-only; the bash normalizer it replaced used sed -g). [finding 6] Intended-behavior findings (property-path WARN default, carve boundary NOTICE) are the #14 fail-closed visibility by design, kept. The oracle numeric_eq soundness tightening is tracked as #62 (v0.6.20). 339/339 pgrx (3 new math tests) · 100/100 regression · 53/53 w3c + oracle · oracle crate 35 · fmt + clippy clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #54, and folds in the pre-v0.6.19 code-review hardening (the review found genuine bugs before the tag — exactly what a pre-release review is for).
#54 — CONSTRUCT result is a set
CONSTRUCT emitted one row per (solution × template-triple), so a template producing the same triple across N solutions returned it N times. W3C §16.2 defines the result as an RDF graph — a set. Row-level dedup at the return (
dedup_construct_rows); minted template blank nodes get a fresh label per solution (§16.2.1) so their rows correctly survive, only byte-identical ground triples collapse. The #17 oracle caught this on its first run (fixture 32 flippedknown-divergence→eligible, now matches spareval).Review hardening (findings, all verified fixed)
src/query/executor.rs(expression surface):math:powoverflow → SQL abort.power(1e200, 2)aborted the whole query, violating the tier's "never a SQL abort" contract. Added the overflow guard (y·ln|x| > 709.78 → UNBOUND) alongside the existing domain guards.INF/-INF/NaNxsd:double literal → SQL abort. These lowered to unquotedINF::numeric(a column reference → abort on a valid SPARQL literal). Now mapped to Postgres' quoted'Infinity'/'-Infinity'/'NaN'numeric special-values; finite values unchanged. (Pre-existing bug, surfaced by the review.)math:expguard too tight.exp(709.5)is a representable double but returned unbound; guard widened709 → 709.78(ln(f64::MAX)).dedup_construct_rowsskips the serialize+hash pass for ≤1 row.tests/oracle/(differential harness):engine()now returns the psql exit status; a non-zero exit (SQL error underON_ERROR_STOP=1) is a hard fixture FAIL.zero_elapsed_msfirst-only. Restored global replace (the bash normalizer usedsed -g).Kept by design (not bugs)
Findings [3] (property-path truncation WARNs by default) and [4] (carve boundary NOTICE) are the #14 fail-closed visibility behavior — intentional, documented in the CHANGELOG.
Deferred with tracking
Finding [7] (oracle
numeric_eqblinds "2" vs "2.0") → #62 (v0.6.20 soundness tightening). The oracle is net-positive as-is (it caught #54 and #55).Verification (isolated container, this branch's
.so)-D warningsCHANGELOG deliberately untouched — folds into the v0.6.19 release cut (#60), where the
[0.6.19]entry gains the #54 + hardening lines.