Skip to content

(fix) cqltypes: fix SyntaxWarning for invalid escape sequence in UDT names - #750

Open
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:fix/cqltypes-strip-frozen-escape-warning
Open

(fix) cqltypes: fix SyntaxWarning for invalid escape sequence in UDT names#750
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:fix/cqltypes-strip-frozen-escape-warning

Conversation

@mykaul

@mykaul mykaul commented Mar 14, 2026

Copy link
Copy Markdown

In cqltype_to_python(), double-quoted UDT names containing backslashes (e.g. '"!@#$%^&*()[]\ frozen >>>") were passed to ast.literal_eval without escaping, producing SyntaxWarning on Python 3.12+:

SyntaxWarning: "\ " is an invalid escape sequence

Escape backslashes before wrapping the token for ast.literal_eval. The reverse operation already exists in python_to_cqltype() at line 159.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@mykaul
mykaul force-pushed the fix/cqltypes-strip-frozen-escape-warning branch from c8797af to c3b8f16 Compare March 14, 2026 21:53
@mykaul
mykaul requested a review from Copilot March 15, 2026 15:40

Copilot AI 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.

Pull request overview

This PR hardens the CQL type parsing utilities to avoid SyntaxWarning when parsing escaped CQL identifiers that contain backslashes, and adds a regression test to ensure the warning does not recur.

Changes:

  • Escape backslashes when converting escaped, double-quoted CQL tokens into a Python literal for ast.literal_eval().
  • Add a unit test that asserts cqltype_to_python() does not emit SyntaxWarning and that python_to_cqltype() round-trips the parsed structure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
cassandra/cqltypes.py Escapes backslashes in escaped-quoted identifier tokens before ast.literal_eval() to prevent invalid-escape SyntaxWarning.
tests/unit/test_metadata.py Adds a regression test covering backslash-containing escaped identifiers and round-trip behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/cqltypes.py Outdated
Comment thread tests/unit/test_metadata.py Outdated
@mykaul
mykaul force-pushed the fix/cqltypes-strip-frozen-escape-warning branch from c3b8f16 to 91cdd32 Compare March 16, 2026 18:02
@mykaul
mykaul requested a review from Copilot March 16, 2026 18:03

Copilot AI 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.

Pull request overview

This PR updates the CQL type string <-> Python representation conversion helpers to correctly handle quoted identifiers containing backslashes and single quotes, and adds unit tests to prevent regressions.

Changes:

  • Escape backslashes and single quotes in cqltype_to_python() to avoid SyntaxWarning/parsing issues during ast.literal_eval.
  • Unescape \' in python_to_cqltype() to support round-tripping identifiers containing '.
  • Add unit tests covering backslash-containing identifiers (no SyntaxWarning) and single-quote-containing identifiers (parse + round-trip).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cassandra/cqltypes.py Adjusts escaping/unescaping in cqltype_to_python and python_to_cqltype for quoted identifiers.
tests/unit/test_metadata.py Adds regression tests for backslash/single-quote handling and round-trip behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cassandra/cqltypes.py
@mykaul mykaul self-assigned this Mar 16, 2026
@Lorak-mmk

Copy link
Copy Markdown

@mykaul This is not a draft, but review was not requested. Please either change to draft, or request review.

@mykaul

mykaul commented May 20, 2026

Copy link
Copy Markdown
Author

@mykaul This is not a draft, but review was not requested. Please either change to draft, or request review.

@Lorak-mmk - who should I ask to review it?

@Lorak-mmk
Lorak-mmk requested review from Lorak-mmk and dkropachev May 20, 2026 10:57
Copilot AI review requested due to automatic review settings July 29, 2026 20:16
@mykaul
mykaul force-pushed the fix/cqltypes-strip-frozen-escape-warning branch from 91cdd32 to d42c4f1 Compare July 29, 2026 20:16
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: aa65c5c1-64ab-400a-8406-faebc109f77e

📥 Commits

Reviewing files that changed from the base of the PR and between d42c4f1 and 4cd9462.

📒 Files selected for processing (2)
  • cassandra/cqltypes.py
  • tests/unit/test_metadata.py
📝 Walkthrough

Walkthrough

CQL type conversion now escapes backslashes and single quotes when converting quoted tokens into Python literals. Reverse conversion unescapes escaped single quotes alongside backslashes. Metadata tests cover round-trip behavior for UDT identifiers containing backslashes and single quotes, including verification that backslash parsing emits no SyntaxWarning.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main fix: preventing SyntaxWarning from invalid escapes in UDT names.
Description check ✅ Passed The description includes a summary and the required checklist, but several items remain unchecked or incomplete.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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.

@mykaul

mykaul commented Jul 29, 2026

Copy link
Copy Markdown
Author

Rebased onto current origin/master (fix commit is now d42c4f1b2); the diff applied cleanly with no conflicts against the current cqltypes.py (the recent ShortType/VectorType/timestamp-precision work landed elsewhere in the file, not in cqltype_to_python/python_to_cqltype).

Verification performed:

  • python3 -W error::SyntaxWarning -c "import cassandra.cqltypes" - clean, no warning.
  • tests/unit/test_metadata.py (57 tests), tests/unit/cqlengine/ (13 tests), and the full tests/unit/ suite (722 passed, 88 skipped, 0 failed) - all green. The only pre-existing SyntaxWarnings in the suite are unrelated, in tests/unit/io/utils.py and tests/unit/test_exception.py.
  • Independently re-checked the earlier Copilot review concern about the blanket .replace("\'", "'") in python_to_cqltype() possibly being lossy for identifiers containing literal backslash-then-quote sequences. Fuzzed round-trips (cqltype_to_python -> python_to_cqltype) with 0-5 consecutive backslashes before a quote, multiple quoted identifiers per expression, and nested frozen/tuple UDTs all round-trip correctly - the escape/unescape ordering (double backslashes first, then escape quotes; and the reverse) is symmetric and lossless for these cases.

Force-pushed the rebase; no code changes were needed beyond the rebase itself.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

cassandra/cqltypes.py:159

  • Reverse the quote escape before collapsing doubled backslashes. With the current order, a valid identifier containing a backslash immediately before an apostrophe (for example map<"a\'b", int>) loses that backslash during a round trip: the first replacement shortens the encoded backslash run, and the second then consumes the remaining backslash as the apostrophe escape.
    cql = ''.join(scanned_tokens).replace('\\\\', '\\').replace("\\'", "'")

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

🧹 Nitpick comments (1)
tests/unit/test_metadata.py (1)

851-885: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for identifiers with multiple apostrophes.

The current single-quote test does not force repr(types) to change quote styles. Add a regression such as map<"a'b'c'd", int> and a mixed backslash/apostrophe case to cover the reverse-conversion boundary.

🤖 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 `@tests/unit/test_metadata.py` around lines 851 - 885, Extend
test_cqltype_single_quote_in_identifier with an identifier containing multiple
apostrophes, such as map<"a'b'c'd", int>, and assert parsing plus round-trip
preservation. Add a separate mixed backslash/apostrophe regression case to
exercise the python_to_cqltype reverse-conversion boundary, retaining the
existing result-structure and round-trip assertions.
🤖 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 `@tests/unit/test_metadata.py`:
- Around line 851-885: Extend test_cqltype_single_quote_in_identifier with an
identifier containing multiple apostrophes, such as map<"a'b'c'd", int>, and
assert parsing plus round-trip preservation. Add a separate mixed
backslash/apostrophe regression case to exercise the python_to_cqltype
reverse-conversion boundary, retaining the existing result-structure and
round-trip assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 448070d7-6690-4edd-ae0a-ad5dcc294143

📥 Commits

Reviewing files that changed from the base of the PR and between b8b714c and d42c4f1.

📒 Files selected for processing (2)
  • cassandra/cqltypes.py
  • tests/unit/test_metadata.py

In cqltype_to_python(), double-quoted UDT names containing backslashes
or single quotes were passed to ast.literal_eval without proper escaping,
producing SyntaxWarning on Python 3.12+ or SyntaxError for single quotes.

Escape both backslashes and single quotes before wrapping the token for
ast.literal_eval. The reverse operation in python_to_cqltype() is updated
to unescape both sequences as well.
@mykaul
mykaul force-pushed the fix/cqltypes-strip-frozen-escape-warning branch from d42c4f1 to 4cd9462 Compare September 13, 2026 11:09
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