[Bartok9]

Summary

• Snowflake query(..., limit=N) now appends LIMIT N after stripping a trailing semicolon instead of only slicing the Arrow table client-side.
• dry_run/describe also strip trailing ;.

Motivation

Client-side arrow_table.slice(0, limit) still schedules and materializes a full warehouse result stage for previews. Arrangements already used for Athena/MySQL (append LIMIT) and postgres-family subquery wraps keep the work for previews on the engine. Same helper strips a pure terminal semicolon so describe/execute do not trip multi-statement quirks.

Verification

• core/wren/.venv/bin/python -m pytest tests/unit/test_snowflake_limit.py -q — 4 passed
• Mocked cursor: query("SELECT 1;", limit=2) executes SELECT 1\nLIMIT 2; dry_run describes SELECT 1
• Did NOT change: auth/private_key/session_parameter wiring

License

core/wren/** only (Apache-2.0).

Summary by CodeRabbit

• Bug Fixes

  • Improved Snowflake query handling by pushing requested row limits into the SQL sent to the database, with safeguards to avoid duplicating existing outer limit clauses.
  • Consistently strips trailing semicolons and extra whitespace before executing queries and performing dry runs.
  • Broadened invalid-SQL handling to standardize diagnostics for programming errors while preserving other error behavior.

• Tests

  • Added unit tests validating limit pushdown, semicolon stripping, correct SQL passthrough without limits, and cleaned-SQL behavior for dry runs.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: b1c1866c-c9ea-4043-925a-e029debdbbae

📥 Commits

Reviewing files that changed from the base of the PR and between 42fb2a8 and a94b240.

📒 Files selected for processing (2)

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

🚧 Files skipped from review as they are similar to previous changes (2)

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

---

Walkthrough

The Snowflake connector now cleans SQL, pushes query limits into execution, lazily imports the driver, and conditionally wraps Snowflake programming errors. Focused unit tests cover query, dry-run, and helper behavior.

Changes

Snowflake SQL execution

Layer / File(s)	Summary
SQL preprocessing and connector loading core/wren/src/wren/connector/snowflake.py, core/wren/tests/unit/test_snowflake_limit.py	Adds helpers for stripping trailing semicolons and appending LIMIT, defers the Snowflake import, and tests helper transformations.
Query and dry-run execution core/wren/src/wren/connector/snowflake.py	Executes or describes transformed SQL, avoids Arrow-table slicing for limits, and wraps only Snowflake ProgrammingError exceptions with SQL metadata.
Execution validation coverage core/wren/tests/unit/test_snowflake_limit.py	Mocks connector and cursor state and verifies limited queries, unchanged queries, cleaned dry-run SQL, and helper behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant QueryCaller
  participant SnowflakeConnector
  participant SnowflakeCursor
  QueryCaller->>SnowflakeConnector: query SQL with optional limit
  SnowflakeConnector->>SnowflakeCursor: execute cleaned SQL with LIMIT
  SnowflakeCursor-->>SnowflakeConnector: fetch Arrow result
  SnowflakeConnector-->>QueryCaller: return Arrow table

Loading

Possibly related PRs

• Canner/WrenAI#2268: Modifies the native Snowflake connector’s query and dry-run behavior.
• Canner/WrenAI#2407: Adds related trailing-semicolon stripping and limit handling.
• Canner/WrenAI#2430: Adds similar SQL preprocessing before applying query limits.

Suggested reviewers: goldmedal

Poem

I nibbled semicolons clean,
Pushed limits through the query stream.
The cursor hops, the arrows flow,
Dry runs shed their trailing snow.
Hop, hop—Snowflake’s SQL is keen!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 Snowflake query changes: pushing LIMIT into SQL and stripping trailing semicolons.
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.}

[coderabbitai]

Actionable comments posted: 1

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

Inline comments:
In `@core/wren/src/wren/connector/snowflake.py`:
- Around line 25-32: Update _apply_limit to avoid generating duplicate top-level
row limits: detect an existing top-level LIMIT or FETCH clause before appending,
and preserve the original SQL when one is present; otherwise append the
requested LIMIT after stripping the trailing semicolon. Handle nested subqueries
safely so only the outer query’s limit is considered.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f3c1507b-4feb-4551-a3cc-1cdab0d1a586

📥 Commits

Reviewing files that changed from the base of the PR and between 0ba4fae and b8a9cc9.

📒 Files selected for processing (2)

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

[Bartok9]

Addressed CodeRabbit: _apply_limit now skips pushdown when the outer statement already ends with LIMIT/FETCH (avoids LIMIT…\nLIMIT…). Thanks.

[goldmedal]

The shared CI failure here (maturin failed → cannot 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. 🙏

[Bartok9]

Done — rebased onto latest main to pick up the core/wren-core-py/Cargo.lock fix from #2478. CI should go green now. Thanks @goldmedal! 🙏

[Bartok9]

Superseded by merged #2456 (multiline subquery wrap + trailing-semicolon strip already on main). This branch no longer rebases cleanly and the append-LIMIT approach conflicts with what landed. Closing as superseded.