Skip to content

fix(redshift): strip trailing semicolon on unlimited query path - #2482

Merged
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/redshift-strip-on-unlimited-query
Jul 22, 2026
Merged

fix(redshift): strip trailing semicolon on unlimited query path#2482
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/redshift-strip-on-unlimited-query

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Redshift query() now strips a terminating semicolon/whitespace run even when limit is None.
  • Keeps behavior aligned with the limited-query wrap path and other connectors that sanitize client SQL before execute.

Motivation

Limited Redshift queries already subquery-wrap after stripping trailing ;. The unlimited path executed raw SQL, so client SQL ending with ; could still fail depending on session/driver batching. Strip for both paths (string-literal-safe terminating-run regex already used by this connector).

Verification

  • core/wren/.venv/bin/python -m pytest tests/unit/test_redshift_semicolon.py -q — 5 passed
  • Real behavior: query("SELECT 1;") sends SELECT 1 on the unlimited path.

Apache path: core/wren/**.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Redshift query execution when no row limit is specified by stripping trailing semicolons and surrounding whitespace.
    • Ensures submitted SQL is normalized consistently whether a limit is provided or not.
  • Tests
    • Added a unit test to verify unlimited queries don’t send SQL that ends with ;.

@github-actions github-actions Bot added python Pull requests that update Python code core labels Jul 12, 2026
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 3b17dfd2-b3bb-491b-8894-ab245aeb4324

📥 Commits

Reviewing files that changed from the base of the PR and between d81554b and 8fda05b.

📒 Files selected for processing (2)
  • core/wren/src/wren/connector/redshift.py
  • core/wren/tests/unit/test_redshift_semicolon.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • core/wren/tests/unit/test_redshift_semicolon.py
  • core/wren/src/wren/connector/redshift.py

Walkthrough

RedshiftConnector.query now strips trailing semicolons and whitespace when no limit is provided. A unit test verifies that SELECT 1; is executed as SELECT 1.

Changes

Redshift query normalization

Layer / File(s) Summary
Normalize unlimited queries
core/wren/src/wren/connector/redshift.py, core/wren/tests/unit/test_redshift_semicolon.py
The unlimited query path strips trailing semicolons, and a unit test verifies the SQL sent to the cursor.

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

Possibly related issues

Possibly related PRs

  • Canner/WrenAI#2407: Related connector SQL handling removes trailing semicolons before execution or wrapping.
  • Canner/WrenAI#2420: Directly modifies Redshift SQL normalization and adds semicolon-related tests.
  • Canner/WrenAI#2430: Applies similar trailing-semicolon handling in another connector.

Poem

A rabbit found a semicolon stray,
And hopped it neatly out of the way.
Redshift runs clean,
Tests guard the scene—
No extra mark delays the day.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: stripping a trailing semicolon on the unlimited Redshift query path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

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

@goldmedal

Copy link
Copy Markdown
Collaborator

The shared CI failure here (maturin failedcannot update the lock file core/wren-core-py/Cargo.lock because --locked was passed) came from the Rust core 0.2.0 bump in #2467 leaving core/wren-core-py/Cargo.lock out of sync. That's now fixed on main by #2478 — please rebase onto latest main to pick up the lockfile fix and turn CI green. 🙏

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

Labels

core python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants