fix(sqlite): Raise ProgrammingError when operating on a closed cursor by ever0de · Pull Request #6339 · RustPython/RustPython

coderabbitai

Walkthrough

This change enhances SQLite cursor state management by introducing eager initialization in executescript and refactoring the Cursor::close method to properly enforce cursor closure state. The modifications ensure inner state is initialized before operations and handle statement cleanup consistently.

Changes

Cohort / File(s)	Summary
SQLite Cursor State Management `crates/stdlib/src/sqlite.rs`	Modified `Cursor::executescript` to eagerly initialize inner state via `zelf.clone().inner(vm)?`. Refactored `Cursor::close` to conditionally reset statement when inner is present and consistently mark cursor as closed, removing duplicate guard checking.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Guard handling logic: Verify the guard branch in Cursor::close properly enforces inner state presence and cleanly handles statement reset.
Initialization flow: Confirm eager inner(vm)? call in executescript aligns with the closed cursor ProgrammingError requirement from issue #6336.
State consistency: Ensure inner.closed is reliably set after statement operations and that no code paths bypass closure checks.

Possibly related PRs

Fix sqlite Cursor initialization check #6198: Modifies SQLite Cursor initialization and close semantics for consistent state management.
Raise UnicodeEncodeError for surrogates in sqlite.executescript #6024: Also adds an early pre-check in Cursor::executescript, complementing initialization validation.
Fix sqlite connection reinitialization #6288: Refactors SQLite connection/cursor internal state initialization and guarding for improved closure handling.

Suggested reviewers

youknowone
ShaharNaveh

Poem

🐰 A cursor once closed tried to dance,
But now we enforce closure's trance,
Guard checks are tidy, state stays true,
No more slipping through the rift anew!
✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The PR addresses issue #6336 by implementing guard checks for cursor closure and reworking Cursor::close behavior, but the summary shows only foundational changes without evidence of full implementation across all required methods.	Verify that ProgrammingError is properly raised in all target methods (execute, executemany, executescript, fetchone, fetchall, etc.) and confirm the ClosedCurTests::test_closed passes.

✅ 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 accurately summarizes the main change: raising ProgrammingError when operating on a closed cursor, matching the PR's primary objective.
Out of Scope Changes check	✅ Passed	The changes are narrowly focused on sqlite.rs with modifications to executescript and Cursor::close, directly supporting the stated objective of enforcing closed cursor checks.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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 and usage tips.}