From 330f25029b8be9704ff2c5191232798d7d221cab Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Wed, 16 Jul 2025 01:06:16 +0900 Subject: [PATCH 1/4] Correctly implement `isolation_level` setter to handle deletion --- Lib/test/test_sqlite3/test_regression.py | 2 -- stdlib/src/sqlite.rs | 39 ++++++++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_sqlite3/test_regression.py b/Lib/test/test_sqlite3/test_regression.py index 870958ceee5..e2afb7f0e3c 100644 --- a/Lib/test/test_sqlite3/test_regression.py +++ b/Lib/test/test_sqlite3/test_regression.py @@ -413,8 +413,6 @@ def callback(*args): del ref support.gc_collect() - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_del_isolation_level_segfault(self): with self.assertRaises(AttributeError): del self.con.isolation_level diff --git a/stdlib/src/sqlite.rs b/stdlib/src/sqlite.rs index 4e9620eeabd..bd28ee7790b 100644 --- a/stdlib/src/sqlite.rs +++ b/stdlib/src/sqlite.rs @@ -61,6 +61,7 @@ mod _sqlite { PyInt, PyIntRef, PySlice, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, }, convert::IntoObject, + function::PySetterValue, function::{ArgCallable, ArgIterable, FsPath, FuncArgs, OptionalArg, PyComparisonValue}, object::{Traverse, TraverseFn}, protocol::{PyBuffer, PyIterReturn, PyMappingMethods, PySequence, PySequenceMethods}, @@ -1350,22 +1351,34 @@ mod _sqlite { self.isolation_level.deref().map(|x| x.to_owned()) } #[pygetset(setter)] - fn set_isolation_level(&self, val: Option, vm: &VirtualMachine) -> PyResult<()> { - if let Some(val) = &val { - begin_statement_ptr_from_isolation_level(val, vm)?; - } + fn set_isolation_level(&self, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { + match value { + PySetterValue::Assign(new_value) => { + let val = if vm.is_none(&new_value) { + None + } else { + Some(new_value.try_into_value::(vm)?) + }; + + if let Some(val_str) = &val { + begin_statement_ptr_from_isolation_level(val_str, vm)?; + } - // If setting isolation_level to None (auto-commit mode), commit any pending transaction - if val.is_none() { - let db = self.db_lock(vm)?; - if !db.is_autocommit() { - // Keep the lock and call implicit_commit directly to avoid race conditions - db.implicit_commit(vm)?; + // If setting isolation_level to None (auto-commit mode), commit any pending transaction + if val.is_none() { + let db = self.db_lock(vm)?; + if !db.is_autocommit() { + // Keep the lock and call implicit_commit directly to avoid race conditions + db.implicit_commit(vm)?; + } + } + let _ = unsafe { self.isolation_level.swap(val) }; + Ok(()) } + PySetterValue::Delete => Err(vm.new_attribute_error( + "'isolation_level' attribute cannot be deleted".to_owned(), + )), } - - let _ = unsafe { self.isolation_level.swap(val) }; - Ok(()) } #[pygetset] From 7f2ba6a2c26c8886054a01cbfacee901c870f975 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Wed, 16 Jul 2025 09:12:41 +0900 Subject: [PATCH 2/4] Fix import --- stdlib/src/sqlite.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stdlib/src/sqlite.rs b/stdlib/src/sqlite.rs index bd28ee7790b..f2eeb669a81 100644 --- a/stdlib/src/sqlite.rs +++ b/stdlib/src/sqlite.rs @@ -61,8 +61,7 @@ mod _sqlite { PyInt, PyIntRef, PySlice, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, }, convert::IntoObject, - function::PySetterValue, - function::{ArgCallable, ArgIterable, FsPath, FuncArgs, OptionalArg, PyComparisonValue}, + function::{ArgCallable, ArgIterable, FsPath, FuncArgs, OptionalArg, PyComparisonValue, PySetterValue}, object::{Traverse, TraverseFn}, protocol::{PyBuffer, PyIterReturn, PyMappingMethods, PySequence, PySequenceMethods}, sliceable::{SaturatedSliceIter, SliceableSequenceOp}, From e61317a442b1d0e98ca66df831fdc0ffddccb5b2 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Wed, 16 Jul 2025 12:55:50 +0900 Subject: [PATCH 3/4] fmt --- stdlib/src/sqlite.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stdlib/src/sqlite.rs b/stdlib/src/sqlite.rs index f2eeb669a81..6dda4641041 100644 --- a/stdlib/src/sqlite.rs +++ b/stdlib/src/sqlite.rs @@ -61,7 +61,10 @@ mod _sqlite { PyInt, PyIntRef, PySlice, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, }, convert::IntoObject, - function::{ArgCallable, ArgIterable, FsPath, FuncArgs, OptionalArg, PyComparisonValue, PySetterValue}, + function::{ + ArgCallable, ArgIterable, FsPath, FuncArgs, OptionalArg, PyComparisonValue, + PySetterValue, + }, object::{Traverse, TraverseFn}, protocol::{PyBuffer, PyIterReturn, PyMappingMethods, PySequence, PySequenceMethods}, sliceable::{SaturatedSliceIter, SliceableSequenceOp}, From 6c8ae4eef9d42501f3b569a95e3c828d43b9f739 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Thu, 17 Jul 2025 14:02:16 +0900 Subject: [PATCH 4/4] Change value type PySetterValue -> PySetterValue> --- stdlib/src/sqlite.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/stdlib/src/sqlite.rs b/stdlib/src/sqlite.rs index 6dda4641041..ec1e4f0631d 100644 --- a/stdlib/src/sqlite.rs +++ b/stdlib/src/sqlite.rs @@ -1353,28 +1353,26 @@ mod _sqlite { self.isolation_level.deref().map(|x| x.to_owned()) } #[pygetset(setter)] - fn set_isolation_level(&self, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { + fn set_isolation_level( + &self, + value: PySetterValue>, + vm: &VirtualMachine, + ) -> PyResult<()> { match value { - PySetterValue::Assign(new_value) => { - let val = if vm.is_none(&new_value) { - None - } else { - Some(new_value.try_into_value::(vm)?) - }; - - if let Some(val_str) = &val { + PySetterValue::Assign(value) => { + if let Some(val_str) = &value { begin_statement_ptr_from_isolation_level(val_str, vm)?; } // If setting isolation_level to None (auto-commit mode), commit any pending transaction - if val.is_none() { + if value.is_none() { let db = self.db_lock(vm)?; if !db.is_autocommit() { // Keep the lock and call implicit_commit directly to avoid race conditions db.implicit_commit(vm)?; } } - let _ = unsafe { self.isolation_level.swap(val) }; + let _ = unsafe { self.isolation_level.swap(value) }; Ok(()) } PySetterValue::Delete => Err(vm.new_attribute_error(