Skip to content

fix: escape cypher debug statement with comments across newlines BED-9065 - #111

Open
computator wants to merge 3 commits into
mainfrom
BED-9065_cypher-comment-escaping
Open

fix: escape cypher debug statement with comments across newlines BED-9065#111
computator wants to merge 3 commits into
mainfrom
BED-9065_cypher-comment-escaping

Conversation

@computator

@computator computator commented Aug 4, 2026

Copy link
Copy Markdown
Member

Description

Resolves: BED-9065

Type of Change

  • Chore (a change that does not modify the application functionality)
  • Bug fix (a change that fixes an issue)
  • New feature / enhancement (a change that adds new functionality)
  • Refactor (no behaviour change)
  • 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):

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

  • Bug Fixes

    • Improved formatting of generated SQL debug comments across multiline Cypher statements.
    • Prevented escaped identifier content from appearing in executable SQL lines.
  • Tests

    • Added coverage to verify sensitive content remains confined to debug comments and is handled consistently across newline formats.

@computator computator self-assigned this Aug 4, 2026
@computator computator added bug Something isn't working go Pull requests that update go code labels Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89bd23e2-a97a-4c72-88fe-ca85d212f0ed

📥 Commits

Reviewing files that changed from the base of the PR and between 9982564 and c84e935.

📒 Files selected for processing (1)
  • cypher/models/pgsql/translate/format_test.go

Walkthrough

FromCypher now emits trimmed Cypher as newline-safe SQL comments before generating SQL. A test checks newline handling in quoted identifiers.

Changes

Cypher comment safety

Layer / File(s) Summary
Newline-safe comment rewriting and validation
cypher/models/pgsql/translate/format.go, cypher/models/pgsql/translate/format_test.go
FromCypher rewrites CRLF, CR, and LF endings into SQL comment continuations. The test verifies that escaped identifier content remains in comment lines.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit marks each newline’s place,
With comment signs in steady pace.
Cypher stays where comments belong,
SQL follows clean and strong.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 clearly states that the change fixes Cypher debug-comment escaping across newlines and includes the issue identifier.
Description check ✅ Passed The description follows the template and identifies the bug fix, ticket, testing, PostgreSQL impact, and checklist status.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-9065_cypher-comment-escaping

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

@computator
computator marked this pull request as ready for review August 5, 2026 01:00

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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/format_test.go`:
- Around line 30-37: Update the line-validation loop in the format test to
assert that every non-boundary line before the “with s0” sentinel begins with
the “--” comment prefix, rather than only checking that it does not contain
“fail”; retain skipping comment lines and the existing boundary behavior as
appropriate.
🪄 Autofix

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: b62577ef-4fcb-4e59-9a60-f8dceaad389e

📥 Commits

Reviewing files that changed from the base of the PR and between 6638cc2 and 9982564.

📒 Files selected for processing (2)
  • cypher/models/pgsql/translate/format.go
  • cypher/models/pgsql/translate/format_test.go

Comment thread cypher/models/pgsql/translate/format_test.go
"github.com/stretchr/testify/require"
)

func TestFromCypherProperlyEscapesDebugComment(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): It would be nice to add the requested tests for matcher keys containing -- and /* */. The approach does already handle them it seems.

@urangel urangel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants