Fix SPARQL path+ (OneOrMore) transitive closure: project each depth branch to its endpoints

[teipsum]

Fixes #369.

What

SPARQL path+ (OneOrMore) did not return a transitive closure for variable
endpoints — it returned only the direct neighbours, with the true multi-hop
endpoint replaced by the first intermediate hop, plus duplicate rows that
DISTINCT failed to collapse. The five sibling operators (/ | ^ * ?) were
already correct.

This was a single missing endpoint projection in the OneOrMore translation.

Root cause

crates/grafeo-engine/src/query/translators/sparql.rs

translate_one_or_more_path pushed each fixed-depth branch raw into the
Union, whereas translate_zero_or_more_path and translate_zero_or_one_path
wrap every depth branch in project_path_endpoints first. The projection strips
the per-hop intermediate columns so all union branches share the endpoint
schema.

Without it, a depth-N branch is wider than the depth-1 branch (one extra column
per intermediate hop). The union output schema is seeded from branch 0 (the
narrow depth-1 branch), and Distinct keys on each chunk's own width while
copying into the narrow branch-0-sized builder — so the visible object column
ends up holding the first intermediate hop instead of the true endpoint, and
endpoints reached at different depths keep distinct (wider) keys and are never
de-duplicated.

The fix

One call site — wrap each depth branch exactly as the sibling operators do:

 for depth in 1..=MAX_DEPTH {
     let branch =
         self.translate_fixed_depth_path(inner_path, &subject, &object, &graph, depth)?;
-    branches.push(branch);
+    branches.push(self.project_path_endpoints(&subject, &object, branch));
 }

No reflexive 0-hop branch is added: path+ excludes the zero-length path
(SPARQL 1.1 §9.1). project_path_endpoints is the existing, unchanged helper;
this only adds a third caller and touches no shared state, so the five working
operators are textually unaffected.

Tests (result-level oracles)

The existing tests/spec/rdf/sparql/property_paths.gtest + cases asserted only
a row count (or single-hop data), which passed even with the bug present —
on the chain graph the buggy output has the right cardinality but every value is
the leaked intermediate hop. This PR:

• Strengthens the two count-only + cases (one_or_more_path_chain,
  sequence_with_one_or_more) to assert the actual reached values.
• Adds a result-level closure oracle set over a->b->c->d->e + c->x
  (predicate p) that asserts the full distinct closure as a sorted multiset,
  so it fails on both missing transitive pairs and duplicate leakage:
  • open-ended ?s :p+ ?o (full 13-pair closure; SELECT DISTINCT count = 13),
  • bound-subject :a :p+ ?o = {b,c,d,e,x},
  • a :p* control on the same graph (regression guard for *),
  • ASK over a 3-hop fully-bound + path,
  • a cycle a <-> b (4 distinct pairs; proves termination + de-duplication),
  • nested ^(:p+) and (:p+)/:q,
  • ?, /, ^ regression guards over the same graph.

Pre-fix these new/strengthened oracles fail with the leaked output (e.g. the
cycle yields 100 rows; SELECT DISTINCT open-ended yields 5 instead of 13);
post-fix all pass. The rest of property_paths.gtest and the SPARQL suites stay
green, demonstrating / | ^ * ? are unchanged.

Run them with:

cargo test -p grafeo-spec-tests --features rdf rdf_sparql_property_paths

Scope / honesty

• Bounded depth, not unbounded ALP. Like path*, path+ expands to a
  bounded MAX_DEPTH = 50 hops plus Distinct (no visited-set), so this is not
  the unbounded ALP of SPARQL 1.1 §18.4; closures over 50 hops truncate. Shared
  with path*, orthogonal to this fix — this PR does not claim spec-complete
  ALP conformance.
• Related, not fixed here. A fully-bound SELECT ?s WHERE { :a :p+ :b }
  (projecting a variable absent from the pattern) errors with GRAFEO-X001: Internal error: Variable 's' not found in input columns. This is identical
  for :p* and :p? (shared with the working operators), so it is a separate
  normalization item left for a follow-up. ASK and SELECT * over
  fully-bound paths already work.

---

Summary by cubic

Fixes SPARQL path+ so it returns the true transitive closure for variable endpoints and removes duplicate leakage. Each fixed-depth branch is now projected to its endpoints before union, matching the behavior of * and ?.

• Bug Fixes
  • Project each OneOrMore depth branch with project_path_endpoints before Union/Distinct; no 0‑hop branch (per spec).
  • Strengthened two + tests to assert values, and added closure oracles (open-ended, bound-subject, cycle, nested ^(:p+) and (:p+)/:q) plus regression guards for *, ?, /, ^.
  • Kept bounded expansion at MAX_DEPTH = 50 and updated a stale test comment; other path operators unchanged.

^{Written for commit f77bae7. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/grafeo-engine/src/query/translators/sparql.rs">

<violation number="1" location="crates/grafeo-engine/src/query/translators/sparql.rs:1934">
P2: `project_path_endpoints` returns `input` unchanged when both endpoints are non-variables. For fully-bound `path+` queries (e.g., `<a> <p>+ <b>`), the newly-added call in `translate_one_or_more_path` is a no-op, so depth branches retain heterogeneous intermediate columns and reach `Union`/`Distinct` with mismatched schemas.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: project_path_endpoints returns input unchanged when both endpoints are non-variables. For fully-bound path+ queries (e.g., <a> <p>+ <b>), the newly-added call in translate_one_or_more_path is a no-op, so depth branches retain heterogeneous intermediate columns and reach Union/Distinct with mismatched schemas.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/grafeo-engine/src/query/translators/sparql.rs, line 1934:

<comment>`project_path_endpoints` returns `input` unchanged when both endpoints are non-variables. For fully-bound `path+` queries (e.g., `<a> <p>+ <b>`), the newly-added call in `translate_one_or_more_path` is a no-op, so depth branches retain heterogeneous intermediate columns and reach `Union`/`Distinct` with mismatched schemas.</comment>

<file context>
@@ -1922,7 +1925,13 @@ impl SparqlTranslator {
+            // without this projection the heterogeneous branches reach `Union`
+            // and `Distinct` unnormalized and the visible object column ends up
+            // holding the first intermediate hop instead of the true endpoint.
+            branches.push(self.project_path_endpoints(&subject, &object, branch));
         }
 
</file context>

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 70 untouched benchmarks

---

_{Comparing teipsum:fix-path-one-or-more (f77bae7) with main (4ebae02)}