Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis change adjusts PostgreSQL traversal-root correlation for pattern predicates by optionally omitting the previous frame as a ChangesPattern predicate correlation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PatternPredicateBuilder
participant TraversalTranslator
participant PostgreSQLCTEs
PatternPredicateBuilder->>TraversalTranslator: mark root to omit previous frame source
TraversalTranslator->>PostgreSQLCTEs: build correlated traversal CTEs
PostgreSQLCTEs-->>PatternPredicateBuilder: evaluate terminal existence predicate
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cypher/models/pgsql/translate/traversal_directionless.go (1)
204-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for multi-step directionless predicates.
This changed path handles unbound directionless roots, but the supplied multi-step regressions are directed; the existing undirected cases are single-hop. Add a two-hop undirected pattern predicate and assert its correlated terminal set to protect this source-selection behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cypher/models/pgsql/translate/traversal_directionless.go` around lines 204 - 210, Add a regression test for a two-hop undirected pattern predicate with an unbound directionless root, asserting the correlated terminal set produced by the traversal. Reuse the existing directionless predicate test setup and ensure the case exercises previousFrameTraversalSource and the resulting source selection in the generated query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cypher/models/pgsql/translate/traversal_directionless.go`:
- Around line 204-210: Add a regression test for a two-hop undirected pattern
predicate with an unbound directionless root, asserting the correlated terminal
set produced by the traversal. Reuse the existing directionless predicate test
setup and ensure the case exercises previousFrameTraversalSource and the
resulting source selection in the generated query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d428fd8-661c-4e86-a02d-2bfa7684bc56
📒 Files selected for processing (7)
cypher/models/pgsql/test/translation_cases/nodes.sqlcypher/models/pgsql/translate/model.gocypher/models/pgsql/translate/predicate.gocypher/models/pgsql/translate/query.gocypher/models/pgsql/translate/traversal.gocypher/models/pgsql/translate/traversal_directionless.gointegration/testdata/cases/bed8321-multi_step_pattern_predicates.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cypher/models/pgsql/translate/predicate_test.go`:
- Around line 272-291: The predicate translation test’s global string assertions
do not verify that each edge-kind filter and source belongs to the correct CTE.
Update the assertions around the formatted query in the affected test to extract
or parse the bodies of s1 and s2, then assert s1 contains the s0 correlation and
kind 2 filter while s2 sources from s1 and contains kind 3; keep the terminal
existence assertion scoped to s2.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9aeb54e-0c91-4840-89f9-b60816c8544d
📒 Files selected for processing (1)
cypher/models/pgsql/translate/predicate_test.go
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
2 participants
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.
Description
Fixes a query decorrelation bug in multi-step pattern predicate translation. When a pattern predicate has more than one traversal step (e.g.
where (:User)-[:MemberOf]->(:Group)-[:MemberOf]->(g)), the root step was comma-joining the enclosing query part's CTE as a FROM source, causing a re-scan that broke row correlationwith the outer query. The fix introduces
OmitPreviousFrameSourceonTraversalStepto suppress that join for pattern predicate roots.Resolves: BED-8321
Type of Change
Testing
make test_allwithCONNECTION_STRINGset)Screenshots (if appropriate):
N/A
Driver Impact
drivers/pg)drivers/neo4j)Checklist
go.mod/go.sumare up to date if dependencies changedSummary by CodeRabbit
Groupcounting.