Fix sqlite Cursor initialization check by ever0de · Pull Request #6198 · RustPython/RustPython

ever0de · 2025-10-20T15:50:39Z

Add proper init validation for sqlite3.Cursor to ensure base class init is called before using cursor methods. This fixes the test_cursor_constructor_call_check test case.

Changes:

Modified Cursor to initialize with inner=None in py_new
Added explicit init method that sets up CursorInner
Updated close() method to check for uninitialized state
Changed error message to match CPython: 'Base Cursor.init not called.'

Summary by CodeRabbit

Release Notes

New Features
- SQLite Cursor now supports explicit initialization through __init__ method.
Bug Fixes
- Improved error messages for invalid cursor operations, distinguishing between uninitialized and closed cursor states.
- Enhanced cursor closing mechanism with better validation and state management.

coderabbitai · 2025-10-20T15:51:09Z

Walkthrough

Cursor lifecycle management in SQLite module was refactored. A new closed field tracks cursor state. Lazy initialization via __init__ replaces immediate inner construction. The close() method now accepts a VirtualMachine parameter and sets the closed flag. Error messages distinguish between uninitialized and closed states.

Changes

Cohort / File(s)	Summary
Cursor Lifecycle Management `stdlib/src/sqlite.rs`	Added `closed: bool` field to `CursorInner`; introduced `new_uninitialized()` constructor for deferred initialization; added Python-visible `__init__(self, _connection, _vm)` method for lazy inner state setup; updated `close()` signature to accept `VirtualMachine` parameter and return `PyResult<()>`; enforced closed-state checks in cursor operations; refined error messages to distinguish uninitialized ("Base Cursor.init not called.") from closed ("Cannot operate on a closed cursor.") states

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PyNew as py_new
    participant Cursor as Cursor
    participant Init as __init__
    participant Inner as CursorInner

    User->>PyNew: create Cursor
    PyNew->>Cursor: new_uninitialized(conn, vm)
    Cursor-->>User: Cursor (inner=None)
    
    User->>Init: __init__(connection, vm)
    rect rgb(220, 240, 255)
        Note over Init: Lazy Initialization
        Init->>Inner: create with closed=false
        Init->>Cursor: set inner, description, rowcount, etc.
    end
    Init-->>User: initialized
    
    User->>Cursor: execute/fetch operations
    rect rgb(255, 240, 220)
        Note over Cursor: Closed State Check
        alt inner is None
            Cursor-->>User: error: "Base Cursor.__init__ not called."
        else inner.closed is true
            Cursor-->>User: error: "Cannot operate on a closed cursor."
        else normal
            Cursor-->>User: operation result
        end
    end
    
    User->>Cursor: close(vm)
    rect rgb(240, 255, 240)
        Note over Cursor: Cleanup & Mark Closed
        Cursor->>Inner: close statement if present
        Cursor->>Inner: set closed=true
    end
    Cursor-->>User: Ok(())

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes introduce a new cursor initialization pattern with state management across multiple interdependent modifications. Review requires understanding the lazy initialization flow, closed-state enforcement logic, and error-handling refinements, with attention to edge cases around uninitialized vs. closed states.

Suggested reviewers

youknowone

Poem

🐰 A cursor now wakes when init's called,
No rushing to construct—it stands tall!
Closed flags keep watch, state held tight,
Lazy paths shine with pure delight—
SQLite queries hop with grace,
In lifecycle's gentle embrace! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Fix sqlite Cursor initialization check" directly and accurately reflects the main change in this PR. The changeset is specifically focused on adding proper init validation for sqlite3.Cursor to ensure the base class initialization is called before cursor methods are used, which is exactly what the title conveys. The title is concise, specific, and avoids vague terminology—it clearly identifies both the component (sqlite Cursor) and the primary action (fix initialization check). A developer reviewing the commit history would understand from this title alone that the PR addresses initialization validation for the sqlite Cursor.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcf1969 and 5f7a823.

⛔ Files ignored due to path filters (1)

Lib/test/test_sqlite3/test_regression.py is excluded by !Lib/**

📒 Files selected for processing (1)

stdlib/src/sqlite.rs (4 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

stdlib/src/sqlite.rs

{vm,stdlib}/**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

stdlib/src/sqlite.rs

🧬 Code graph analysis (1)

stdlib/src/sqlite.rs (2)

Lib/test/test_sqlite3/test_regression.py (1)

Connection (229-231)

common/src/lock/thread_mutex.rs (2)

map (171-183)

map (238-250)

🔇 Additional comments (7)

stdlib/src/sqlite.rs (7)
1462-1463: LGTM! Appropriate field addition for lifecycle tracking.

The closed field correctly tracks cursor state with the skip traversal attribute since it's a primitive type.

1474-1492: LGTM! Proper initialization of closed state.

The closed: false initialization is correct. This method is used for internal cursor creation (e.g., from Connection.execute()), which appropriately bypasses the Python __init__ mechanism since these cursors are fully initialized.

1494-1501: LGTM! Proper lazy initialization support.

This method correctly creates an uninitialized Cursor with inner=None, enabling the lazy initialization pattern required for proper __init__ validation.

1503-1519: LGTM! Correct implementation of explicit initialization.

The __init__ method properly:

Provides idempotent initialization (safe for multiple calls)

Initializes all CursorInner fields including closed: false

Enables the validation pattern where subclasses must call super().__init__()

1521-1539: LGTM! Proper state validation with correct error messages.

The method correctly:

Distinguishes between uninitialized (inner is None) and closed (closed is true) states

Uses appropriate error messages matching CPython

Safely uses unwrap_unchecked after the is_some() check

Returns the mapped guard only when the cursor is operational

1756-1773: LGTM! Proper close implementation with initialization check.

The updated close() method correctly:

Validates that __init__ was called before allowing close

Resets any active statement

Sets the closed flag

Is idempotent (safe to call multiple times)

Maintains the error message consistent with CPython

The signature change (adding vm parameter) is handled transparently by the #[pymethod] macro for Python callers.

1859-1859: Manual verification complete; cargo verification required locally.

The sandbox environment does not support cargo tooling, preventing automated rustfmt and clippy verification. Manual inspection of the code shows proper Rust formatting and idiomatic patterns.

Please verify locally by running:
cargo fmt --check -- stdlib/src/sqlite.rs
cargo clippy --all-targets -- -D warnings
The conceptual correctness of the new_uninitialized pattern is sound and enables proper lazy initialization for the Python constructor path.

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

Add proper __init__ validation for sqlite3.Cursor to ensure base class __init__ is called before using cursor methods. This fixes the test_cursor_constructor_call_check test case. Changes: - Modified Cursor to initialize with inner=None in py_new - Added explicit __init__ method that sets up CursorInner - Updated close() method to check for uninitialized state - Changed error message to match CPython: 'Base Cursor.__init__ not called.' This ensures CPython compatibility where attempting to use a Cursor instance without calling the base __init__ raises ProgrammingError.

youknowone

👍

ever0de force-pushed the fix/sqlite-cursor-init-check branch from ddc3666 to 1d623e5 Compare October 20, 2025 15:51

ever0de changed the title ~~Fix sqlite3 Cursor initialization check~~ Fix sqlite Cursor initialization check Oct 20, 2025

ever0de force-pushed the fix/sqlite-cursor-init-check branch from 1d623e5 to 5f7a823 Compare October 20, 2025 16:26

ever0de marked this pull request as ready for review October 20, 2025 16:58

youknowone approved these changes Oct 21, 2025

View reviewed changes

youknowone merged commit 25a464e into RustPython:main Oct 21, 2025
12 checks passed

coderabbitai bot mentioned this pull request Oct 21, 2025

Fix sqlite Connection initialization check #6199

Merged

This was referenced Dec 7, 2025

fix(sqlite): Raise ProgrammingError when operating on a closed cursor #6339

Merged

refactor(sqlite): add check_cursor_valid method for cursor state validation #6342

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sqlite Cursor initialization check#6198

Fix sqlite Cursor initialization check#6198
youknowone merged 1 commit intoRustPython:mainfrom
ever0de:fix/sqlite-cursor-init-check

ever0de commented Oct 20, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 20, 2025 •

edited

Loading

Uh oh!

youknowone left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ever0de commented Oct 20, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ever0de commented Oct 20, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 20, 2025 •

edited

Loading