fix(redshift): strip trailing semicolon before subquery-wrapping

[Bartok9]

Summary

• The Redshift connector wraps user SQL as SELECT * FROM (...) AS _q LIMIT N (in both query() and dry_run()). When the user SQL ends in a ;, Redshift rejects it inside the subquery with syntax error at or near ";".
• User-facing impact: any saved query / preview SQL that ends in a semicolon fails on Redshift even though it runs fine standalone.

Motivation

Mirrors the postgres fix in #2407 (merged). The same subquery-wrap idiom is used across the postgres/canner/trino/clickhouse connectors, all of which already strip a trailing semicolon before wrapping. Redshift was the remaining connector that wrapped without stripping.

The fix adds _strip_trailing_semicolon() (identical regex [;\s]+\Z to postgres) and applies it in both query() and dry_run(). Only the terminating run of semicolons/whitespace is stripped, so semicolons inside string literals (SELECT 'a;b') are preserved.

Verification

• uv run --no-sync pytest tests/unit/test_redshift_semicolon.py -q — 4 passed.

Real behavior proof

Regression test core/wren/tests/unit/test_redshift_semicolon.py asserts the executed SQL and FAILS without the fix.

Without the fix (helper absent):

ImportError: cannot import name '_strip_trailing_semicolon' from 'wren.connector.redshift'
1 error in 0.41s

With the fix:

....                                                                     [100%]
4 passed in 0.34s

The query test asserts the executed SQL is exactly SELECT * FROM (SELECT 1) AS _q LIMIT 5 (no ;)), proving the semicolon is stripped before wrapping.

Scope / risk

• Touches only core/wren/src/wren/connector/redshift.py (Apache-2.0 path) + a new unit test.
• Does NOT touch type parsing, connection logic, or other connectors.
• String-literal semicolons preserved (covered by a helper test).

Summary by CodeRabbit

• Bug Fixes
  • Improved Redshift query handling so SQL with a trailing semicolon now runs correctly when wrapped for limits or dry runs.
  • Preserved semicolons inside quoted text while removing only unnecessary trailing semicolons and whitespace.
• Tests
  • Added coverage for trailing-semicolon handling in query execution and dry runs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5b3da18d-28be-4b51-98cb-c65b86400e69

📥 Commits

Reviewing files that changed from the base of the PR and between 3122f4f and ec3d696.

📒 Files selected for processing (2)

• core/wren/src/wren/connector/redshift.py
• core/wren/tests/unit/test_redshift_semicolon.py

---

Walkthrough

Adds a _strip_trailing_semicolon() regex helper to redshift.py that removes trailing semicolons and whitespace from SQL strings. Both query() and dry_run() are updated to apply this helper before wrapping user SQL in a subquery. A new unit test file validates the helper and the two call sites using a mocked connector.

Redshift trailing semicolon fix

Layer / File(s)	Summary
Helper implementation and call sites core/wren/src/wren/connector/redshift.py	Adds re import, _TRAILING_SEMICOLONS_RE module-level regex, and _strip_trailing_semicolon(sql) helper; applies it in query() and dry_run() before constructing SELECT * FROM (...) LIMIT subqueries.
Unit tests core/wren/tests/unit/test_redshift_semicolon.py	Adds mock connector factory and four tests covering query(), dry_run(), semicolon-inside-literal preservation, and no-trailing-semicolon passthrough.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Canner/WrenAI#2407: Introduces the identical trailing-semicolon stripping helper and applies it in query() and dry_run() in the Postgres connector — the direct parallel to this Redshift change.

Suggested reviewers

• goldmedal

🐇 A semicolon snuck to the end of the line,
But the rabbit said, "No! You shan't subquery-combine!"
With a regex so clever, it snipped off the tail,
Now SELECT * FROM (...) will never fail.
Hop hop, clean SQL prevails! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main Redshift fix and matches the changed behavior.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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