Skip to content

fix: joining patterns across expansions BED-7468#42

Merged
urangel merged 2 commits intomainfrom
BED-7468
Mar 17, 2026
Merged

fix: joining patterns across expansions BED-7468#42
urangel merged 2 commits intomainfrom
BED-7468

Conversation

@urangel
Copy link
Contributor

@urangel urangel commented Mar 16, 2026

Joining expansion pattern steps is updated so that result sets contain path resolutions that satisfy the combined query pattern rather than only a part of a multi step relationship pattern.

The following examples use the active directory sample data as the data source.
Examples:

Inbound multi step relationship pattern with consecutive expansion steps

MATCH p = (t:Base)<-[:MemberOf*1..]-(:Base)<-[:Contains*1..]-()
WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0'          
RETURN p  
LIMIT 3

Before:
image

After:
image

Outbound multi step relationship pattern with consecutive expansion steps

MATCH p = ()-[:Contains*1..]->(:Base)-[:MemberOf*1..]->(t:Base)
WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0'          
RETURN p  
LIMIT 3

Before:
image

After:
image

Inbound multi step relationship pattern with single non-expansion step joined to expansion

MATCH p = (t:Base)<-[:MemberOf]-(:Base)<-[:Contains*1..]-()
WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0'          
RETURN p  
LIMIT 3

Before:
image

After:
image

Outbound multi step relationship pattern with single non-expansion step joined to expansion

MATCH p = ()-[:Contains]->(:Base)-[:MemberOf*1..]->(t:Base)
WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0'          
RETURN p  
LIMIT 3

Before:
image

After:
image

Summary by CodeRabbit

  • Refactor
    • Improved internal query translation logic for path traversal and constraint handling.
    • Enhanced consistency in how pattern-based queries are constructed and processed.
    • Updated internal architecture to streamline traversal step handling across multiple query types.

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

Walkthrough

The changes introduce a new TraversalStepContext struct to encapsulate traversal step information, refactoring method signatures in the pattern translation logic to accept this context wrapper instead of separate parameters. Simultaneously, SQL test translation cases are updated with adjusted alias references and simplified filter conditions across multiple query scenarios.

Changes

Cohort / File(s) Summary
SQL Test Translation Cases
cypher/models/pgsql/test/translation_cases/multipart.sql, cypher/models/pgsql/test/translation_cases/pattern_binding.sql, cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
Adjustments to SQL translation test cases including simplified filters on nodes (removing objectid-based conditions), updated alias consistency in nested CTEs, reworked predicate constraints on node/edge components, and modified join conditions for path root linkage validation.
Pattern Context Type Introduction
cypher/models/pgsql/translate/pattern.go
Introduces new TraversalStepContext struct containing PreviousStep, CurrentStep, and IsRootStep fields. Updates method signatures for buildExpansionPattern and buildShortestPathsExpansionPattern to accept context object instead of separate step and flag parameters.
Expansion Method Signature Updates
cypher/models/pgsql/translate/expansion.go
Refactors method signatures in buildExpansionPatternRoot, buildExpansionPatternStep, and buildExpansionProjectionConstraints to accept TraversalStepContext. Adds internal step extraction logic and introduces joinCondition computation comparing previous step's frame binding against current step's left node and expansion root ID.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A context wraps the steps so neat,
Where previous and current softly meet,
SQL aliases dance in new formation,
Join conditions spring from consolidation!
The patterns bloom with structured grace,
Refactored with a rabbit's embrace! 🌿✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: joining patterns across expansions BED-7468' clearly describes the main change: fixing how pattern joins work across expansion steps in Cypher-to-SQL translation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-7468
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can disable sequence diagrams in the walkthrough.

Disable the reviews.sequence_diagrams setting to disable sequence diagrams in the walkthrough.

@urangel urangel marked this pull request as ready for review March 17, 2026 14:32
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cypher/models/pgsql/test/translation_cases/multipart.sql`:
- Line 48: The recursive CTE s2 only seeds depth=1 paths from edge e0, so *0..
is missing the zero-length (root = target) case; add a zero-depth seed row to s2
before the existing seed (or union it) that sets root_id = e0.end_id, next_id =
e0.end_id, depth = 0, satisfied based on the same predicate used for the seed
(e.g. (n1.properties ->> 'objectid') like '%-516' and n1.kind_ids @> ...),
is_cycle = false (or true as appropriate), and path = an empty array (array[]
cast to the same id array type) so the recursive walk includes the 0-hop branch.
Reference: modify the s2(...) AS (select ... from edge e0 ...) seed block that
currently creates (e0.end_id, e0.start_id, 1, ...) to also include the
zero-depth union entry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d2288301-b1de-4c99-a6df-4a399e1dd1e4

📥 Commits

Reviewing files that changed from the base of the PR and between f97f931 and 4c9ac54.

📒 Files selected for processing (5)
  • cypher/models/pgsql/test/translation_cases/multipart.sql
  • cypher/models/pgsql/test/translation_cases/pattern_binding.sql
  • cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/pattern.go

Copy link
Contributor

@zinic zinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good bit of plumbing. Test case changes pass muster. Ship it!

@urangel urangel merged commit 0e1fe09 into main Mar 17, 2026
2 checks passed
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.

3 participants