Skip to content

CySQL Translation Fixes#76

Merged
zinic merged 17 commits into
SpecterOps:mainfrom
zinic:main
May 12, 2026
Merged

CySQL Translation Fixes#76
zinic merged 17 commits into
SpecterOps:mainfrom
zinic:main

Conversation

@zinic
Copy link
Copy Markdown
Contributor

@zinic zinic commented May 11, 2026

Description

Adds PostgreSQL Cypher translation fixes for recent query-shape defects: zero-hop variable-length expansions, temporal arithmetic comparisons against relationship properties, path list functions in expansion predicates, and shortest path aggregation alias preservation. Adds translation and integration coverage for the reported shapes.

Resolves:

  • BP-2518
  • BED-8260
  • BED-5292
  • BED-5945

Type of Change

  • Chore
  • Bug fix
  • New feature / enhancement
  • Refactor
  • Test coverage
  • Build / CI / tooling
  • Documentation

Testing

  • Unit tests added / updated
  • Integration tests added / updated
  • Full test suite run (make test_all with CONNECTION_STRING set)

Screenshots (if appropriate):

N/A

Driver Impact

  • PostgreSQL driver (drivers/pg)
  • Neo4j driver (drivers/neo4j)

Checklist

  • Code is formatted
  • All existing tests pass
  • go.mod / go.sum are up to date if dependencies changed

Summary by CodeRabbit

  • New Features

    • Added Cypher functions: head(), tail(), nodes(), relationships().
    • Support for zero-length variable-length expansions (*0..).
    • New shortest-path guard that reports a clear error when endpoints resolve to the same node.
  • Behavioral Improvements

    • Improved temporal arithmetic handling (date/time ± interval) and smarter temporal-aware query rewrites.
  • Tests

    • Many new unit/integration tests covering path functions, zero-hop expansions, quantifiers, temporal arithmetic, and translation cases.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@zinic has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 23 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4543b025-6f2f-4bb5-89ec-e6d3047971b5

📥 Commits

Reviewing files that changed from the base of the PR and between c02addd and 1366719.

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

Walkthrough

Implements Cypher HEAD/TAIL/NODES/RELATIONSHIPS translation and zero-depth (*0..) expansions, with array-slice formatting/walking, projection/renamer updates, temporal type inference, quantifier/property handling, Neo4j query-rewrite, SQL helper, and extensive tests.

Changes

Feature Implementation

Layer / File(s) Summary
Function and Model Contracts
cypher/models/cypher/functions.go, cypher/models/pgsql/functions.go, cypher/models/pgsql/pgtypes.go
Adds HeadFunction/TailFunction/NodesFunction/RelationshipsFunction, PG nodes/edges column identifiers, and extends/reformats PG function identifiers.
Temporal Type System
cypher/models/pgsql/pgtypes.go, cypher/models/pgsql/pytypes_test.go
Adds DataType.IsTemporalType() and TemporalIntervalArithmeticResultType(), updates OperatorResultType for interval/temporal arithmetic, and adds unit tests.
PgSQL Model Changes
cypher/models/pgsql/model.go
Case implements NodeType/AsExpression/AsSelectItem; TypeCast implements AsSelectItem; ArrayIndex/ArraySlice gain CastType and TypeHint behavior.
Formatter & Walk Cursor
cypher/models/pgsql/format/format.go, cypher/models/walk/walk_pgsql.go
Adds formatCase, formats ArraySlice slices, and adds walk cursor handling for Case and ArraySlice nodes; includes formatter test.
Translator Inference & Function Translation
cypher/models/pgsql/translate/function.go, cypher/models/pgsql/translate/*
Adds inference helpers for identifier/row-column/array/path types, enforces array typing for head/tail, implements translators for head, tail, nodes, and relationships, and extends dispatch.
Path Projection Refactor
cypher/models/pgsql/translate/projection.go
Centralizes pathBindingReference/pathCompositeReference helpers and refactors path composite expression construction.
Zero-Depth Expansion
cypher/models/pgsql/translate/expansion.go
Adds ZeroDepthStatement, builds optional depth-0 SELECTs when MinDepth==0, rewrites ArraySlice bound seeds preserving CastType, unions depth-0 results with recursive CTE, and refactors shortest-path self-endpoint guards to CASE-based logic.
ArraySlice Support & Renamer
cypher/models/pgsql/translate/renamer.go
Adds rewrite helpers for ArraySlice and Case nodes across many AST positions; tests validate slice rewriting in projections and order-by.
Property Lookup & Quantifiers
cypher/models/pgsql/translate/expression.go, cypher/models/pgsql/translate/quantifiers.go
Detects composite-property-capable expression types, rewrites property lookups on non-identifiers to RowColumnReference(ColumnProperties), and converts quantifier property-lookups to JSONB->text[] with collection-type inference.
Scope & Locality
cypher/models/pgsql/translate/model.go, cypher/models/pgsql/translate/renamer_test.go
Expands identifier reference detection to multiple pgsql node types, adds expressionReferencesOnlyLocalIdentifiers helper, refactors isLocalToScope, and adds renamer tests for ArraySlice.
Tests & Translation Cases
cypher/models/pgsql/test/*, integration/testdata/cases/*
Adds/updates many PG translation cases and integration JSON tests covering temporal filtering, zero-hop expansion, pattern-binding with HEAD/TAIL/NODES, quantifiers, and shortest-path aggregation; updates test kind mapper.
Neo4j Rewrite & Wiring
drivers/neo4j/query_rewrite.go, drivers/neo4j/transaction.go, drivers/neo4j/query_rewrite_internal_test.go
Adds rewriteQuery with pre-check and temporal property comparison rewriter, tests for detection/rewrite behaviors, and wires transaction.Query to use rewriteQuery.
Postgres helper SQL & validation
drivers/pg/query/sql/schema_up.sql, drivers/pg/query/sql/schema_down.sql, cypher/models/pgsql/test/validation_integration_test.go
Adds public.shortest_path_self_endpoint_error(root_id int8, terminal_id int8) that raises on equal endpoints, includes drop in schema_down, and adds validation test coverage.

Sequence Diagram(s)

sequenceDiagram
  participant Cypher as Cypher Parser
  participant Translator as PG Translator
  participant TypeInfer as Type Inference
  participant Expander as ExpansionBuilder
  participant Renamer as FrameBindingRewriter
  participant SQL as PostgreSQL Output

  Cypher->>Translator: HEAD(arr)/TAIL(arr)/NODES(p)/RELATIONSHIPS(p)
  Translator->>TypeInfer: infer array/path element or composite type
  TypeInfer-->>Translator: DataType (with CastType hint)
  Translator->>SQL: emit array index/slice or casted row-column reference

  Cypher->>Expander: expansion with MinDepth=0
  Expander->>TypeInfer: infer terminal satisfaction/locality
  Expander->>Renamer: rewrite bound endpoint seeds (ArraySlice) as needed
  Expander->>SQL: build zero-depth SELECT and union with recursive body

  Translator->>Renamer: emit expressions that may be scope-rewritten
  Renamer->>SQL: final rewritten AST -> SQL output
Loading

🎯 4 (Complex) | ⏱️ ~60 minutes

"🐰
I nibble arrays, take head and tail,
zero hops find paths along the trail,
time ticks with intervals clear and neat,
projections stitch nodes into paths complete,
SQL slices sing — a rabbit's code-time feat!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.49% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'CySQL Translation Fixes' is directly related to the main change: PostgreSQL Cypher translation fixes for query-shape defects including zero-hop expansions, temporal arithmetic, path functions, and alias preservation.
Description check ✅ Passed The PR description fully covers the required template sections: it provides a clear description of changes (PostgreSQL Cypher translation fixes for zero-hop expansions, temporal arithmetic, path functions, and alias preservation), identifies the type of change (bug fixes, features/enhancements, and test coverage), confirms testing (unit/integration tests added and full test suite run), specifies driver impact (PostgreSQL driver), and completes the checklist items.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@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: 8

🤖 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/pgtypes.go`:
- Around line 119-127: IsTemporalType currently treats Date as a temporal
leading to mis-typing of Date ± Interval; remove Date from
DataType.IsTemporalType() so Date is not treated as a generic temporal, then
update OperatorResultType() to explicitly handle date/interval arithmetic: add
clauses that map Date + Interval and Date - Interval to TimestampWithoutTimeZone
(and leave other temporal combos alone), remove the old Date case in
OperatorResultType() that returned Date for those operations, and adjust the
Interval case logic in OperatorResultType() to account for the new explicit
Date+Interval handling so Interval results stay correct.

In `@cypher/models/pgsql/test/translation_cases/multipart.sql`:
- Line 51: The CTEs s2 and s5 include an explicit "one-hop" union arm (the
SELECT ... from s2_seed join edge e0 ... producing depth = 1 and the equivalent
SELECT ... from s5_seed join edge e2 ...) which duplicates the same results
produced by the recursive branch (s2 join lateral ... and s5 join lateral ...
with s?.depth + 1). Remove those explicit depth=1 seed union arms (the clauses
referencing s2_seed join edge e0 and s5_seed join edge e2) so each recursive CTE
only has the initial seed row (depth = 0 or the seed) plus the recursive
union-all step, leaving the lateral recursive logic and path construction
(s2.path, s5.path, e0/e2 handling) intact.

In `@cypher/models/pgsql/translate/expansion.go`:
- Around line 590-606: The zero-hop branch currently uses a local predicate
returned by expansionTerminalSatisfactionLocality even if that predicate
references the traversal edge alias, which is not in scope for depth=0; update
the block handling expansionModel.TerminalNodeSatisfactionProjection so that
after computing localSatisfaction :=
expansionTerminalSatisfactionLocality(s.traversalStep) you check if
referencesIdentifier(localSatisfaction, s.traversalStep.Edge.Identifier) and, if
true, treat localSatisfaction as nil (i.e., skip setting satisfiedExpression and
skip adding zeroDepthNodeJoin entries to fromClause.Joins); ensure all uses in
this block (satisfiedExpression and the two join appends) are conditional on
localSatisfaction being non-nil so edge-dependent terminal predicates do not
drive a zero-depth select.

In `@cypher/models/pgsql/translate/expression.go`:
- Around line 56-60: The code currently returns nil for non-composite
expressions in the default case after calling
s.inferExpressionType(translatedAtom), which silently discards an unsupported
property lookup; instead, detect when
expressionHasCompositeProperties(expressionType) is false and return a clear
error (e.g., "unsupported property lookup on non-composite expression") rather
than nil so the caller sees a direct error; update the default branch in
expression.go to return that error (referencing translatedAtom,
s.inferExpressionType, and expressionHasCompositeProperties) instead of
returning nil.

In `@cypher/models/pgsql/translate/function.go`:
- Around line 134-141: expressionForPath currently only checks s.scope.Lookup
for the identifier and fails on aliased identifiers; update expressionForPath to
resolve identifier aliases before validating the binding type. After obtaining
binding := s.scope.Lookup(typedExpression), if the binding represents an alias
(use the scope's alias/link fields or a ResolveAlias/OriginalIdentifier property
that inferExpressionType already handles), follow the alias to the original
binding (iteratively if needed) and then verify binding.DataType ==
pgsql.PathComposite and call expressionForPathComposite(binding, s.scope); this
ensures WITH p AS q RETURN nodes(q) / relationships(q) resolve correctly.

In `@cypher/models/pgsql/translate/quantifiers.go`:
- Around line 93-97: The code currently hard-fails inside the switch on
quantifierArrayExpression when encountering a *pgsql.BinaryExpression that is
not a property lookup (checked via pgsql.OperatorIsPropertyLookup), returning
"unknown cypher array type" and preventing the later inference path; remove that
early return and allow non-property binary expressions to fall through into the
inference logic (the same change should be applied to the other similar branches
around the other *pgsql.BinaryExpression checks at the noted spots) so that
non-property binaries are inferred instead of rejected.

In `@cypher/models/pgsql/translate/renamer.go`:
- Around line 182-229: The switch only handles the pointer form
(*pgsql.ArraySlice) but not the value form (pgsql.ArraySlice), so value-form
nodes skip rewriting Expression/Lower/Upper and keep stale frame refs; add a new
case for pgsql.ArraySlice that mirrors the logic in the existing
*pgsql.ArraySlice case, calling rewriteIdentifierScopeReference and
rewriteCompoundIdentifierScopeReference with s.scope for
typedExpression.Expression, typedExpression.Lower and typedExpression.Upper so
all identifier/compound-identifier variants are rebound correctly.

In `@integration/testdata/cases/pattern_binding_inline.json`:
- Around line 214-240: The test fixture doesn't exercise the HEAD(r).enforced
branch because the enforced path is length 1 so TAIL(TAIL(NODES(p))) is empty;
add a longer enforced path that contains a later OU with blocksinheritance=true
so NONE(...) would be false without HEAD(r).enforced. Concretely, add a node
like "ou-blocked-2" (kinds ["OU"], properties {"blocksinheritance": true}) and
an edge from "gpo-enforced" to "ou-blocked-2" (kind "GPLink", properties
{"enforced": true}) and another edge from "ou-blocked-2" to "base-enforced"
(kind "Contains"), then update the expected "path_node_ids" to include the
enforced multi-hop path (e.g., ["gpo-enforced","ou-blocked-2","base-enforced"])
and corresponding "path_edge_kinds" (["GPLink","Contains"]) so the test proves
HEAD(r).enforced is required.
🪄 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: 46246fd4-3b4f-4f91-b37e-73dae887e725

📥 Commits

Reviewing files that changed from the base of the PR and between eea249f and ea126b4.

📒 Files selected for processing (22)
  • cypher/models/cypher/functions.go
  • cypher/models/pgsql/format/format.go
  • cypher/models/pgsql/model.go
  • cypher/models/pgsql/pgtypes.go
  • cypher/models/pgsql/pytypes_test.go
  • 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/test/translation_cases/shortest_paths.sql
  • cypher/models/pgsql/test/translation_test.go
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/expression.go
  • cypher/models/pgsql/translate/function.go
  • cypher/models/pgsql/translate/projection.go
  • cypher/models/pgsql/translate/quantifiers.go
  • cypher/models/pgsql/translate/renamer.go
  • cypher/models/pgsql/translate/with.go
  • cypher/models/walk/walk_pgsql.go
  • integration/testdata/cases/expansion_inline.json
  • integration/testdata/cases/pattern_binding_inline.json
  • integration/testdata/cases/shortest_paths_inline.json
  • integration/testdata/cases/temporal_inline.json

Comment thread cypher/models/pgsql/pgtypes.go
Comment thread cypher/models/pgsql/test/translation_cases/multipart.sql Outdated
Comment thread cypher/models/pgsql/translate/expansion.go
Comment thread cypher/models/pgsql/translate/expression.go Outdated
Comment thread cypher/models/pgsql/translate/function.go
Comment thread cypher/models/pgsql/translate/quantifiers.go Outdated
Comment thread cypher/models/pgsql/translate/renamer.go
Comment thread integration/testdata/cases/pattern_binding_inline.json
Copy link
Copy Markdown

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cypher/models/pgsql/translate/renamer.go (1)

97-113: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add pgsql.ArraySlice handling in pgsql.Projection and *pgsql.OrderBy branches.

The walker visits both value-form and pointer-form pgsql.ArraySlice nodes, but pgsql.Projection (lines 97–113) and *pgsql.OrderBy (lines 268–276) lack cases for them. When an ArraySlice appears directly in either position, the rewrite is lost because the parent does not write back the mutated value. Other parent types like pgsql.CompositeValue and pgsql.FunctionCall already include explicit write-back for both forms.

Suggested fix
 case pgsql.Projection:
 	for idx, projection := range typedExpression {
 		switch typedProjection := projection.(type) {
 		case pgsql.Identifier:
 			if rewritten, err := rewriteIdentifierScopeReference(s.scope, typedProjection); err != nil {
 				return err
 			} else {
 				typedExpression[idx] = rewritten
 			}

 		case pgsql.CompoundIdentifier:
 			if rewritten, err := rewriteCompoundIdentifierScopeReference(s.scope, typedProjection); err != nil {
 				return err
 			} else {
 				typedExpression[idx] = rewritten
 			}
+
+		case pgsql.ArraySlice:
+			if err := s.rewriteArraySlice(&typedProjection); err != nil {
+				return err
+			}
+			typedExpression[idx] = typedProjection
+
+		case *pgsql.ArraySlice:
+			if err := s.rewriteArraySlice(typedProjection); err != nil {
+				return err
+			}
 		}
 	}

 case *pgsql.OrderBy:
 	switch typedOrderByExpression := typedExpression.Expression.(type) {
 	case pgsql.Identifier:
 		if rewritten, err := rewriteIdentifierScopeReference(s.scope, typedOrderByExpression); err != nil {
 			return err
 		} else {
 			typedExpression.Expression = rewritten
 		}

 	case pgsql.CompoundIdentifier:
 		if rewritten, err := rewriteCompoundIdentifierScopeReference(s.scope, typedOrderByExpression); err != nil {
 			return err
 		} else {
 			typedExpression.Expression = rewritten
 		}
+
+	case pgsql.ArraySlice:
+		if err := s.rewriteArraySlice(&typedOrderByExpression); err != nil {
+			return err
+		}
+		typedExpression.Expression = typedOrderByExpression
+
+	case *pgsql.ArraySlice:
+		if err := s.rewriteArraySlice(typedOrderByExpression); err != nil {
+			return err
+		}
 	}
🤖 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/renamer.go` around lines 97 - 113, Handle
pgsql.ArraySlice in the pgsql.Projection and *pgsql.OrderBy branches: when
iterating pgsql.Projection (inside the case for pgsql.Projection) add cases for
both pgsql.ArraySlice and *pgsql.ArraySlice, call the appropriate rewrite (same
pattern used for Identifier/CompoundIdentifier) and write the rewritten node
back into typedExpression (e.g., typedExpression[idx] = rewritten or
typedExpression[idx] = &rewritten for pointer form); similarly, in the
*pgsql.OrderBy branch add handling for both pgsql.ArraySlice and
*pgsql.ArraySlice for the order field and assign the rewritten value back into
typedOrderBy.Field (using value or pointer as appropriate) so the parent retains
the mutated node.
♻️ Duplicate comments (1)
cypher/models/pgsql/translate/expansion.go (1)

590-605: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't carry edge-bound terminal predicates into the zero-hop arm.

expansionTerminalSatisfactionLocality can still return a predicate that references s.traversalStep.Edge.Identifier, but the depth-0 SELECT never has that alias in scope. The new zero-depth branch will therefore emit invalid SQL for relationship-aware terminal predicates. Leave the zero-hop row at its default false satisfaction in that case, and only add the node joins when the remaining local predicate is still usable.

Suggested fix
 	if expansionModel.TerminalNodeSatisfactionProjection != nil {
 		localSatisfaction, _ := expansionTerminalSatisfactionLocality(s.traversalStep)
-		if localSatisfaction == nil {
+		if localSatisfaction != nil &&
+			referencesIdentifier(localSatisfaction, s.traversalStep.Edge.Identifier) {
+			localSatisfaction = nil
+		} else if localSatisfaction == nil {
 			localSatisfaction = pgsql.NewLiteral(true, pgsql.Boolean)
 		}
 
-		satisfiedExpression = localSatisfaction
-
-		if seed != nil && referencesIdentifier(localSatisfaction, s.traversalStep.LeftNode.Identifier) {
-			fromClause.Joins = append(fromClause.Joins, zeroDepthNodeJoin(s.traversalStep.LeftNode.Identifier, rootIDExpression))
-		}
-
-		if s.traversalStep.RightNode.Identifier != s.traversalStep.LeftNode.Identifier &&
-			referencesIdentifier(localSatisfaction, s.traversalStep.RightNode.Identifier) {
-			fromClause.Joins = append(fromClause.Joins, zeroDepthNodeJoin(s.traversalStep.RightNode.Identifier, rootIDExpression))
+		if localSatisfaction != nil {
+			satisfiedExpression = localSatisfaction
+
+			if seed != nil && referencesIdentifier(localSatisfaction, s.traversalStep.LeftNode.Identifier) {
+				fromClause.Joins = append(fromClause.Joins, zeroDepthNodeJoin(s.traversalStep.LeftNode.Identifier, rootIDExpression))
+			}
+
+			if s.traversalStep.RightNode.Identifier != s.traversalStep.LeftNode.Identifier &&
+				referencesIdentifier(localSatisfaction, s.traversalStep.RightNode.Identifier) {
+				fromClause.Joins = append(fromClause.Joins, zeroDepthNodeJoin(s.traversalStep.RightNode.Identifier, rootIDExpression))
+			}
 		}
 	}
🤖 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/expansion.go` around lines 590 - 605,
expansionTerminalSatisfactionLocality may return a predicate that references the
edge alias (s.traversalStep.Edge.Identifier), which is not in scope for the
zero-depth SELECT; change the logic in the TerminalNodeSatisfactionProjection
branch so you first compute localSatisfaction via
expansionTerminalSatisfactionLocality(s.traversalStep) and then validate that
localSatisfaction does NOT reference the edge identifier (using
referencesIdentifier(localSatisfaction, s.traversalStep.Edge.Identifier)); if it
does reference the edge, discard it and treat zero-hop satisfaction as the
default false literal (pgsql.NewLiteral(false,...)) and do NOT add any
zero-depth node joins; only append zeroDepthNodeJoin entries to fromClause.Joins
when the retained localSatisfaction is usable and still references the relevant
node identifiers (use existing referencesIdentifier checks for
LeftNode.Identifier and RightNode.Identifier).
🤖 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.

Outside diff comments:
In `@cypher/models/pgsql/translate/renamer.go`:
- Around line 97-113: Handle pgsql.ArraySlice in the pgsql.Projection and
*pgsql.OrderBy branches: when iterating pgsql.Projection (inside the case for
pgsql.Projection) add cases for both pgsql.ArraySlice and *pgsql.ArraySlice,
call the appropriate rewrite (same pattern used for
Identifier/CompoundIdentifier) and write the rewritten node back into
typedExpression (e.g., typedExpression[idx] = rewritten or typedExpression[idx]
= &rewritten for pointer form); similarly, in the *pgsql.OrderBy branch add
handling for both pgsql.ArraySlice and *pgsql.ArraySlice for the order field and
assign the rewritten value back into typedOrderBy.Field (using value or pointer
as appropriate) so the parent retains the mutated node.

---

Duplicate comments:
In `@cypher/models/pgsql/translate/expansion.go`:
- Around line 590-605: expansionTerminalSatisfactionLocality may return a
predicate that references the edge alias (s.traversalStep.Edge.Identifier),
which is not in scope for the zero-depth SELECT; change the logic in the
TerminalNodeSatisfactionProjection branch so you first compute localSatisfaction
via expansionTerminalSatisfactionLocality(s.traversalStep) and then validate
that localSatisfaction does NOT reference the edge identifier (using
referencesIdentifier(localSatisfaction, s.traversalStep.Edge.Identifier)); if it
does reference the edge, discard it and treat zero-hop satisfaction as the
default false literal (pgsql.NewLiteral(false,...)) and do NOT add any
zero-depth node joins; only append zeroDepthNodeJoin entries to fromClause.Joins
when the retained localSatisfaction is usable and still references the relevant
node identifiers (use existing referencesIdentifier checks for
LeftNode.Identifier and RightNode.Identifier).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 104ba301-1150-467b-b2b9-8770be3f7d4f

📥 Commits

Reviewing files that changed from the base of the PR and between ea126b4 and 166b0b2.

📒 Files selected for processing (15)
  • cypher/models/pgsql/pgtypes.go
  • cypher/models/pgsql/pytypes_test.go
  • cypher/models/pgsql/test/translation_cases/multipart.sql
  • cypher/models/pgsql/test/translation_cases/nodes.sql
  • cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
  • cypher/models/pgsql/test/translation_cases/quantifiers.sql
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/expression.go
  • cypher/models/pgsql/translate/quantifiers.go
  • cypher/models/pgsql/translate/renamer.go
  • cypher/models/pgsql/translate/renamer_test.go
  • cypher/models/pgsql/translate/semantic_drift_test.go
  • integration/testdata/cases/expansion_inline.json
  • integration/testdata/cases/pattern_binding_inline.json
  • integration/testdata/cases/quantifiers_inline.json
✅ Files skipped from review due to trivial changes (1)
  • integration/testdata/cases/quantifiers_inline.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • cypher/models/pgsql/pytypes_test.go
  • cypher/models/pgsql/test/translation_cases/pattern_expansion.sql
  • integration/testdata/cases/pattern_binding_inline.json
  • cypher/models/pgsql/pgtypes.go
  • cypher/models/pgsql/translate/expression.go
  • integration/testdata/cases/expansion_inline.json
  • cypher/models/pgsql/translate/quantifiers.go

Copy link
Copy Markdown

@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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@drivers/neo4j/query_rewrite.go`:
- Around line 51-55: Add a cheap fast-path guard in rewriteQuery to avoid
parsing queries that cannot match temporal rewrites: implement a helper like
containsTemporalRewriteCandidate(query string) bool (using strings.ToLower and
substr checks for "date(", "time(", "localtime(", "datetime(", "localdatetime(")
and call it at the top of rewriteQuery; if it returns false immediately return
the original query and parameters without calling frontend.ParseCypher. Remember
to import the strings package and keep the existing rewriteQuery signature and
error behavior unchanged.
🪄 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: e57eae66-c2c1-4925-99d7-2eb14a6426ec

📥 Commits

Reviewing files that changed from the base of the PR and between 166b0b2 and c83def2.

📒 Files selected for processing (11)
  • cypher/models/pgsql/model.go
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/expansion_test.go
  • cypher/models/pgsql/translate/function.go
  • cypher/models/pgsql/translate/function_test.go
  • cypher/models/pgsql/translate/model.go
  • cypher/models/pgsql/translate/renamer.go
  • cypher/models/pgsql/translate/renamer_test.go
  • drivers/neo4j/query_rewrite.go
  • drivers/neo4j/query_rewrite_internal_test.go
  • drivers/neo4j/transaction.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • cypher/models/pgsql/model.go
  • cypher/models/pgsql/translate/function.go
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/renamer.go

Comment thread drivers/neo4j/query_rewrite.go
Copy link
Copy Markdown

@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 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/expansion.go`:
- Around line 1939-1954: The code currently replaces the original primer ∪
recursive set with zeroDepth ∪ recursive when ZeroDepthStatement is present,
dropping the primer branch and changing first-hop behavior; instead keep the
primer branch as the left side of the overall expansion. Modify
ExpansionBuilder.Build so that when s.ZeroDepthStatement != nil you construct
expansionBody as a SetOperation whose LOperand preserves the primer branch (e.g.
a SetExpression combining s.PrimerStatement and *s.ZeroDepthStatement with the
same Operator and s.UseUnionAll) and whose ROperand remains
s.RecursiveStatement, ensuring PrimerStatement, ZeroDepthStatement,
RecursiveStatement and UseUnionAll are used to compose the nested SetExpression
rather than replacing the primer entirely.
🪄 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: 92c1e731-8b6a-408c-acac-35cd1e279ed7

📥 Commits

Reviewing files that changed from the base of the PR and between c83def2 and c02addd.

📒 Files selected for processing (15)
  • cypher/models/pgsql/format/format.go
  • cypher/models/pgsql/format/format_test.go
  • cypher/models/pgsql/functions.go
  • cypher/models/pgsql/model.go
  • cypher/models/pgsql/test/translation_cases/shortest_paths.sql
  • cypher/models/pgsql/test/validation_integration_test.go
  • cypher/models/pgsql/translate/expansion.go
  • cypher/models/pgsql/translate/expansion_test.go
  • cypher/models/pgsql/translate/renamer.go
  • cypher/models/pgsql/translate/renamer_test.go
  • cypher/models/walk/walk_pgsql.go
  • drivers/neo4j/query_rewrite.go
  • drivers/neo4j/query_rewrite_internal_test.go
  • drivers/pg/query/sql/schema_down.sql
  • drivers/pg/query/sql/schema_up.sql
✅ Files skipped from review due to trivial changes (1)
  • cypher/models/pgsql/format/format_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • drivers/neo4j/query_rewrite_internal_test.go
  • drivers/neo4j/query_rewrite.go
  • cypher/models/pgsql/translate/renamer.go
  • cypher/models/pgsql/translate/renamer_test.go

Comment thread cypher/models/pgsql/translate/expansion.go
@zinic zinic merged commit 1366719 into SpecterOps:main May 12, 2026
5 of 6 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.

1 participant