gregfelice commented on PR #2360: URL: https://github.com/apache/age/pull/2360#issuecomment-4772632447
Follow-up on the remaining review items — PR body updated, plus one finding worth flagging. **Test coverage — `RETURN (a:Label)`:** while adding this case I found it isn't a missing test, it's a latent bug. A single-node labeled pattern as a boolean (`WHERE (a:Person)` / `RETURN (a:Person)`) is accepted but **doesn't filter by label** — it's trivially true. Root cause is pre-existing in the transform layer: `make_path_join_quals()` (`cypher_clause.c:5454`) early-returns for vertex-only patterns (`list_length(entities) < 3`), so the label-filter quals — only emitted for relationship patterns — are skipped. It's byte-identical on `master` and this PR touches no transform code; the grammar change just makes it reachable via the new `(a:Label)` surface. Relationship patterns (this PR's actual feature) correlate and filter correctly. I opened #2443 to track the transform fix rather than scope-creep it into the grammar PR, and documented it as a known limitation in the PR body. (Verified by building this branch in a container and running the repros.) **Doc notes added to the PR body:** - `YYRHSLOC` change is a required GLR correctness fix (different RHS stack layout under GLR), not cosmetic. - Pattern expressions in projection are EXISTS-sublink-per-row (O(rows × subquery)), distinct from WHERE where the planner may pull them up. So the review items net out as: #1 (-Werror) done, #2 (%expect) solved via toolchain pinning, #3 (parse-time) measured — no regression, #4 (coverage) done for relationship patterns with the single-node label case tracked in #2443. Ready for another look whenever. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
