Skip to content

Support SQLAlchemy 2 database operations - #719

Merged
juarezr merged 3 commits into
petl-developers:masterfrom
umd0730:fix/sqlalchemy-2-support
Sep 21, 2026
Merged

juarezr merged 3 commits into
petl-developers:masterfrom
umd0730:fix/sqlalchemy-2-support

Conversation

@umd0730

@umd0730 umd0730 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #648.

SQLAlchemy 2 removes Engine.execute() and rejects raw SQL strings passed to Connection.execute(). As a result, petl rejects engines and fails to read/write through connections; sessions additionally run into autobegun transaction conflicts.

Recognize engines without the removed method, execute driver SQL through exec_driver_sql() when available, and retain execute() for SQLAlchemy expressions and older versions. Session SQL strings use text(). Centralize transaction handling so committed operations handle autobegun transactions, roll back on failure, and use the Session's own commit/rollback methods. commit=False leaves transactions under caller control. Internally opened connections and streaming results are released on completion, failure or iterator close.

Remove the <2.0 bound from the database extra/test requirements and update both MySQL tests' raw SQL calls. Reset the PostgreSQL fixture before its named-cursor case, because the preceding Session case now correctly persists its final row. Document parameter styles, transaction ownership and iterator cleanup.

Compatibility note: commit=True commits the Session through its own API and commits an active connection transaction on SQLAlchemy 1.4/2.x. SQLAlchemy 1.3 connections retain their historical subtransaction behavior when an outer transaction is already active, leaving that outer commit/rollback to the caller. This is documented and tested separately from commit=False, which leaves transaction control to the caller across all versions. Raw strings on an engine/connection use driver parameters; legacy keyword, flat-list and scalar positional forms are normalized for modern execution APIs. Sessions use SQLAlchemy named binds.

Validation (Windows, Python 3.12.14):

  • Before production changes, all 16 initial SQLAlchemy 2 regression cases failed.
  • SQLAlchemy 2.0.54: full suite 674 passed, 16 skipped.
  • SQLAlchemy 1.4.54: full suite 674 passed, 16 skipped, including future-mode cases.
  • SQLAlchemy 1.3.24: full suite 650 passed, 40 skipped; 24 skips are future-mode cases unsupported by 1.3.
  • Added tests cover Engine, Connection and Session read/write/create/drop, append/truncate, parameterized strings/expressions, rollback after an insert failure, caller-owned commit/rollback and early iterator cleanup, using actual file-backed SQLite databases and separate connections.
  • Additional parameter regressions reproduced 14 failures before normalization, then passed. Active-transaction tests verify the documented 1.3 versus 1.4/2.x behavior with commit=True.
  • Ruff on changed Python files passes with the existing E741 in _hasmethods excluded. Staged diff check passes.
  • A development wheel installed with [db] into a fresh environment selected SQLAlchemy 2.0.54. Outside the checkout, the installed package persisted an Engine write and Session append and read both rows back; pip check passed.

The local skips include optional dependencies and external database tests. The runs emitted existing missing-driver/cursor-cleanup warnings; 1.3 also emits a Python 3.12 deprecation warning. Live MySQL/PostgreSQL, other Python versions and the documentation build were not tested locally; the existing upstream CI includes database services and the wider platform matrix.

Upstream validation on 9da2d4f: Test Changes passed all 27 jobs, including the database/platform matrix and documentation build; CodeQL also passed. The initial database CI failures were resolved by updating the remaining MySQL setup query and resetting the PostgreSQL named-cursor fixture after the Session case.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Support SQLAlchemy 2 database operations

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Restores Engine, Connection, and Session operations across SQLAlchemy 1.3 through 2.x.
• Centralizes execution, transaction ownership, rollback, and resource cleanup semantics.
• Adds regression coverage and documents parameter, commit, and iterator behavior.
Diagram

graph TD
  API["Database APIs"] --> Dispatch["Object dispatch"] --> Engine["Engine"] --> Connection["Connection"] --> Execute["SQL router"] --> DB[(Database)]
  Dispatch --> Session["Session"] --> Execute
  API --> Tx["Transaction manager"] --> Connection
  Tx --> Session
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Normalize all SQL strings with text()
  • ➕ Uses one SQLAlchemy execution path for strings and expressions.
  • ➕ Avoids direct dependency on exec_driver_sql capability detection.
  • ➖ Changes engine and connection parameters from driver-native styles to named binds.
  • ➖ Breaks positional raw SQL parameters and existing driver-level behavior.
2. Use begin() scopes for every committed operation
  • ➕ Uses SQLAlchemy's conventional scoped transaction API.
  • ➕ Makes newly created transaction boundaries explicit.
  • ➖ Conflicts with SQLAlchemy 1.4 future-mode and 2.x autobegun transactions.
  • ➖ Cannot preserve caller-controlled transactions when commit is disabled.
  • ➖ May commit or nest through the wrong owner for Session operations.

Recommendation: Keep the capability-based execution router and centralized transaction context. It preserves SQLAlchemy 1.3 compatibility and driver parameter styles while correctly handling SQLAlchemy 2 autobegin, Session ownership, rollback, and commit=False semantics.

Files changed (8) +248 / -60

Bug fix (3) +89 / -57
db.pyModernize SQLAlchemy reads and writes +47/-41

Modernize SQLAlchemy reads and writes

• Routes connection SQL through the compatibility executor, converts Session strings to text expressions, and closes streaming results and engine-owned connections reliably. Writes now use centralized transaction handling and SQLAlchemy 2's driver_connection attribute when available.

petl/io/db.py

db_create.pyApply compatible execution and transactions to DDL +8/-14

Apply compatible execution and transactions to DDL

• Uses shared SQLAlchemy execution and transaction helpers for table creation and deletion. Engine-owned connections are context-managed, while Session commits and rollbacks remain Session-owned.

petl/io/db_create.py

db_utils.pyAdd SQLAlchemy 2 execution and transaction helpers +34/-2

Add SQLAlchemy 2 execution and transaction helpers

• Recognizes engines without Engine.execute(), routes raw strings through exec_driver_sql when supported, and retains execute() for expressions and older versions. Adds transaction handling for active autobegun transactions, rollback failures, Session ownership, and commit=False.

petl/io/db_utils.py

Tests (2) +134 / -1
test_db_server.pyUse a SQLAlchemy expression for MySQL setup +1/-1

Use a SQLAlchemy expression for MySQL setup

• Wraps the MySQL SQL mode statement with sqlalchemy.text() so it remains executable through SQLAlchemy 2 connections.

petl/test/io/test_db_server.py

test_sqlalchemy.pyAdd cross-version SQLAlchemy regression coverage +133/-0

Add cross-version SQLAlchemy regression coverage

• Adds parameterized Engine, Connection, and Session tests across default and future modes. Covers CRUD, raw and expression parameters, rollback recovery, caller-owned transactions, DDL commit control, and early iterator resource cleanup.

petl/test/io/test_sqlalchemy.py

Documentation (1) +23 / -0
io.rstDocument SQLAlchemy compatibility and transaction ownership +23/-0

Document SQLAlchemy compatibility and transaction ownership

• Documents supported SQLAlchemy versions, parameter conventions, default commit behavior, caller-managed transactions, and iterator cleanup. Includes a commit=False transaction example.

docs/io.rst

Other (2) +2 / -2
requirements-database.txtAllow SQLAlchemy 2 in database tests +1/-1

Allow SQLAlchemy 2 in database tests

• Removes the SQLAlchemy <2.0 upper bound from database test requirements while retaining the 1.3.6 minimum.

requirements-database.txt

setup.pyAllow SQLAlchemy 2 in the database extra +1/-1

Allow SQLAlchemy 2 in the database extra

• Removes the SQLAlchemy <2.0 upper bound from the optional database dependency.

setup.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Keyword query parameters now fail ✓ Resolved 🐞 Bug ≡ Correctness
Description
_execute_sqlalchemy forwards raw-query **kwargs directly to exec_driver_sql(), whose keyword
arguments are not named bind parameters. On SQLAlchemy 1.4 and 2.x, calls such as
fromdb(connection, query, id=2) now raise TypeError, and engine-backed reads reach the same
path.
Code

petl/io/db_utils.py[R59-60]

+    if isinstance(query, string_types) and _hasmethod(connection, 'exec_driver_sql'):
+        return connection.exec_driver_sql(query, *args, **kwargs)
Evidence
Connection reads forward all positional and keyword arguments into the new helper, while the
raw-string branch passes those keywords unchanged to exec_driver_sql(). The added tests cover only
mapping and tuple parameter forms, leaving the previously forwarded keyword form unnormalized.

petl/io/db.py[210-218]
petl/io/db_utils.py[56-61]
petl/test/io/test_sqlalchemy.py[62-70]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Raw SQL keyword bind parameters are forwarded as unsupported `exec_driver_sql()` keyword arguments, breaking an argument form previously accepted by SQLAlchemy connections.
## Fix Focus Areas
- petl/io/db_utils.py[56-61]
- petl/io/db.py[210-218]
## Recommended Fix
For raw SQL dispatched through `exec_driver_sql()`, normalize named bind keyword arguments into its single parameters mapping rather than forwarding them as function keywords. Preserve positional driver parameters and add coverage for keyword-parameter reads through both connections and engines.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Version 1.3 writes can stay pending ✓ Resolved 🐞 Bug ≡ Correctness
Description
_sqlalchemy_transaction calls begin() when a connection lacks get_transaction(), so an
already-transactional SQLAlchemy 1.3 connection yields a subtransaction whose commit does not commit
the caller's root transaction. Despite the new documentation promising otherwise, default-commit
writes and table operations remain subject to the caller's later root commit or rollback under that
condition.
Code

docs/io.rst[R438-440]

+Writes commit by default, including any transaction already active on a supplied
+connection or session. Pass ``commit=False`` to leave the transaction under the
+caller's control. For example::
Evidence
The documentation makes an unconditional active-transaction commitment claim, but the helper can
reuse an active transaction only when get_transaction() exists and otherwise invokes begin().
Both data writes and table creation or removal use this helper, while the added caller-owned
transaction test invokes petl only with commit=False and therefore does not verify the claim.

docs/io.rst[438-453]
petl/io/db_utils.py[64-84]
petl/io/db.py[666-677]
petl/io/db_create.py[364-379]
petl/test/io/test_sqlalchemy.py[84-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new documentation says default writes commit every active supplied transaction, but SQLAlchemy 1.3 connections cannot expose the active root transaction through the helper's selected API and instead create a subtransaction.
## Fix Focus Areas
- docs/io.rst[432-453]
- petl/io/db_utils.py[64-84]
- petl/test/io/test_sqlalchemy.py[84-105]
## Recommended Fix
Either implement a version-compatible way to commit an existing SQLAlchemy 1.3 root connection transaction, or explicitly document that callers using an active 1.3 connection retain transaction ownership. Add a test using `commit=True` inside an already-active connection transaction so the documented behavior is verified separately from the existing `commit=False` coverage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread petl/io/db_utils.py Outdated
Comment thread docs/io.rst Outdated
@umd0730

umd0730 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the two review findings in b791edb: legacy keyword, flat-list and scalar positional parameters are normalized, and the documentation now distinguishes SQLAlchemy 1.3 outer-transaction behavior from 1.4/2.x. Added regression tests for both findings (14 parameter cases failed before the fix).

The initial database CI also exposed two test setup issues, fixed in 9da2d4f: a second MySQL setup query still passed a raw string to execute(), and the PostgreSQL named-cursor case needed its own reset after the preceding Session correctly committed its final row. The original empty-table assertions remain intact.

Local full suites pass with SQLAlchemy 2.0/1.4 (674 tests each) and 1.3 (650 tests, with unsupported future-mode cases skipped). The latest test-only follow-up was rerun on 2.0 with 674 passed. The updated upstream database/platform CI is pending.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35565880800

Coverage increased (+0.05%) to 92.169%

Details

  • Coverage increased (+0.05%) from the base build.
  • Patch coverage: 9 uncovered changes across 4 files (197 of 206 lines covered, 95.63%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
petl/test/io/test_sqlalchemy.py 128 122 95.31%
petl/io/db.py 37 36 97.3%
petl/io/db_utils.py 31 30 96.77%
petl/test/io/test_db_server.py 2 1 50.0%
Total (6 files) 206 197 95.63%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 15706
Covered Lines: 14476
Line Coverage: 92.17%
Coverage Strength: 0.92 hits per line

💛 - Coveralls

@juarezr juarezr added Maintainability Issues for code modernization, improve development, testing dependencies Pull requests that update a dependency file labels Sep 21, 2026
@juarezr juarezr self-assigned this Sep 21, 2026
@juarezr

juarezr commented Sep 21, 2026

Copy link
Copy Markdown
Member

@umd0730

  • LGTM
  • I'll review as soon as I get some spare time
  • I'm thinking about the case for running the CI tests for SQLAlchemy 1.4.x in CI, in addition to 2.x

@juarezr

juarezr commented Sep 21, 2026

Copy link
Copy Markdown
Member

@umd0730
* I'm thinking about the case for running the CI tests for SQLAlchemy 1.4.x in CI, in addition to 2.x

The python3.6 in the job matrix already tests SQLAlchemy-1.4.54.

@juarezr
juarezr merged commit ef5a644 into petl-developers:master Sep 21, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file Maintainability Issues for code modernization, improve development, testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support for SQLAlchemy 2

3 participants