Skip to content

Fix SQLite large integer overflow error handling #5916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Jul 9, 2025
Merged

Fix SQLite large integer overflow error handling #5916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Lib/test/test_sqlite3/test_userfunctions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,6 @@ def test_nan_float(self):
# SQLite has no concept of nan; it is converted to NULL
self.assertTrue(cur.fetchone()[0])

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_too_large_int(self):
err = "Python int too large to convert to SQLite INTEGER"
self.assertRaisesRegex(OverflowError, err, self.con.execute,
Expand Down
4 changes: 3 additions & 1 deletion stdlib/src/sqlite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2609,7 +2609,9 @@ mod _sqlite {
let ret = if vm.is_none(obj) {
unsafe { sqlite3_bind_null(self.st, pos) }
} else if let Some(val) = obj.payload::<PyInt>() {
let val = val.try_to_primitive::<i64>(vm)?;
let val = val.try_to_primitive::<i64>(vm).map_err(|_| {
vm.new_overflow_error("Python int too large to convert to SQLite INTEGER")
})?;
Comment on lines +2612 to +2614
Copy link

@gemini-code-assist gemini-code-assist bot Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Returning a new error without providing context about the original error makes debugging difficult. It would be helpful to include the original error in the new error message using thiserror::Error or similar.

Consider using map_err with ? to propagate the original error.

                let val = val.try_to_primitive::<i64>(vm).map_err(|e| {
                    vm.new_overflow_error(format!("Python int too large to convert to SQLite INTEGER: {}", e))
                })?;

Copy link
Contributor Author

@ever0de ever0de Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using assertRaisesRegex in Python to check the error message, we can extract the original error message from the args of PyBaseException and append it.
If you would like me to add it this way, I can submit a commit with this change.

unsafe { sqlite3_bind_int64(self.st, pos, val) }
} else if let Some(val) = obj.payload::<PyFloat>() {
let val = val.to_f64();
Expand Down
Loading