feat: Add sh:sparql (SHACL-SPARQL) constraint support - #1717
Conversation
9a54aa2 to
0094358
Compare
0d2d698 to
1d3dd0e
Compare
SPARQL-based constraints (SHACL-SPARQL §5) now validate at transaction
staging time and via /validate. sh:select queries parse at shape-compile
time with the spec's pre-binding restrictions enforced (no MINUS, SERVICE,
VALUES, $this reassignment; sub-SELECTs must project $this), lower per
validation against the data snapshot, and pre-bind $this (and $PATH on
property shapes) via a VALUES row injected into every evaluation scope.
Solutions map to violations per spec: ?value (defaulting to the focus
node), ?path, ?message, sh:message {?var} templates, and
sh:SPARQLConstraintComponent. sh:prefixes declarations resolve through the
owl:imports closure; invalid queries surface as validation failures scoped
to the shapes that use them.
The W3C sparql/ test section is wired into testsuite-shacl as
shacl_sparql_w3c_testsuite: 17/22 pass, covering every sh:sparql test.
Remaining failures are SPARQL constraint components (sh:validator) and
optional $shapesGraph support, both documented as gaps. Core suite is
unchanged at 81/98.
The genesis (overlay-only) arm of range_with_overlay collected EVERY overlay flake for the graph — full clone + sort + stale-removal — on every range() call, then filtered down to the requested match. Against an unindexed all-novelty ledger (fluree validate <file>, memory-mode staging, testsuite runs) every point probe paid O(novelty log novelty), so per-focus-node loops like SHACL validation went quadratic: 1k subjects 0.82s, 4k 16.9s, 31.6k killed at 26 minutes — while the same corpus validated in 1.8s against an indexed ledger. The overlay interface already supports bounded walks (novelty partition-point-seeks its segments and prunes via zone maps); the collector just never passed bounds. Equality matches whose bound components form a prefix of the index order (SPOT s / s+p, PSOT/POST p) now derive min/max sentinel bounds and seek. The sentinels carry t = i64::MIN/MAX so every real flake sits strictly inside them, keeping the overlay's left-exclusive (first, rhs] contract lossless; the exact range filter still applies afterward, so non-prefix shapes and over-returning overlays stay correct. The bounded API's genesis arm prunes by its inclusive end bound the same way. The 31.6k-subject corpus now validates in ~22s in a DEBUG build (previously killed at 26 minutes in release); 1k→4k scales 0.15s→0.55s, linear. Regression test pins both properties: prefix probes reach the overlay bounded, and results match the unbounded walk.
Three residual O(data)-per-query costs made per-focus-node sh:sparql validation against an unindexed (all-novelty) ledger unusable — the 31.6k-subject corpus was killed after 10+ minutes in file mode while the identical shapes ran in 2.6s against an indexed ledger: 1. Stats rebuilt per execution. The stats-view cache lives on the binary store's leaflet cache, so store-less ledgers rebuilt the view — a full novelty walk in assemble_fast_stats — inside every prepare_execution (62% of the profile). A small process-global LRU now serves the store-less path, keyed by the existing cache key plus the overlay's content_version (the documented globally-unique content stamp); overlays without a version stamp keep the old rebuild-per-call behavior. 2. Bound-object probes unbounded. overlay_eq_bounds only covered subject- and predicate-led prefixes; per-row object-bound joins probe OPST/POST, which still walked everything. POST now seeks its p+o prefix and OPST its object prefix (OPST bounds stop at the object: cmp_object orders value then datatype before the predicate, so an o+p bound covers the same span as o alone). 3. Selectivity invisible. Novelty-assembled property stats carried ndv 0 unless an indexed entry provided it, so the planner's count/ndv_values bound-object estimate ranked a one-value predicate and a unique-key predicate identically and lowering order picked the join order — a uniqueness constraint probed the unselective pattern first, quadratic in the value-group size. The POST walk in assemble_fast_stats now accumulates live distinct values/subjects for exactly the predicates whose indexed entry has no ndv (every predicate on memory ledgers, only brand-new ones on indexed ledgers, which skip the tracking entirely). File-mode sh:sparql on the 31.6k corpus: killed at 10+ min before, 30s in a debug build after, findings intact. The 4k fixture: 111s to 1.2s with the query text untouched.
Adds opts.validationMode ("warn" / "reject") — TxnOpts::validation_mode —
so a single write can adjust how SHACL failures are handled without
changing the ledger's standing posture. Gating is asymmetric and runs at
the merge_shacl_opts choke point that already carried the override-control
scaffolding:
- Strengthening (config Warn, request Reject) is always honored.
- Softening (config Reject, request Warn) is granted only when the SHACL
group's f:overrideControl permits it for the request's verified
identity: f:OverrideAll (the default) permits everyone, f:OverrideNone
pins the configured posture, and an identity-restricted list checks
membership. A denied request keeps the configured mode with a warning
log rather than failing the transaction.
- The request can never toggle f:shaclEnabled.
The gate identity decodes from the staged policy context — built by the
server from the verified bearer / credential DID — never from the
user-settable opts.identity. On the no-config shapes-exist heuristic path
there is no override control to consult, so the group default governs and
a requested warn softens the heuristic's reject.
Turtle inserts and commit replay have no request surface and always run
the configured posture (replay re-validates what the leader already
staged, so an authoring-time softening decision is not re-litigated).
Use case: a remediation agent whose corrective writes transiently violate
shapes (e.g. merging duplicate-candidate parts flagged by a sh:sparql
uniqueness constraint) softens exactly its own writes — under
IdentityRestricted, only its writes — while every other writer stays
rejected.
…d-shape reuse Cross-ledger f:shapesSource was enforced only on the JSON-LD staging path; the direct-flake Turtle insert path and the validate surfaces (CLI ledger mode, HTTP endpoint, Fluree::validate_ledger) rejected the config with 'not yet supported'. Both now resolve the wire through the shared governance resolver and enforce M's shapes, including sh:class value-set membership against M. The shapes wire whitelist now mirrors the ShapeCompiler predicate scan: sh:node, qualified-value constraints, sh:deactivated, and the sh:sparql family (sh:select, sh:prefixes/sh:declare/sh:prefix/sh:namespace plus the owl:imports closure), so SPARQL-based constraints survive the wire. Compiled-shape reuse now covers cross-ledger sources: the compile cache keys on the wire origin (model ledger, graph, resolved_t) alongside the local epochs, so while M's head is unchanged a transaction skips wire translation, sh:sparql parsing, and shape compilation entirely — the steady-state cost is the single nameservice head lookup that produces resolved_t. Reuse requires an empty staged-namespace delta, since new namespaces can change which of M's shapes translate. Commit replay (graph-sync push) now skips SHACL re-validation for cross-ledger sources instead of failing the push: the origin validated against M when the commit was authored, and re-resolving M at replay time could see a different head.
…stry sh:sparql constraint queries lowered against the data snapshot's namespace registry, so a constraint over a namespace introduced by the in-flight transaction encoded to a never-matching Sid and silently no-oped — the very first write minting a namespace passed unconstrained. NamespaceRegistry now implements the query layer's IriEncoder (same contract as LedgerSnapshot: unknown namespaces fall back to the never-matching EMPTY-namespace Sid, lookup-only, no allocation), and the staged registry threads from StagedShaclContext through validate_view_with_shacl into the sh:sparql lowering. Write paths now enforce constraints against staged data in first-mint namespaces; commit replay keeps snapshot lowering. Constraints over vocabulary the ledger has never seen anywhere remain deliberately inert — they lower to terms that match no data and yield no rows, never an error — so shapes can ship rules for classes and predicates the data doesn't use yet (pinned by sh_sparql_over_unknown_vocabulary_is_inert).
a6b6f1e to
788a4b1
Compare
aaj3f
left a comment
There was a problem hiding this comment.
This is a strong PR @bplatz and it seems like the crux of the work (and its hard parts) are done well w/ almost no notes from me.: the pre-binding restrictions are enforced over the parsed AST rather than by text-matching, the parse-at-compile / lower-at-validate split is the right seam, unresolvable constraints are scoped to the shapes that use them instead of poisoning the shape set, and the range-seek sentinels are correct against all four comparators in a way that would have been easy to do poorly.
Two things I'd consider looking at before merge:
(1) tx.rs:1269 — on a ledger with no config graph, opts.validationMode: "warn" softens SHACL rejection with no identity and no override control, because that branch never reaches merge_shacl_opts. Every ledger that has shapes but no #config graph goes from unconditionally enforcing to advisory-on-request for anyone who can write. #1720 correctly identifies the AllowAll default problem, but its fix doesn't reach this line — this branch consults no OverrideControl at all. It's also the one branch of the new feature with no test. Perhaps this is the design posture and I'm reading the code + intention wrong, but it seems very possible that this is not the desired behavior.
(2) validate.rs:595 — the validate path builds every GraphDbRef with GraphDbRef::new(...), so tracker is None, and ContextConfig::default() leaves cancellation: None. A sh:sparql constraint therefore runs on /validate with no fuel, no timeout, no memory ceiling, and no row cap — once per focus node, across the whole ledger. Pre-binding $this doesn't bound the query body.
Beyond those: the novelty-derived-ndv change (runtime_stats.rs:287) adds a per-flake FlakeValue clone and hash to assemble_fast_stats with no test and no bench, while its two sibling perf changes are both properly pinned; and the store-less stats cache adds 54 lines with zero new tests, leaving its invalidation-on-content_version behavior unheld even though class_coverage_trustworthy rides on it. I'd recommend closing out both, and I'd recommend at least a sentence in the security doc stating that shape-write is effectively root-read now that constraint queries execute unfiltered and echo bound values into reports.
Adherence checklist:
- Performance — net positive for the targeted workload; two of three perf pieces verified sound and one (ndv accumulation) carries an unmeasured per-flake allocation. Not neutral, not a regression I can demonstrate — but unpinned.
- Correctness-preserving fallback — honored. Non-prefix and non-
Eqrange shapes take the unbounded walk; unknown vocabulary lowers to a never-matching Sid and yields no rows rather than erroring; a broken config read during validate degrades to defaults. - SPARQL ↔ JSON-LD IR parity — not applicable in the usual direction (this is SHACL consuming the SPARQL front end, not a new query-language feature), and the shared
IriEncoderextension atencode.rs:32is additive. No parity obligation triggered. - Shared abstractions — extended rather than reinvented:
IriEncoder,GraphDbRef, the existingmerge_*_optschoke point, the existing compile-cache slot,ShaclErrorgaining athiserrorvariant rather than reaching foranyhow. No parallel dispatch introduced.
CI note for whoever merges: this is stacked on fix/raft-noop-republish, so ci.yml (which is pull_request: branches: [main]) never ran — the only green check on the head is the Release workflow's plan job with everything else skipped. Please retarget to main and let real CI plus both W3C suites run before merging. I ran fmt, clippy -D warnings (default features, --all-targets), and nextest for the six touched crates locally: fmt clean, clippy clean, 2754 + 1032 tests passing with all 12 new tests seen by name. The --all-features clippy leg didn't run here — cxx v1.0.194 won't build on my machine ('algorithm' file not found) — so that leg is genuinely unexercised for this change.
One thing I'd like written down rather than fixed.
sh:sparql is the first SHACL construct whose reach isn't structurally bounded to the focus node. Every existing constraint can only report values reachable from $this; a sh:sparql body can read anywhere in the graph, and sh:message {?var} templates render arbitrary bound values into the report (sparql.rs:522-548, render_value at sparql.rs:321) — which reaches the writer as the ShaclViolation error string and the /validate caller as sh:value.
SHACL validation runs with policy_enforcer: None, which is correct and pre-existing — validation has to see everything to be sound. But that choice becomes load-bearing in a new way here: on a default_allow ledger with a view policy, whoever can write a shape can now read what the policy hides, by writing a constraint that binds the hidden value and echoing it through a violation message. Whether that's a real escalation depends entirely on whether writing to the shapes graph is a higher bar than reading the data, which is a per-deployment question.
I'm not asking for a policy gate on constraint queries — I think that would be wrong, and would make validation unsound. What I'd like is a paragraph in docs/security/cross-ledger-policy.md (the file this PR already touches for the cost story) saying plainly: installing a shape is equivalent to root read on the ledger, because a sh:sparql constraint executes unfiltered and its bound values reach the report. Operators can then size the shapes-graph write permission accordingly. Right now that's true and undocumented, which is the combination that bites.
5. .github/workflows/ci.yml — nothing runs testsuite-shacl, so the compliance numbers in the PR body aren't held by anything
grep -rn "shacl" .github/workflows/*.yml returns zero hits. testsuite-sparql has a job at ci.yml:88 that runs fmt, clippy, the full W3C suite, and check_testsuite (which fails when a registered skip now passes, so the register can only shrink). testsuite-shacl has none of that: it's report-only unless SHACL_STRICT=1, and there's no register and no stale-register guard at all.
That's not something this PR introduced. But this PR is the one that makes the SHACL suite carry a load-bearing claim — "17/22 pass, core unchanged at 81/98" is now the evidence for a spec-conformance feature, and today nothing stops a later change from silently taking it to 0/22. We may want a testsuite-shacl job mirroring the sparql one, with SHACL_STRICT=1 against a checked-in expected-failure list. If that's too much for this PR I'd understand — but I'd rather it land as a decision than as an omission.
(I could not run either W3C suite myself: testsuite-shacl/data-shapes is a submodule and is absent from a bare review worktree. So the 17/22 and 81/98 figures are unverified by me.)
| // governs and a transaction-requested warn mode softens the heuristic's | ||
| // reject posture. Config-present paths never reach this branch with an | ||
| // unhonored request — the gate already ran inside `merge_shacl_opts`. | ||
| let heuristic_softened = !has_config |
There was a problem hiding this comment.
A transaction-requested warn softens the shapes-exist heuristic with no identity check and no override control consulted.
The branch is:
let heuristic_softened = !has_config
&& ctx.requested_validation_mode
== Some(fluree_db_core::ledger_config::ValidationMode::Warn);and when it fires, every reject violation is moved into the warn bucket and the transaction commits. merge_shacl_opts — the choke point that carries the whole permits_override gate — is never reached on this path, because there is no f:shaclDefaults group to read an f:overrideControl off of.
Consequence. A ledger that has SHACL shapes but no #config graph is exactly the back-compat default (the code one screen up calls it that: "No config + no shapes → skip (backward compat: shapes-exist heuristic)"). Before this PR those shapes were unconditionally enforcing. After it, any client that can write can append "opts": {"validationMode": "warn"} and commit shape-violating data, with the violations reduced to a tracing::warn! line nobody is reading. There is no identity involved, so this holds for an anonymous writer on an unauthenticated dev stack and for an authenticated non-privileged writer alike.
I want to be careful to separate this from the thing you already filed. #1720 is about the AllowAll default on f:overrideControl, and it's a good issue — but its proposed fix (per-group defaults, OverrideNone for SHACL softening) would not reach this line, because this line never consults OverrideControl at all. Changing the default fixes the config-present case and leaves the config-absent case exactly as open as it is now. So this one belongs to this PR rather than to #1720.
Fix. Make the no-config path fail closed: drop heuristic_softened and let the heuristic's reject posture stand regardless of requested_validation_mode, so softening requires an operator to have written a config group and (per #1720) explicitly permitted it. If you'd rather keep the affordance, the equivalent is to route the heuristic through merge_shacl_opts with a synthesized group whose override_control is OverrideNone — that keeps one gate rather than two policies.
Either way this branch needs a test. All three of shacl_txn_validation_mode_* seed a config graph via seed_shacl_mode_ledger, so the ungated path is the one branch of the new feature with no coverage at all — I checked, heuristic_softened appears nowhere under fluree-db-api/tests/ or src/shacl_tests.rs.
There was a problem hiding this comment.
Addressed in 6b4c211. heuristic_softened is gone — the no-config heuristic keeps its reject posture regardless of requested_validation_mode, so softening requires an operator to have written a config group, which runs the permits_override gate inside merge_shacl_opts.
You read it correctly, and the identity point is sharper than it looks: the comment four lines above the mode read insists identity comes from a "bearer / credential DID — never from user-settable opts directly," and this branch consulted none at all.
New test shacl_heuristic_without_config_ignores_requested_warn_mode. Mutation-checked: restoring the branch lets the violating write commit at t=2. I've also noted on #1720 that its fix doesn't reach this path.
| @@ -487,8 +593,12 @@ pub async fn validate_view( | |||
| } | |||
|
|
|||
| let data_db = GraphDbRef::new(snapshot, data_g_id, novelty, to_t); | |||
There was a problem hiding this comment.
Every GraphDbRef on the validate path is built with GraphDbRef::new(...), which leaves tracker: None, so the constraint query's only bound is removed.
The chain: validate_view_inner builds data_db with GraphDbRef::new(snapshot, data_g_id, novelty, to_t) and hands it to validate_all_with_membership. That db reaches validate_sparql_constraint, which builds its execution config as:
ContextConfig {
tracker: db.tracker, // None on this path
..Default::default() // cancellation: None
}(fluree-db-shacl/src/sparql.rs:481-486). tracker: None means no fuel is charged — the per-flake charge documented on GraphDbRef::tracker and #1681's row-drain pricing both no-op. cancellation: None matters twice over: it's the cooperative-cancellation handle and the carrier for the per-query memory ceiling, which execute_prepared_into only installs inside if let Some(cancellation) = config.cancellation (fluree-db-query/src/execute/runner.rs:831-845). And nothing caps rows — execute collects all batches, then every solution row allocates a ValidationResult.
Consequence. GET /validate/*ledger (fluree-db-server/src/routes/mod.rs:334) runs validate_all_with_membership, which is one constraint query per focus node across the whole ledger. Pre-binding $this does not bound the query body — a shape carrying SELECT $this ?a ?b WHERE { $this ?p ?o . ?x ?y ?a . ?z ?w ?b } is a cartesian product per focus node, with no fuel to exhaust, no deadline to hit, and no memory ceiling to trip. One request pins a core until the process dies. The staging path is better off (the tracker propagates through stage.rs:3496 from tracker_for_limits(txn_json)), but note that even there the fuel limit is set by the requesting transaction, so a shape that taxes every future write is bounded only by the victim's own max_fuel.
Fix. Attach a tracker on the validate path — validate_view_inner has the LedgerView and can build one from server-supplied tracking options the same way the transact route does — and give the constraint query a QueryCancellation so the memory checkpoint arms. A row cap on solutions per constraint per focus node would also be worth having; a constraint producing a million violations for one focus node is a broken shape, not a report anyone wants.
There was a problem hiding this comment.
Addressed in 6b4c211, with one deliberate omission.
ValidateOptions now carries max_fuel and a cancellation; validate_view_inner attaches the tracker to the data db, and ShaclEngine::with_cancellation threads the handle through ClassMembershipCtx — which already flows exactly where constraint queries run — into the ContextConfig. The route takes the same request-scoped control /query uses via current_query_execution_options; note exec_options has to stay alive across the call, since dropping it aborts the timeout task through QueryTimeoutGuard. The CLI keeps the unbounded posture (..Default::default()): operator-run, no request scope.
I did not add the row cap. Silently truncating a validation report trades an availability bug for a correctness one — a caller can't tell a capped report from a conforming tail. The memory ceiling errors instead of truncating, which seems like the better failure mode for the same threat. Happy to revisit if you disagree.
New test validate_sparql_constraint_is_bounded_by_fuel; mutation-checked by dropping the with_tracker call.
|
|
||
| let sid_key = (flake.p.namespace_code, flake.p.name.to_string()); | ||
| *property_counts.entry(sid_key.clone()).or_insert(0) += delta; | ||
| if !indexed_ndv.contains(&sid_key) { |
There was a problem hiding this comment.
The accumulation is:
if !indexed_ndv.contains(&sid_key) {
let acc = ndv_acc.entry(sid_key.clone()).or_default();
*acc.0.entry((flake.o.clone(), flake.dt.clone())).or_insert(0) += delta;
*acc.1.entry(flake.s.clone()).or_insert(0) += delta;
}Per flake that's a second (u16, String) clone on top of the one property_counts already takes, a full FlakeValue clone (a heap String for every string-valued flake), a Sid clone, and a hash of the whole FlakeValue. The map then holds an entry per distinct object value in novelty. This is inside assemble_fast_stats, which every prepare_execution depends on.
The commit message frames the guard as "only brand-new predicates on indexed ledgers, which skip the tracking entirely," and in the steady state that's right. But indexed_ndv is built from p.ndv_values > 0 || p.ndv_subjects > 0, which is a proxy for "indexed" rather than the thing itself — a predicate whose indexed entry carries zero ndv (an index generation from before ndv recording, or one that legitimately recorded 0) falls into full tracking on a large indexed ledger. I may be wrong about how common that is in practice, and you'd know better than me whether any deployed index generation lacks ndv.
The part I'm more confident about: this is a hot-path allocation change with no test in runtime_stats.rs and no bench that would move on it — query_hot_bsbm and query_overlay_matrix are the two per-PR advisory benches and neither runs a store-less ledger. Given how carefully the other two perf pieces are pinned (the range seek has a RecordingOverlay test that goes red when you remove the seek — I checked), this one stands out as the unpinned third. A unit test asserting the derived ndv on a small novelty, plus either a bench or a documented cap on accumulator size, would bring it up to the same bar as its siblings.
There was a problem hiding this comment.
Not addressed in this PR — deferring rather than dismissing, and I think you've split it correctly into three separable things.
The allocation is real and I'm not disputing it: per flake it's flake.o.clone() + flake.dt.clone() + flake.s.clone() plus a second sid_key.clone(), in a function every prepare_execution depends on.
On the indexed_ndv proxy — you're right that ndv_values > 0 || ndv_subjects > 0 is a proxy for "indexed" rather than the thing itself, and neither of us can answer from the code whether a deployed index generation records zero ndv. That's a question for whoever owns the index format, and I'd rather it get answered than guessed at under review pressure.
The piece I'd want regardless of how that resolves is the accumulator bound — an entry per distinct object value in novelty, uncapped. Filing all three (unit test, cap, ndv-generation question) as a follow-up issue so it doesn't ride on the answer to the proxy question.
| // globally-unique content stamp — since `epoch()` alone is only unique | ||
| // within one overlay instance's lifetime. No version stamp → no caching | ||
| // (identical to before). | ||
| if let Some(version) = db.overlay.content_version() { |
There was a problem hiding this comment.
The two tests in mod tests (uncached_builder_still_merges_novelty_without_store, semantic_elision_vouch_gates_class_coverage_trust) are both pre-existing — the diff adds 54 lines of cache and zero lines of test. uncached_builder_still_merges_novelty_without_store now happens to route through the new cache, but it calls once and asserts a count, so it would pass identically with the cache ripped out.
What isn't pinned is the pair of properties that make the cache safe: a second call at the same content_version hits, and a bumped content_version misses. That matters more than a normal plan cache would, because class_coverage_trustworthy rides on the cached StatsView and gates semantic elision of rdf:type filters — a correctness lever, not just a plan lever. It looks right to me as written (allow_semantic_elision is folded into cache_key, and I confirmed semantic_elision_vouch_gates_class_coverage_trust would catch its removal), but the invalidation half has nothing holding it.
Two small #[test]s would cover it. Minor and non-blocking — but if you agree it's right, I'd rather see it folded in now than lost in the backlog.
Separately, and this genuinely is just a question: CAPACITY = 8 with a process-global Mutex<Vec<Slot>>. A process holding more than 8 concurrently-active memory-mode ledgers thrashes back to rebuild-per-call plus a global lock round trip on every prepare. The build correctly happens outside the lock so it's a small constant rather than a cliff, and I suspect ">8 concurrent memory ledgers" is outside the shapes we care about — but it's an assumption about deployment rather than about the code, so I wanted to name it.
There was a problem hiding this comment.
Addressed in 6b4c211. Two tests: storeless_stats_view_is_reused_at_the_same_content_version (Arc::ptr_eq on a second call) and storeless_stats_view_misses_when_content_version_diverges.
The miss test uses divergent clones rather than a bumped overlay, because cache_key already folds in epoch() — so a plain mutation changes the key and never exercises the content_version guard at all. Divergent clones are the case the guard actually exists for, and content_version_is_globally_unique_across_divergent_clones (novelty/lib.rs:1719) pins that they collide on epoch. The test asserts that collision as an explicit precondition, then that each clone sees its own stats. Keying on cache_key alone fails it with left: 1, right: 2 — one clone served the other's stats.
On CAPACITY = 8: agreed it's an assumption about deployment, and I think it's the right one to hold for now rather than tune blind. The degradation is a rebuild plus one uncontended lock round-trip per prepare, which is exactly the pre-cache cost — so >8 concurrent memory ledgers regresses to today's behavior rather than falling off a cliff, and the build staying outside the lock is what keeps that true. Left as-is deliberately.
| } => Some(s.clone()), | ||
| _ => None, | ||
| }); | ||
| let message = bound_message |
There was a problem hiding this comment.
constraint.messages.first() picks one message and drops the rest. Per §5.3.1 a sh:SPARQLConstraint with multiple sh:message values should populate sh:resultMessage with all of them (the multi-language case is the obvious one — an @en and an @es message on the same constraint). Very minor, and possibly deliberate given the report shape only carries one string today.
There was a problem hiding this comment.
Not addressed — your own hedge is the reason. ValidationResult.message is a single String, so honoring multiple sh:message per §5.3.1 means changing the report shape and every surface that renders it (the ShaclViolation error text, ReportResult, the JSON report). That's a bigger change than belongs in this PR, and the multi-language case deserves to be designed rather than bolted on.
Filing it as an issue with the §5.3.1 reference.
| messages: Vec<String>, | ||
| deactivated: bool, | ||
| ) -> SparqlConstraint { | ||
| let uses_path = select.contains("$PATH") || select.contains("?PATH"); |
There was a problem hiding this comment.
select.contains("$PATH") || select.contains("?PATH") will fire on a $PATH occurring inside a string literal, which then makes the constraint hard-error on a node shape ("$PATH is only supported … on property shapes", sparql.rs:465) rather than running. The same is true of the reserved-variable scan at sparql.rs:127, though there the doc comment already acknowledges it and the failure direction is safe (reject a weird query). For uses_path the failure direction is less safe: a valid node-shape constraint that happens to mention the string is turned into a validation failure. Since you already have the parsed AST by then, reading the variable set off it would be exact. I know this seems pedantic — it's the kind of thing that surfaces once, in someone's regex-matching constraint, a year from now.
There was a problem hiding this comment.
Addressed in 6b4c211. Not pedantic at all — the failure direction is what makes it worth fixing.
The lowered VarRegistry already holds the exact variable set, and VarRegistry::get is non-inserting, so the decision is now vars.get("?PATH").is_some() after lowering and the uses_path field is gone from SparqlConstraint entirely rather than being fixed in place.
New test shacl_sparql_node_constraint_with_path_in_a_literal — a node shape filtering on the literal string "$PATH". Restoring the text scan fails it with exactly the error you predicted: "$PATH is only supported in sh:sparql constraints on property shapes with a plain predicate path".
…sparql Review findings from PR #1717. **The shapes-exist heuristic no longer honors a requested warn mode.** On a ledger with shapes but no config graph — the back-compat default — `opts.validationMode: "warn"` moved every reject violation into the warn bucket and committed. That path never reaches `merge_shacl_opts`, which carries the whole `permits_override` gate, so the softening consulted no `f:overrideControl` and no identity: any writer could downgrade enforcement to a log line. Softening now requires an operator to have written a config group that permits it. **`sh:sparql` constraint queries are bounded on the validate path.** Every other SHACL constraint can only read what is reachable from the focus node; a `sh:sparql` body can walk anywhere, once per focus node. `GraphDbRef::new` leaves `tracker: None` and `ContextConfig::default()` leaves `cancellation: None`, so `/validate` ran constraints with no fuel, no deadline, and — since the per-query memory ceiling is installed only when a cancellation is present — no memory ceiling. `ValidateOptions` now carries `max_fuel` and a cancellation; the route takes the same request-scoped execution control the query endpoints use, and `ShaclEngine::with_cancellation` threads the handle to the constraint query. The CLI keeps the unbounded posture: operator-run, no request scope. **`$PATH` use is read off the lowered variable set, not the query text.** `select.contains("$PATH")` fired on a `$PATH` inside a string literal, turning a valid node-shape constraint into a hard validation failure. The lowered `VarRegistry` is exact, so `uses_path` is gone entirely. **Store-less stats cache invalidation is pinned.** Two tests for the properties that make it safe: a second call at the same `content_version` hits, and divergent clones — which collide on `epoch` — do not share a view. `class_coverage_trustworthy` rides on this. Separately, `ledger_exists_on_file_storage` asserted the pre-43d758610 semantics and was failing on main: that commit deliberately made a retracted ledger read as absent on the query path. The test now pins both halves — `exists` is false, and the soft-dropped record survives carrying the flag. All four new tests mutation-checked: each fails against the code it pins.
SHACL validation runs with `policy_enforcer: None`, which is correct and pre-existing — a policy-filtered view would make validation unsound, since data hidden from the validator would silently conform. `sh:sparql` makes that choice load-bearing in a new way. Every other SHACL constraint can only report values reachable from the focus node; a `sh:sparql` body reads anywhere in the graph, and `sh:message` templates interpolate bound variables into the result, which reaches a writer as the `ShaclViolation` text and a `/validate` caller as `sh:resultMessage` and `sh:value`. On a policy-restricted ledger, whoever can write the shapes graph can therefore read what the view policy hides. Not a behavior change and deliberately not gated — a policy gate on constraint queries would break soundness. Written down so operators can size shapes-graph write permission against unfiltered read instead of treating it as schema authoring. Also records the two bounds that are off by default: `/validate`'s `maxFuel`, and the config-group requirement for a transaction-requested warn mode.
Review follow-upTwo commits: 6b4c211 (the four code items) and b70c413 (the security doc). What the blocking items turned into
All four new tests mutation-checked: each fails against the code it pins. The doc you asked forb70c413 adds "Installing a shape is equivalent to root read" to One thing you couldn't have seen
CINow green on all six real jobs — Still open, as decisions rather than omissions
|
Contents (five pieces, in commit order):
feat:sh:sparql(SHACL-SPARQL §5) constraints — the main feature, detailed below.perf: file-mode / memory-ledger validation & planning fixes — overlay range seeks, store-less stats caching, novelty-derived ndv (found while validating the feature at corpus scale; sections below).feat: per-transaction SHACL validation mode (opts.validationMode) underf:overrideControlgating.feat: cross-ledgerf:shapesSourceon the remaining surfaces + compiled-shape reuse — Turtle insert and validate now enforce it, replay skips it deliberately, and the sh:sparql family survives the wire.fix: sh:sparql lowering against the staged namespace registry — constraints now see data from the very transaction that first introduces a namespace.What
Implements SPARQL-based constraints (SHACL-SPARQL §5):
sh:sparqlon node and property shapes now validates at transaction staging time (against the staged view, so constraints see the transaction's writes exactly as they would commit) and through/validate/fluree validate.fluree-db-shacl/src/sparql.rs, new):sh:SPARQLConstraintnodes are collected (sh:select,sh:message,sh:deactivated,sh:prefixes→sh:declareresolved through theowl:importsclosure into a PREFIX header). Queries parse once at shape-compile time with the spec's pre-binding restrictions (Appendix B) enforced: noMINUS/SERVICE/VALUES, no reassignment of$this, sub-SELECTs must explicitly project$this(SELECT *rejected). An invalid query compiles into a deferred error that surfaces as a validation failure only for shapes that use it — same scoping as unresolvablesh:path.$this— and$PATHon property shapes with a plain predicate path — are pre-bound by injecting a one-row VALUES into every evaluation scope (top level, union branches, optionals, GRAPH bodies, sub-selects), the standard implementation of the spec's solution-mapping semantics. Execution reuses the query engine (fluree_db_query::execute) and charges the caller's fuel tracker.sh:focusNode=$this;sh:value=?valuebinding, defaulting to the focus node;sh:resultPath=?pathwhen an IRI, else the property shape's path;sh:resultMessage=?message, elsesh:messagewith{?var}/{$var}template substitution;sh:sourceConstraintComponent=sh:SPARQLConstraintComponent.New
fluree-db-shacl → fluree-db-sparqldependency (parser only; no cycle).Testing
sparql/suite newly wired intotestsuite-shaclasshacl_sparql_w3c_testsuite: 17/22, everysh:sparqltest passes (node 4/4, property 1/1, pre-binding 12/14). The 5 failures are SPARQL constraint components (sh:validator) and optional$shapesGraphsupport — both documented gaps, out of scope here.fluree-db-api(staging-time rejection,$PATH+ message templating, fail-closed invalid query); full SHACL module 82/82.--all-features --all-targetsclean on touched crates.Docs
cookbook-shacl.mdgains ansh:sparqlsection;shacl-compliance.mdgaps updated; crate-map notes the new dependency; testsuite Makefile covers both suite sections.Also bundled: overlay-only range seek (
perf(core))Investigating slow
fluree validate <file>runs surfaced that the genesis (overlay-only) arm ofrange_with_overlaycollected every overlay flake — clone + sort + stale-removal — on everyrange()call before filtering, making per-focus-node probe loops quadratic against unindexed all-novelty ledgers: 1k subjects 0.82s, 4k 16.9s, 31.6k killed at 26 minutes (the same corpus validated in 1.8s against an indexed ledger).Equality probes whose bound components form a prefix of the index order (SPOT
s/s+p, PSOT/POSTp) now derive min/max sentinel bounds and hand them to the overlay, which already knew how to partition-point-seek its segments — the bounds just were never passed. The exact range filter still runs afterward, so non-prefix shapes and over-returning overlays stay correct.After: the 31.6k-subject corpus validates in ~22s in a debug build; 1k→4k scales 0.15s→0.55s (linear). Benefits every read against big-novelty memory ledgers (file-mode validate, memory-mode staging SHACL, testsuite runs), not just validation. Regression test pins that prefix probes reach the overlay bounded and results match the unbounded walk.
Follow-up (
perf(query)): file-mode sh:sparql residualRetest surfaced that sh:sparql in file mode still scanned (31.6k pre-bound queries, killed at 10+ min while ledger mode ran 2.6s). Three compounding causes, all fixed:
prepare_execution(62% of the profile). A small process-global LRU now serves the store-less path, keyed by the existing key + the overlay'scontent_version.p+oprefix and OPST its object prefix.ndv 0, so the planner'scount/ndv_valuesestimate ranked a one-value predicate and a unique-key predicate identically and join order fell to lowering order — quadratic in the value-group size for uniqueness constraints.assemble_fast_statsnow accumulates live distinct values/subjects for exactly the predicates whose indexed entry has no ndv (indexed ledgers skip the tracking).After: the 31.6k corpus + uniqueness sh:sparql shape completes in ~30s in a debug build (killed at 10+ min before), findings intact; the 4k fixture went 111s → 1.2s with the query text untouched. Regression coverage: both W3C suites unchanged, 349 SPARQL + 422 query integration tests, novelty/query/core/api suites all green.
Added (
feat(shacl)): per-transaction validation mode under override controlopts.validationMode("warn"/"reject",TxnOpts::validation_mode) lets a single write adjust SHACL failure handling without changing the graph's standing posture — the remediation-agent use case. Gating is asymmetric at the existingmerge_shacl_optschoke point: strengthening is always honored; softening requires the SHACL group'sf:overrideControlto permit the request's verified identity (bearer/credential DID via the policy context, never user-settable opts).f:OverrideAll(default) permits everyone,f:OverrideNonepins the posture, identity-restricted lists limit softening to named agents. The request never togglesf:shaclEnabled; Turtle/replay paths always run the configured posture. Covered by unit tests (identity matrix) + three integration tests; docs updated (cookbook, setting-groups, override-control).Added (
feat(shacl)): cross-ledgerf:shapesSourceon every surface + compiled-shape reuseCross-ledger shapes (
f:shapesSourcewithf:ledger) were enforced only on the JSON-LD staging path — the direct-flake Turtle insert path and the validate surfaces (CLI ledger mode, HTTP endpoint,Fluree::validate_ledger) rejected the config outright with "not yet supported", and commit replay would have failed the push. Now:sh:classvalue-set membership against M (shared helperopen_cross_ledger_shapes_model).StagedShaclContext::origin_validated_replay): the origin already validated against M when the commit was authored, and re-resolving M at replay time could see a different head.ShapeCompilerpredicate scan — previouslysh:node, the qualified-value constraints,sh:deactivated, and the entire sh:sparql family (sh:select,sh:prefixes/sh:declare/sh:prefix/sh:namespace,owl:importsclosure) were silently dropped in transit.model,graph,resolved_t), so while M's head is unchanged a transaction skips wire translation, sh:sparql parsing, and shape compilation entirely — steady-state cost is the single nameservice head lookup that producesresolved_t. A commit on M invalidates on the very next transaction. Reuse requires the transaction introduced no namespaces (a namespace delta can change which of M's shapes translate).5 new integration tests (Turtle rejection, validate report, sh:sparql over the wire, head-advance invalidation via
sh:deactivated, inert unknown vocabulary); design doc's stale "Reserved" entry corrected;cross-ledger-policy.mdgains the enforcement-surface matrix.Fixed (
fix(shacl)): sh:sparql lowering against the staged namespace registrysh:sparql queries lowered against the data snapshot's namespace registry, so a constraint over a namespace introduced by the in-flight transaction encoded to a never-matching Sid and silently no-oped — the very first write minting a namespace passed unconstrained.
NamespaceRegistrynow implements the query layer'sIriEncoder(same contract asLedgerSnapshot: unknown namespaces fall back to the never-matching EMPTY-namespace Sid, lookup-only), and the staged registry threads fromStagedShaclContextthroughvalidate_view_with_shaclinto the lowering. Write paths are now exactly consistent: a query term matches iff the data — committed or staged — exists.Constraints over vocabulary the ledger has never seen anywhere remain deliberately inert (no rows, never an error), so model ledgers can ship rules for classes and predicates the data doesn't use yet; pinned by
sh_sparql_over_unknown_vocabulary_is_inert.