Skip to content

fix(duckdb): strip trailing semicolon on unlimited query path#2489

Open
Bartok9 wants to merge 1 commit into
Canner:mainfrom
Bartok9:fix/duckdb-strip-unlimited-query
Open

fix(duckdb): strip trailing semicolon on unlimited query path#2489
Bartok9 wants to merge 1 commit into
Canner:mainfrom
Bartok9:fix/duckdb-strip-unlimited-query

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Strip trailing ; for DuckDB limited and unlimited query() paths.

Motivation

Clients paste terminated SQL. LIMIT path stripped; unlimited did not, so behavior depended on limit presence.

Verification

  • Without fix: unlimited execute keeps ; (test fail)
  • With fix: pytest tests/unit/test_duckdb_semicolon_unlimited.py → 2 passed
  • Apache-2.0 core/**

Files

  • core/wren/src/wren/connector/duckdb.py
  • core/wren/tests/unit/test_duckdb_semicolon_unlimited.py

Summary by CodeRabbit

  • Bug Fixes

    • Improved DuckDB query handling by consistently stripping trailing semicolons and extra trailing whitespace before execution.
    • Ensured behavior is consistent whether a row limit is provided or omitted.
  • Tests

    • Added unit test coverage to verify trailing semicolons are removed for unlimited queries.
    • Added unit test coverage to confirm limited queries are still wrapped appropriately after semicolon stripping.

@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: e820c56a-62c5-40ce-b83c-7b7315ede12b

📥 Commits

Reviewing files that changed from the base of the PR and between abcb3d9 and de4063d.

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

Walkthrough

DuckDBConnector.query now strips trailing semicolons and whitespace before both limited and unlimited execution. New unit tests cover each execution path.

Changes

DuckDB query semicolon handling

Layer / File(s) Summary
Strip SQL before execution
core/wren/src/wren/connector/duckdb.py, core/wren/tests/unit/test_duckdb_semicolon_unlimited.py
DuckDBConnector.query consistently strips trailing SQL terminators, while tests verify unlimited execution and limited subquery wrapping.

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

Possibly related issues

Possibly related PRs

  • Canner/WrenAI#2480: Directly modifies DuckDB query semicolon handling and adds focused tests.
  • Canner/WrenAI#2490: Updates unlimited connector query handling and related semicolon tests.
  • Canner/WrenAI#2407: Applies the same connector SQL sanitization pattern before limited query wrapping.

Suggested reviewers: goldmedal

Poem

A rabbit tidies SQL in the den,
No stray semicolons remain again.
Limited queries wrap up just right,
Unlimited ones run clean and light.
Tests hop along—what a sight!

🚥 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 is accurate and directly reflects the main fix, though it only highlights the unlimited path of a broader DuckDB semicolon-stripping change.
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.

Unlimited execute previously retained terminating ; while the LIMIT
wrap path already stripped. Normalize both paths.
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