Skip to content

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

Closed
Bartok9 wants to merge 1 commit into
Canner:mainfrom
Bartok9:fix/redshift-strip-unlimited-semicolon
Closed

Bartok9 wants to merge 1 commit into
Canner:mainfrom
Bartok9:fix/redshift-strip-unlimited-semicolon

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Redshift only stripped ; when wrapping with LIMIT.
  • Unlimited path executed client terminators as-is.
  • Strip unconditionally, then wrap if limited.

License

core/wren/** Apache-2.0.

Verification

python3 -m pytest core/wren/tests/unit/test_redshift_strip_unlimited_query.py -q

Summary by CodeRabbit

  • Bug Fixes
    • Improved Redshift query execution by consistently handling trailing semicolons, including queries without a row limit.
    • Prevented formatting-related issues when submitting SQL statements to Redshift.
    • Added coverage to verify semicolon handling for unlimited queries.

Unlimited execute left client `;` intact while limited wrap stripped.
Strip first for both paths.
@github-actions github-actions Bot added python Pull requests that update Python code core labels Jul 18, 2026
@Bartok9

Bartok9 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of already-open #2482 (same unlimited-path strip). Sorry for the noise.

@Bartok9 Bartok9 closed this Jul 18, 2026
@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 894b51ea-fe4e-400f-b2a5-57dad818f769

📥 Commits

Reviewing files that changed from the base of the PR and between 3dac00a and dbb497b.

📒 Files selected for processing (2)
  • core/wren/src/wren/connector/redshift.py
  • core/wren/tests/unit/test_redshift_strip_unlimited_query.py

Walkthrough

Redshift query SQL is now stripped of trailing semicolons before optional limit wrapping. A unit test verifies that the unlimited-query path references and applies the stripping behavior.

Changes

Redshift SQL normalization

Layer / File(s) Summary
Normalize and test Redshift queries
core/wren/src/wren/connector/redshift.py, core/wren/tests/unit/test_redshift_strip_unlimited_query.py
query strips trailing semicolons before limit handling, and regression coverage validates the unlimited-query behavior.

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

Possibly related issues

Possibly related PRs

  • Canner/WrenAI#2407 — Applies the same semicolon normalization pattern before SQL wrapping.
  • Canner/WrenAI#2489 — Adds analogous unlimited-query semicolon stripping and regression coverage.
  • Canner/WrenAI#2534 — Aligns connector query() implementations around unconditional trailing-semicolon removal.

Poem

A rabbit found a semicolon bright,
Trailing at the query’s flight.
Strip it clean before limits hop,
Tests stand guard and never stop.
Redshift now runs smooth and neat—
Thump, thump, query complete!

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

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.

1 participant