unix-ffi/sqlite3: Fix 64 bit support, statement finalization, and add UFI, commit, and rollback support.
#905
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rhermanklink
force-pushed
the
unix-ffi-sqlite3
branch
from
286a9a0 to
a1b646d
Compare
rhermanklink
changed the title
UFI, commit support.UFI, commit, and rollback support.
|
This looks in a reasonable state to merge; thanks for the contribution! I've ensured that the tests pass and, for convenience, have updated the For reference, here's how one of the test files can be executed (running the command at the root of a micropython-lib clone): > docker run -ti --rm -v $(pwd):/mplib -v $(pwd)/unix-ffi/sqlite3:/code -w /code micropython/unix
MicroPython v1.23.0 on 2024-08-20; linux [GCC 12.2.0] version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> import sys; sys.path.append("/mplib/unix-ffi/ffilib")
>>> import test_sqlite3
(1, 'foo', 3.0)
(3, 3, 3)
>>>It could do with a README explaining how to use the library - but to be fair, the whole unix-ffi documentation should be improved! Merging this as-is will mean that folks that want to use sqlite in the unix port have a good option. |
Currently, the bytes object used to store the sqlite3 database pointer is always 4 bytes, which causes segfaults on 64 bit platforms with 8 byte pointers. To address this, the size is now dynamically determined using the uctypes modules pointer size. Signed-off-by: Robert Klink <[email protected]>
Currently, statements are only finalized upon a call to Cursor.close(). However, in Cursor.execute() new statements get created without the previous statements being finalized, causing those to get leaked, preventing the database from being closed. The fix addresses this by finalizing the previous statement if it exists. Signed-off-by: Robert Klink <[email protected]>
This commit adds the ability to enable URI on the connect, as can be done in the cpython sqlite3 module. URI allows, among other things, to create a shared named in-memory database, which non URI filenames cannot create. Signed-off-by: Robert Klink <[email protected]>
The sqlite3_prepare and sqlite3_close have been changed to use the v2 version. For the prepare this was done as the v1 version is "legacy", and for close the documentation describes the v2 version to be used for "host languages that are garbage collected, and where the order in which destructors are called is arbitrary", which fits here. Some clean-up to comments has also be done, and the tests now also close the Cursor and Connections. Signed-off-by: Robert Klink <[email protected]>
To increase the similarity between this module and CPythons sqlite3 module the commit() and rollback() as defined in CPythons version have been added, along with the different (auto)commit behaviors present there. The defaults are also set to the same as in CPython, and can be changed with the same parameters in connect(), as is showcased in the new test. Signed-off-by: Robert Klink <[email protected]>
The previous commits fixed bugs and added new features. Signed-off-by: Damien George <[email protected]>
Signed-off-by: Damien George <[email protected]>
dpgeorge
force-pushed
the
unix-ffi-sqlite3
branch
from
e5158aa to
66fa62b
Compare
|
Thanks for the contribution, this is a very nice improvement! I have added commits to bump the manifest.py version, and include the sqlite3 tests in CI. I also removed trailing whitespace in the last commit. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current
sqlite.pyfile only takes 4 bytes for pointers, disallowing 64 bit platforms, and also doesn't finalize statements except in theCursor.close(), causing leaks and issues withConnections.close(). This PR fixes these issues by determining the pointer size dynamically for the platform, and finalizes the previous statements when a new one is made inexecute().Along with these fixes, this PR also adds the ability to use
URI, andcommit&rollbackfunctionality to increase similarity with the CPython sqlite3 module. The addition ofcommit&rollbackincludes the addition of the autocommit and transaction posibilities as with the CPython sqlite3 module, with the same default mode enabled here.