Skip to content

fix(engine): CONSTRUCT and DESCRIBE go through PreparedQuery and honour BASE - #6677

Merged
jeswr merged 7 commits into
mainfrom
fix/engine-batch-3
Oct 8, 2026
Merged

jeswr merged 7 commits into
mainfrom
fix/engine-batch-3

Conversation

@jeswr

@jeswr jeswr commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Before: construct_or_describe (the graph-valued entry point the CLI, server, Python wheel and MCP use) parsed the query directly. CONSTRUCT and DESCRIBE never went through the algebra-rewrite pass that EXPLAIN shows (#4748). They also never installed the query's BASE, so IRI("rel") in a CONSTRUCT WHERE clause errored and left the template slot unbound.

After: construct_or_describe parses through PreparedQuery::parse and delegates to the prepared CONSTRUCT/DESCRIBE paths, so it sees the same rewrites as EXPLAIN. Those prepared paths install the query BASE with the same scoped guard as SELECT and ASK, so relative IRIs resolve.

The rewrite pass only touches the WHERE pattern and keeps the same solutions, so CONSTRUCT and DESCRIBE output is unchanged.

How: one function in construct.rs now delegates instead of duplicating the evaluation. set_query_base is called in the prepared CONSTRUCT/DESCRIBE functions. New tests: rewrite_seam_tests in construct.rs and a CONSTRUCT BASE case in tests/query_base_scope.rs.

Closes #4748.

Benchmark

Local sparq-cli bench, operators suite (2,000 entities, 16k triples), release-fast binaries of main a6ce3785b4 and this PR. Best of 5 iterations per round, minimum over 5 interleaved rounds; row counts match on every query. Geomean ratio 0.998. The one query over 1.05 (q13_bind 1.053) was 0.995 on a second run, which instead showed q01_bgp at 1.156 (0.98-1.02 in the first run); neither repeats, so both are noise. q27_construct and q28_describe, the paths this PR touches, are 1.007 and 0.984.

query main (µs) PR (µs) PR/main
q01_bgp 3.4 3.4 1.000
q02_star3 10.2 10.6 1.039
q03_chain 235.3 239.0 1.016
q04_triangle 4120.9 4147.4 1.006
q05_union 6.9 6.9 1.000
q06_optional 50.0 49.2 0.984
q07_optional_notbound 31.5 31.3 0.994
q08_minus 14.8 14.9 1.007
q09_filter_numeric 165.1 172.6 1.045
q10_filter_string 797.2 834.3 1.047
q11_filter_in 247.1 244.3 0.989
q12_filter_exists 418.7 429.2 1.025
q13_bind 840.9 885.5 1.053
q14_values 10.0 10.2 1.020
q15_agg_group_having 302.3 306.7 1.015
q16_distinct 9.8 9.9 1.010
q17_orderby_limit_offset 18.3 18.2 0.995
q18_path_plus 602.6 618.8 1.027
q19_path_star 1130.8 1100.0 0.973
q20_path_opt 8.0 7.0 0.875
q21_path_seq 10.9 10.6 0.972
q22_path_alt 7.2 6.6 0.917
q23_path_inverse 7.2 7.2 1.000
q24_path_negated_pset 7.2 6.7 0.931
q25_subquery 418.4 438.6 1.048
q26_ask 74.5 73.3 0.984
q27_construct 14.9 15.0 1.007
q28_describe 6.4 6.3 0.984
geomean 0.998

🤖 Generated with Claude Code

https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR


Generated by Claude Code


Generated by Claude Code

claude added 3 commits October 5, 2026 19:31
The graph-valued entry point used by the CLI, server, wheel and MCP parsed
the query directly, so CONSTRUCT / DESCRIBE never saw the algebra-rewrite
pass while EXPLAIN did. Parse through PreparedQuery and delegate to the
prepared CONSTRUCT / DESCRIBE paths. The pass rewrites only the WHERE
pattern and is bag-equivalent on its solutions, so output is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
The CONSTRUCT / DESCRIBE entry points never set the query's BASE IRI,
so IRI("rel") in their WHERE clause errored (an unbound template slot)
instead of resolving against the BASE. Install it with the same scoped
guard as the SELECT / ASK entry points.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
@jeswr
jeswr marked this pull request as ready for review October 6, 2026 00:10
@jeswr

jeswr commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head ce70ba98884e. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Routing through the rewrite introduces false zero-length path matches — crates/sparq-engine/src/construct.rs:128

    With algebra-rewrite enabled (the CLI/server defaults), running this through construct_or_describe on an empty graph previously returned no triples:

    PREFIX ex: <http://ex/>
    CONSTRUCT { ?s ex:r ?o }
    WHERE { ?s ex:p* ?o FILTER(?s = ex:missing) }

    The rewrite substitutes ex:missing into the path and rebinds ?s. eval_path then produces the constant endpoint’s zero-length match, constructing ex:missing ex:r ex:missing. The original variable-to-variable path has no matches on an empty graph; these forms have different SPARQL zero-length path semantics. This exposes an existing rewrite bug through the newly routed graph entry point.

    Make equality substitution decline zero-length-capable paths unless equivalence is established, and add a regression comparing against unrewritten prepared algebra.

Verdict: Fix the path rewrite regression before merging.

… path endpoint

?s ex:p* ?o FILTER(?s = ex:missing) has no matches on a graph without
ex:missing, but the substituted ex:missing ex:p* ?o matches the zero-length
pair. Equality substitution now declines when the variable is an endpoint of
a path that can match zero steps.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
@jeswr

jeswr commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head c0339ac28dbd. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] BASE is unavailable during parallel expression evaluation — crates/sparq-engine/src/construct.rs:58
    The guard installs BASE only on the calling thread. With the default parallel feature and at least 50,000 WHERE solutions, extend_bindings evaluates BIND expressions on Rayon workers without propagating QUERY_BASE. For BASE <https://base.example/> CONSTRUCT { ?s <http://ex/q> ?r } WHERE { ?s ?p ?o BIND(IRI("rel") AS ?r) }, worker-evaluated rows leave ?r unbound and silently lose their template triples. DESCRIBE has the same problem.

    Propagate the scoped BASE into parallel expression workers, alongside the existing context snapshots, and add a regression above the parallel threshold.

  2. [medium] Nested nullable paths still permit incorrect equality substitution — crates/sparq-engine/src/rewrite.rs:292
    OneOrMore can match zero steps when its child can, so treating every OneOrMore as nonempty is incorrect. With algebra-rewrite enabled, graph ex:a ex:missing ex:b and query CONSTRUCT { ?s ex:r ?o } WHERE { ?s (ex:p?)+ ?o FILTER(?s = ex:missing) } (using prefix ex: <http://ex/>), the original evaluation returns nothing: ex:missing occurs only as a predicate. Substitution seeds the path with its dictionary ID, allowing the inner ? identity match and incorrectly constructing ex:missing ex:r ex:missing. Routing construct_or_describe through the rewrite introduces this regression.

    Handle OneOrMore(q) by recursively checking path_may_be_empty(q) and test nested nullable paths with predicate-only endpoints.

Verdict: Fix these correctness issues before merging.

…mpty

IRI()/URI() on rayon workers saw an empty QUERY_BASE, so above the parallel
threshold relative IRIs went unbound. Every parallel evaluation site now
snapshots the base and re-installs it per item, like NOW(). OneOrMore(q) can
match zero steps when q can, so equality substitution declines it too.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
@jeswr

jeswr commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 84417b3a30af. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR is safe to merge as is.

@jeswr

jeswr commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 15b32a215fc4. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Worker helpers break the WASM lint gate — crates/sparq-engine/src/exec.rs:15726
    query_base_snapshot and query_base_worker_install compile unconditionally, but all their callers require parallel. The WASM dependency disables that feature, leaving both helpers unused and triggering dead_code errors in the existing cargo clippy -p sparq-wasm --target wasm32-unknown-unknown --all-targets -- -D warnings gate.
    Gate both helpers with #[cfg(feature = "parallel")], or apply the conditional dead-code allowance used by the sibling worker helpers.

Verdict: Fix the no-parallel lint failure before merging.

…arallel feature

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1gMf8RJim1LzuKapZNpZR
@jeswr

jeswr commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 2f931bafa231. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR is safe to merge as is.

jeswr commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

docs-quality quick-gates fails in the step "Cache/artifact posture" (scripts/tests/test_mergequeue_cache_posture.py). The test flags that .github/workflows/notation3tests.yml:71 has a rust-cache step without save-if.

This failure is not from this PR. The check fails the same way on main at 126a92e, and this PR does not touch workflows. The fix is #6688, which adds the save-if line. I have not ported it here, because a push would reset this PR's queued ci-fast run and its Codex review of the current head. docs-quality is not a required check.


Generated by Claude Code

@jeswr
jeswr merged commit 758866f into main Oct 8, 2026
9 of 10 checks passed
@jeswr
jeswr deleted the fix/engine-batch-3 branch October 8, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

engine: CONSTRUCT/DESCRIBE bypass the algebra-rewrite seam on every surface

2 participants