Fix backtick stripping in Cypher map literal keys - #3322
Conversation
Summary of ChangesHello @ExtReMLapin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue in the Cypher parser where map literal keys enclosed in backticks were not being properly unquoted. This fix ensures that keys like Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue with backticked keys in Cypher map literals by introducing and applying the stripBackticks utility method. The change to make stripBackticks static is a good design choice for reusability, and the new tests provide solid coverage for the fixed scenarios. However, the fix appears to be partial, as similar logic is needed in other parts of the parser to ensure consistent handling of backticked keys across all map-like constructs. I've added a specific comment detailing the missed locations.
🧪 CI InsightsHere's what we observed from your CI run for 42b9260. 🟢 All jobs passed!But CI Insights is watching 👀 |
|
Claude wrote all of this. Fixes #3321 |
5e4d1d3 to
1c64267
Compare
Fixed an issue where backticks in map literal keys and property names
were being included instead of being properly stripped. Applied the fix
consistently across all map-related and property access parsing contexts.
Changes:
- Made CypherASTBuilder.stripBackticks() static and package-private
to allow access from CypherExpressionBuilder
- Updated CypherExpressionBuilder.parseMapLiteralExpression() to strip
backticks from map keys (RETURN clause map literals)
- Updated CypherExpressionBuilder.parseMapProperties() to strip
backticks from map keys (pattern properties)
- Updated CypherASTBuilder.visitMap() to strip backticks from map keys
(CREATE/MERGE clause map literals)
- Updated CypherExpressionBuilder.parseMapProjection() to strip
backticks from both explicit keys and property names (map projections)
- Updated CypherExpressionBuilder.parseExpression2WithPostfix() to strip
backticks from property names in property access expressions
- Updated CypherExpressionBuilder.parseExpressionText() to strip
backticks from property names in text-based parsing
- Added comprehensive test cases in CypherMapBackticksTest to verify:
- Single backticked key in RETURN (e.g., `@rid`)
- Multiple backticked keys in RETURN
- Escaped backticks within keys (e.g., `key``with``backticks`)
- Backticked keys in CREATE clause
- Backticked keys in map projections
- Property access with backticked names (e.g., n.`@id`)
Example queries that now work correctly:
- RETURN: collect({`@rid`: ID(c), text: c.text})
- CREATE: CREATE (n {`@special`: 'value'})
- Map projection: n{.name, `@id`: n.id}
- Property access: RETURN n.`@special`
Before: keys/properties were stored/accessed as "`@rid`" (with backticks)
After: keys/properties are stored/accessed as "@Rid" (without backticks)
https://claude.ai/code/session_01ATpxvdUh9HNtuBW7x9NmLT
1c64267 to
42b9260
Compare
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3322 +/- ##
==========================================
- Coverage 56.61% 56.58% -0.03%
==========================================
Files 1368 1368
Lines 100618 100618
Branches 20486 20486
==========================================
- Hits 56961 56934 -27
- Misses 34538 34545 +7
- Partials 9119 9139 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…3322) Fixed an issue where backticks in map literal keys and property names were being included instead of being properly stripped. Applied the fix consistently across all map-related and property access parsing contexts. Changes: - Made CypherASTBuilder.stripBackticks() static and package-private to allow access from CypherExpressionBuilder - Updated CypherExpressionBuilder.parseMapLiteralExpression() to strip backticks from map keys (RETURN clause map literals) - Updated CypherExpressionBuilder.parseMapProperties() to strip backticks from map keys (pattern properties) - Updated CypherASTBuilder.visitMap() to strip backticks from map keys (CREATE/MERGE clause map literals) - Updated CypherExpressionBuilder.parseMapProjection() to strip backticks from both explicit keys and property names (map projections) - Updated CypherExpressionBuilder.parseExpression2WithPostfix() to strip backticks from property names in property access expressions - Updated CypherExpressionBuilder.parseExpressionText() to strip backticks from property names in text-based parsing - Added comprehensive test cases in CypherMapBackticksTest to verify: - Single backticked key in RETURN (e.g., `@rid`) - Multiple backticked keys in RETURN - Escaped backticks within keys (e.g., `key``with``backticks`) - Backticked keys in CREATE clause - Backticked keys in map projections - Property access with backticked names (e.g., n.`@id`) Before: keys/properties were stored/accessed as "`@rid`" (with backticks) After: keys/properties are stored/accessed as "@Rid" (without backticks) (cherry picked from commit ada9a89)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.31.4 to 4.31.5. Release notes *Sourced from [github/codeql-action's releases](https://github.com/github/codeql-action/releases).* > v4.31.5 > ------- > > CodeQL Action Changelog > ======================= > > See the [releases page](https://github.com/github/codeql-action/releases) for the relevant changes to the CodeQL CLI and language packs. > > 4.31.5 - 24 Nov 2025 > -------------------- > > * Update default CodeQL bundle version to 2.23.6. [ArcadeData#3321](https://redirect.github.com/github/codeql-action/pull/3321) > > See the full [CHANGELOG.md](https://github.com/github/codeql-action/blob/v4.31.5/CHANGELOG.md) for more information. Changelog *Sourced from [github/codeql-action's changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md).* > CodeQL Action Changelog > ======================= > > See the [releases page](https://github.com/github/codeql-action/releases) for the relevant changes to the CodeQL CLI and language packs. > > [UNRELEASED] > ------------ > > No user facing changes. > > 4.31.5 - 24 Nov 2025 > -------------------- > > * Update default CodeQL bundle version to 2.23.6. [ArcadeData#3321](https://redirect.github.com/github/codeql-action/pull/3321) > > 4.31.4 - 18 Nov 2025 > -------------------- > > No user facing changes. > > 4.31.3 - 13 Nov 2025 > -------------------- > > * CodeQL Action v3 will be deprecated in December 2026. The Action now logs a warning for customers who are running v3 but could be running v4. For more information, see [Upcoming deprecation of CodeQL Action v3](https://github.blog/changelog/2025-10-28-upcoming-deprecation-of-codeql-action-v3/). > * Update default CodeQL bundle version to 2.23.5. [ArcadeData#3288](https://redirect.github.com/github/codeql-action/pull/3288) > > 4.31.2 - 30 Oct 2025 > -------------------- > > No user facing changes. > > 4.31.1 - 30 Oct 2025 > -------------------- > > * The `add-snippets` input has been removed from the `analyze` action. This input has been deprecated since CodeQL Action 3.26.4 in August 2024 when this removal was announced. > > 4.31.0 - 24 Oct 2025 > -------------------- > > * Bump minimum CodeQL bundle version to 2.17.6. [ArcadeData#3223](https://redirect.github.com/github/codeql-action/pull/3223) > * When SARIF files are uploaded by the `analyze` or `upload-sarif` actions, the CodeQL Action automatically performs post-processing steps to prepare the data for the upload. Previously, these post-processing steps were only performed before an upload took place. We are now changing this so that the post-processing steps will always be performed, even when the SARIF files are not uploaded. This does not change anything for the `upload-sarif` action. For `analyze`, this may affect Advanced Setup for CodeQL users who specify a value other than `always` for the `upload` input. [ArcadeData#3222](https://redirect.github.com/github/codeql-action/pull/3222) > > 4.30.9 - 17 Oct 2025 > -------------------- > > * Update default CodeQL bundle version to 2.23.3. [ArcadeData#3205](https://redirect.github.com/github/codeql-action/pull/3205) > * Experimental: A new `setup-codeql` action has been added which is similar to `init`, except it only installs the CodeQL CLI and does not initialize a database. Do not use this in production as it is part of an internal experiment and subject to change at any time. [ArcadeData#3204](https://redirect.github.com/github/codeql-action/pull/3204) > > 4.30.8 - 10 Oct 2025 > -------------------- > > No user facing changes. > > 4.30.7 - 06 Oct 2025 > -------------------- > > * [v4+ only] The CodeQL Action now runs on Node.js v24. [ArcadeData#3169](https://redirect.github.com/github/codeql-action/pull/3169) > > 3.30.6 - 02 Oct 2025 > -------------------- > > * Update default CodeQL bundle version to 2.23.2. [ArcadeData#3168](https://redirect.github.com/github/codeql-action/pull/3168) ... (truncated) Commits * [`fdbfb4d`](github/codeql-action@fdbfb4d) Merge pull request [ArcadeData#3322](https://redirect.github.com/github/codeql-action/issues/3322) from github/update-v4.31.5-ec2ee575c * [`81f6d64`](github/codeql-action@81f6d64) Update changelog for v4.31.5 * [`ec2ee57`](github/codeql-action@ec2ee57) Merge pull request [ArcadeData#3321](https://redirect.github.com/github/codeql-action/issues/3321) from github/update-bundle/codeql-bundle-v2.23.6 * [`ecc8787`](github/codeql-action@ecc8787) Add changelog note * [`1d2a238`](github/codeql-action@1d2a238) Update default bundle to codeql-bundle-v2.23.6 * [`ce729e4`](github/codeql-action@ce729e4) Merge pull request [ArcadeData#3315](https://redirect.github.com/github/codeql-action/issues/3315) from github/henrymercer/dead-code-elimination * [`ac359aa`](github/codeql-action@ac359aa) Add return type * [`112cd07`](github/codeql-action@112cd07) Merge branch 'main' into henrymercer/dead-code-elimination * [`0b43179`](github/codeql-action@0b43179) Merge pull request [ArcadeData#3306](https://redirect.github.com/github/codeql-action/issues/3306) from github/dependabot/npm\_and\_yarn/types/sinon-21.0.0 * [`e818008`](github/codeql-action@e818008) Merge pull request [ArcadeData#3305](https://redirect.github.com/github/codeql-action/issues/3305) from github/dependabot/npm\_and\_yarn/eslint/compat-2.0.0 * Additional commits viewable in [compare view](github/codeql-action@e12f017...fdbfb4d) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
…queries (ArcadeData#3322) Fixed an issue where backticks in map literal keys and property names were being included instead of being properly stripped. Applied the fix consistently across all map-related and property access parsing contexts. Changes: - Made CypherASTBuilder.stripBackticks() static and package-private to allow access from CypherExpressionBuilder - Updated CypherExpressionBuilder.parseMapLiteralExpression() to strip backticks from map keys (RETURN clause map literals) - Updated CypherExpressionBuilder.parseMapProperties() to strip backticks from map keys (pattern properties) - Updated CypherASTBuilder.visitMap() to strip backticks from map keys (CREATE/MERGE clause map literals) - Updated CypherExpressionBuilder.parseMapProjection() to strip backticks from both explicit keys and property names (map projections) - Updated CypherExpressionBuilder.parseExpression2WithPostfix() to strip backticks from property names in property access expressions - Updated CypherExpressionBuilder.parseExpressionText() to strip backticks from property names in text-based parsing - Added comprehensive test cases in CypherMapBackticksTest to verify: - Single backticked key in RETURN (e.g., `@rid`) - Multiple backticked keys in RETURN - Escaped backticks within keys (e.g., `key``with``backticks`) - Backticked keys in CREATE clause - Backticked keys in map projections - Property access with backticked names (e.g., n.`@id`) Before: keys/properties were stored/accessed as "`@rid`" (with backticks) After: keys/properties are stored/accessed as "@Rid" (without backticks)
What does this PR do?
This PR fixes an issue where backticks in map literal keys were not being properly stripped during Cypher query parsing. The
stripBackticksmethod inCypherASTBuilderis now made static and called when parsing map literals inCypherExpressionBuilder, ensuring that backticked keys (e.g.,`@rid`) are correctly converted to their unquoted form (e.g.,@rid).Motivation
When using map literals in Cypher queries with backticked keys (commonly needed for special characters like
@), the backticks were being retained in the resulting map keys. This caused queries like:to produce maps with keys like
`@rid`instead of@rid, breaking downstream code that expected the unquoted key names.Related issues
This fix addresses the handling of backticked identifiers in map literals, which is part of the Cypher specification for escaping reserved words and special characters.
Additional Notes
stripBackticksmethod is nowstaticto allow reuse across different builder classesCypherExpressionBuildernow call this method:parseMapLiteralExpressionandparseMapProperties→` ``)Checklist
mvn clean packagecommandhttps://claude.ai/code/session_01ATpxvdUh9HNtuBW7x9NmLT