Skip to content

fix(snowflake): strip trailing semicolon before dry_run describe - #2487

Merged
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/snowflake-dry-run-strip-semicolon
Jul 20, 2026
Merged

fix(snowflake): strip trailing semicolon before dry_run describe#2487
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/snowflake-dry-run-strip-semicolon

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Strip trailing semicolon before Snowflake cursor.describe dry runs.
  • Mirrors the existing _strip_trailing_semicolon used for query LIMIT wrap.

Motivation

SQL clients often terminate statements with ;. describe("SELECT 1;") fails with ProgrammingError while query already strips. Dry-run then reports invalid SQL for valid statements.

Verification

  • Without fix: dry_run describe keeps trailing ; (test AssertionError)
  • With fix: pytest tests/unit/test_snowflake_limit_pushdown.py → 6 passed
  • Path: Apache-2.0 core/**

Files

  • core/wren/src/wren/connector/snowflake.py
  • core/wren/tests/unit/test_snowflake_limit_pushdown.py

Summary by CodeRabbit

  • Bug Fixes
    • Fixed Snowflake SQL validation for statements ending with a semicolon.
    • Preserved semicolons inside quoted string values during validation.
  • Tests
    • Added coverage to verify trailing-semicolon handling and preservation of semicolons within SQL literals.

Snowflake ProgrammingError on trailing ; for cursor.describe matches
query/limit composition strip already used on the execute path.
@github-actions github-actions Bot added python Pull requests that update Python code core labels Jul 13, 2026
@coderabbitai

coderabbitai Bot commented Jul 13, 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: 9e3c29f7-993d-47d6-b783-efb6798eb2c1

📥 Commits

Reviewing files that changed from the base of the PR and between f0270c1 and fb32151.

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

Walkthrough

Snowflake dry runs now strip trailing semicolons and whitespace before calling cursor.describe. Unit tests verify trailing terminator removal while preserving semicolons inside string literals.

Changes

Snowflake dry-run handling

Layer / File(s) Summary
Clean SQL before description
core/wren/src/wren/connector/snowflake.py, core/wren/tests/unit/test_snowflake_limit_pushdown.py
dry_run preprocesses SQL before cursor.describe; tests cover trailing semicolon removal and preservation of semicolons inside literals.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • Canner/WrenAI#2407: Applies equivalent trailing-semicolon handling in another connector’s dry_run path.
  • Canner/WrenAI#2420: Adds matching SQL cleanup and interior-semicolon coverage for Redshift.
  • Canner/WrenAI#2430: Uses _strip_trailing_semicolon before downstream SQL processing.

Suggested reviewers: goldmedal

Poem

I’m a rabbit who trims SQL with care,
Leaving inner semicolons nestled there.
Snowflake now describes a cleaner line,
While tests guard every literal sign.
Hop, hop—the dry run’s fine!

🚥 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 clearly summarizes the main Snowflake dry_run fix: stripping a trailing semicolon before describe.
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
goldmedal merged commit 724c9bd into Canner:main Jul 20, 2026
10 checks passed
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