chore: unpin duckdb by tokoko · Pull Request #296 · substrait-io/substrait-python

tokoko · 2026-10-08T17:47:03Z

drops the duckdb version pin from dev dependencies.

coderabbitai · 2026-10-08T17:47:22Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: substrait-io/substrait-python/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 9f7bb943-5b88-473b-8bb9-41ed145f6344

📥 Commits

Reviewing files that changed from the base of the PR and between a6aa7c6 and eb9b4a8.

⛔ Files ignored due to path filters (2)

pixi.lock is excluded by !**/*.lock
uv.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

examples/duckdb_example.py
pyproject.toml
tests/integration/test_sql_engine_roundtrip.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

Improvements
- Updated the DuckDB example to work with a newer DuckDB release and handle extension installation errors more reliably.
Tests
- Expanded DuckDB integration checks for schema and query-result retrieval, table creation from test data, and limited queries across supported engines.

Walkthrough

The example and development dependency declarations update DuckDB versions or constraints. The integration tests use to_arrow_table(), create tables from test data, and include DuckDB in the affected engines parameterizations.

Changes

DuckDB Updates

Layer / File(s)	Summary
Update DuckDB dependency declarations `examples/duckdb_example.py`, `pyproject.toml`	The example pins DuckDB to version `1.5.6` and catches `duckdb.HTTPException`. The development dependency no longer has a version or Python-version constraint.
Update DuckDB integration tests `tests/integration/test_sql_engine_roundtrip.py`	The tests use `to_arrow_table()` for schema and result conversion, create `stores` and `sales` tables from test data, and use `engines` instead of `engines_duckdb_xfail` for the affected tests.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: nielspardon

Merge Risk: ⚪ Minimal · up to eb9b4

The dependency update and example fallback are compatible with the supported Python versions. No concrete integration failure is established, so no specific merge blocker remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: removing the DuckDB version pin. It uses a valid Conventional Commit format.
Description check		The description states the primary rationale and matches the pull request objective. It is brief but sufficiently relevant for this small change.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

… into chore/unpin-duckdb # Conflicts: # pixi.lock # pyproject.toml

nielspardon · 2026-10-09T06:12:30Z

@coderabbitai resume

coderabbitai · 2026-10-09T06:12:38Z

✅ Action performed

Reviews resumed and review finished.

nielspardon

Please regenerate pixi.lock with pixi v0.73.0 or newer (the version CI pins): the merge resolution rewrote it in the older v6 lock format, so pixi lock --check fails and the next pixi command silently rewrites it to v7. Running pixi lock on this branch fixes it and only changes the format.

nielspardon · 2026-10-09T10:22:23Z

+        conn.execute("CREATE TABLE stores AS SELECT * FROM data")
+        conn.execute("CREATE TABLE sales AS SELECT * FROM sales_data")


Pass the Arrow tables in explicitly so this doesn't depend on DuckDB finding data by Python variable name:

Suggested change

conn.execute("CREATE TABLE stores AS SELECT * FROM data")

conn.execute("CREATE TABLE sales AS SELECT * FROM sales_data")

conn.from_arrow(data).create("stores")

conn.from_arrow(sales_data).create("sales")

nielspardon · 2026-10-09T10:22:23Z

 try:
    duckdb.install_extension("substrait")
-except duckdb.duckdb.HTTPException:
+except duckdb.HTTPException:


Use .to_arrow_table().schema at examples/duckdb_example.py:34 too, to match the tests (that line isn't in the diff, so no suggestion block).

nielspardon · 2026-10-09T10:22:23Z


 [dependency-groups]
-dev = ["pytest >= 7.0.0", "substrait-antlr==0.102.0", "pyyaml", "sqloxide", "deepdiff", "duckdb<=1.2.2; python_version < '3.14'", "datafusion"]
+dev = ["pytest >= 7.0.0", "substrait-antlr==0.102.0", "pyyaml", "sqloxide", "deepdiff", "duckdb", "datafusion"]


Would a range like duckdb>=1.5,<1.6 be worth keeping here? The old pin guarded against a lock upgrade landing on a DuckDB release the substrait community extension doesn't support yet.

chore: unpin duckdb

a6aa7c6

tokoko added 2 commits October 8, 2026 21:52

fix: duckdb http exception

14f63d8

Merge branch 'main' of https://github.com/substrait-io/substrait-python…

eb9b4a8

… into chore/unpin-duckdb # Conflicts: # pixi.lock # pyproject.toml

nielspardon reviewed Oct 9, 2026

View reviewed changes

Navigation Menu

Repository navigation

chore: unpin duckdb - #296

chore: unpin duckdb#296
tokoko wants to merge 3 commits into
substrait-io:mainfrom
tokoko:chore/unpin-duckdb

tokoko commented Oct 8, 2026

Uh oh!

coderabbitai Bot commented Oct 8, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nielspardon commented Oct 9, 2026

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

nielspardon left a comment

Uh oh!

nielspardon Oct 9, 2026

Uh oh!

nielspardon Oct 9, 2026

Uh oh!

nielspardon Oct 9, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		conn.execute("CREATE TABLE stores AS SELECT * FROM data")
		conn.execute("CREATE TABLE sales AS SELECT * FROM sales_data")

Conversation

tokoko commented Oct 8, 2026

Uh oh!

coderabbitai Bot commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

nielspardon commented Oct 9, 2026

Uh oh!

coderabbitai Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nielspardon left a comment

Choose a reason for hiding this comment

Uh oh!

nielspardon Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

nielspardon Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

nielspardon Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Oct 8, 2026 •

edited

Loading

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading