From 8d6ebf57a2e9e610cb11edcd9d3704505e091ba5 Mon Sep 17 00:00:00 2001 From: Robert Klink Date: Wed, 31 Jul 2024 13:58:54 +0200 Subject: [PATCH 1/7] unix-ffi/sqlite3: Fix bytes to accommodate for different pointer sizes. 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 --- unix-ffi/sqlite3/sqlite3.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/unix-ffi/sqlite3/sqlite3.py b/unix-ffi/sqlite3/sqlite3.py index 0f00ff508..1f8bdd6c9 100644 --- a/unix-ffi/sqlite3/sqlite3.py +++ b/unix-ffi/sqlite3/sqlite3.py @@ -1,5 +1,6 @@ import sys import ffilib +import uctypes sq3 = ffilib.open("libsqlite3") @@ -61,6 +62,10 @@ def check_error(db, s): raise Error(s, sqlite3_errmsg(db)) +def get_ptr_size(): + return uctypes.sizeof({"ptr": (0 | uctypes.PTR, uctypes.PTR)}) + + class Connections: def __init__(self, h): self.h = h @@ -83,13 +88,13 @@ def execute(self, sql, params=None): params = [quote(v) for v in params] sql = sql % tuple(params) print(sql) - b = bytearray(4) - s = sqlite3_prepare(self.h, sql, -1, b, None) - check_error(self.h, s) - self.stmnt = int.from_bytes(b, sys.byteorder) - # print("stmnt", self.stmnt) + + stmnt_ptr = bytes(get_ptr_size()) + res = sqlite3_prepare(self.h, sql, -1, stmnt_ptr, None) + check_error(self.h, res) + self.stmnt = int.from_bytes(stmnt_ptr, sys.byteorder) self.num_cols = sqlite3_column_count(self.stmnt) - # print("num_cols", self.num_cols) + # If it's not select, actually execute it here # num_cols == 0 for statements which don't return data (=> modify it) if not self.num_cols: @@ -127,10 +132,9 @@ def fetchone(self): def connect(fname): - b = bytearray(4) - sqlite3_open(fname, b) - h = int.from_bytes(b, sys.byteorder) - return Connections(h) + sqlite_ptr = bytes(get_ptr_size()) + sqlite3_open(fname, sqlite_ptr) + return Connections(int.from_bytes(sqlite_ptr, sys.byteorder)) def quote(val): From 0a65c3d34a4f7c6bf5ed88fe78d4b7f24dc71cdd Mon Sep 17 00:00:00 2001 From: Robert Klink Date: Wed, 31 Jul 2024 14:34:41 +0200 Subject: [PATCH 2/7] unix-ffi/sqlite3: Fix statements not being finalized. 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 --- unix-ffi/sqlite3/sqlite3.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/unix-ffi/sqlite3/sqlite3.py b/unix-ffi/sqlite3/sqlite3.py index 1f8bdd6c9..24175ec17 100644 --- a/unix-ffi/sqlite3/sqlite3.py +++ b/unix-ffi/sqlite3/sqlite3.py @@ -84,6 +84,11 @@ def __init__(self, h): self.stmnt = None def execute(self, sql, params=None): + if self.stmnt: + # If there is an existing statement, finalize that to free it + res = sqlite3_finalize(self.stmnt) + check_error(self.h, res) + if params: params = [quote(v) for v in params] sql = sql % tuple(params) From ab9c5a01b0a62ad97b47ca02caf6bad8eb8f5a42 Mon Sep 17 00:00:00 2001 From: Robert Klink Date: Wed, 31 Jul 2024 14:38:25 +0200 Subject: [PATCH 3/7] unix-ffi/sqlite3: Add optional parameter for URI support. 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 --- unix-ffi/sqlite3/sqlite3.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/unix-ffi/sqlite3/sqlite3.py b/unix-ffi/sqlite3/sqlite3.py index 24175ec17..9c645200c 100644 --- a/unix-ffi/sqlite3/sqlite3.py +++ b/unix-ffi/sqlite3/sqlite3.py @@ -6,6 +6,8 @@ sq3 = ffilib.open("libsqlite3") sqlite3_open = sq3.func("i", "sqlite3_open", "sp") +# int sqlite3_config(int, ...); +sqlite3_config = sq3.func("i", "sqlite3_config", "ii") # int sqlite3_close(sqlite3*); sqlite3_close = sq3.func("i", "sqlite3_close", "p") # int sqlite3_prepare( @@ -52,6 +54,8 @@ SQLITE_BLOB = 4 SQLITE_NULL = 5 +SQLITE_CONFIG_URI = 17 + class Error(Exception): pass @@ -136,7 +140,9 @@ def fetchone(self): check_error(self.h, res) -def connect(fname): +def connect(fname, uri=False): + sqlite3_config(SQLITE_CONFIG_URI, int(uri)) + sqlite_ptr = bytes(get_ptr_size()) sqlite3_open(fname, sqlite_ptr) return Connections(int.from_bytes(sqlite_ptr, sys.byteorder)) From 83598cdb3c7fd92928d2ac953141fc0f70793e05 Mon Sep 17 00:00:00 2001 From: Robert Klink Date: Thu, 1 Aug 2024 11:28:22 +0200 Subject: [PATCH 4/7] unix-ffi/sqlite3: Change to use close and prepare v2 versions, clean-up. 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 --- unix-ffi/sqlite3/sqlite3.py | 40 ++++++++++++++++-------------- unix-ffi/sqlite3/test_sqlite3.py | 3 +++ unix-ffi/sqlite3/test_sqlite3_2.py | 3 +++ 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/unix-ffi/sqlite3/sqlite3.py b/unix-ffi/sqlite3/sqlite3.py index 9c645200c..35c0c011e 100644 --- a/unix-ffi/sqlite3/sqlite3.py +++ b/unix-ffi/sqlite3/sqlite3.py @@ -5,11 +5,15 @@ sq3 = ffilib.open("libsqlite3") +# int sqlite3_open( +# const char *filename, /* Database filename (UTF-8) */ +# sqlite3 **ppDb /* OUT: SQLite db handle */ +# ); sqlite3_open = sq3.func("i", "sqlite3_open", "sp") # int sqlite3_config(int, ...); sqlite3_config = sq3.func("i", "sqlite3_config", "ii") -# int sqlite3_close(sqlite3*); -sqlite3_close = sq3.func("i", "sqlite3_close", "p") +# int sqlite3_close_v2(sqlite3*); +sqlite3_close = sq3.func("i", "sqlite3_close_v2", "p") # int sqlite3_prepare( # sqlite3 *db, /* Database handle */ # const char *zSql, /* SQL statement, UTF-8 encoded */ @@ -17,7 +21,7 @@ # sqlite3_stmt **ppStmt, /* OUT: Statement handle */ # const char **pzTail /* OUT: Pointer to unused portion of zSql */ # ); -sqlite3_prepare = sq3.func("i", "sqlite3_prepare", "psipp") +sqlite3_prepare = sq3.func("i", "sqlite3_prepare_v2", "psipp") # int sqlite3_finalize(sqlite3_stmt *pStmt); sqlite3_finalize = sq3.func("i", "sqlite3_finalize", "p") # int sqlite3_step(sqlite3_stmt*); @@ -26,20 +30,17 @@ sqlite3_column_count = sq3.func("i", "sqlite3_column_count", "p") # int sqlite3_column_type(sqlite3_stmt*, int iCol); sqlite3_column_type = sq3.func("i", "sqlite3_column_type", "pi") +# int sqlite3_column_int(sqlite3_stmt*, int iCol); sqlite3_column_int = sq3.func("i", "sqlite3_column_int", "pi") -# using "d" return type gives wrong results +# double sqlite3_column_double(sqlite3_stmt*, int iCol); sqlite3_column_double = sq3.func("d", "sqlite3_column_double", "pi") +# const unsigned char *sqlite3_column_text(sqlite3_stmt*, int iCol); sqlite3_column_text = sq3.func("s", "sqlite3_column_text", "pi") # sqlite3_int64 sqlite3_last_insert_rowid(sqlite3*); -# TODO: should return long int -sqlite3_last_insert_rowid = sq3.func("i", "sqlite3_last_insert_rowid", "p") +sqlite3_last_insert_rowid = sq3.func("l", "sqlite3_last_insert_rowid", "p") # const char *sqlite3_errmsg(sqlite3*); sqlite3_errmsg = sq3.func("s", "sqlite3_errmsg", "p") -# Too recent -##const char *sqlite3_errstr(int); -# sqlite3_errstr = sq3.func("s", "sqlite3_errstr", "i") - SQLITE_OK = 0 SQLITE_ERROR = 1 @@ -78,8 +79,10 @@ def cursor(self): return Cursor(self.h) def close(self): - s = sqlite3_close(self.h) - check_error(self.h, s) + if self.h: + s = sqlite3_close(self.h) + check_error(self.h, s) + self.h = None class Cursor: @@ -96,7 +99,6 @@ def execute(self, sql, params=None): if params: params = [quote(v) for v in params] sql = sql % tuple(params) - print(sql) stmnt_ptr = bytes(get_ptr_size()) res = sqlite3_prepare(self.h, sql, -1, stmnt_ptr, None) @@ -104,22 +106,23 @@ def execute(self, sql, params=None): self.stmnt = int.from_bytes(stmnt_ptr, sys.byteorder) self.num_cols = sqlite3_column_count(self.stmnt) - # If it's not select, actually execute it here - # num_cols == 0 for statements which don't return data (=> modify it) if not self.num_cols: v = self.fetchone() + # If it's not select, actually execute it here + # num_cols == 0 for statements which don't return data (=> modify it) assert v is None self.lastrowid = sqlite3_last_insert_rowid(self.h) def close(self): - s = sqlite3_finalize(self.stmnt) - check_error(self.h, s) + if self.stmnt: + s = sqlite3_finalize(self.stmnt) + check_error(self.h, s) + self.stmnt = None def make_row(self): res = [] for i in range(self.num_cols): t = sqlite3_column_type(self.stmnt, i) - # print("type", t) if t == SQLITE_INTEGER: res.append(sqlite3_column_int(self.stmnt, i)) elif t == SQLITE_FLOAT: @@ -132,7 +135,6 @@ def make_row(self): def fetchone(self): res = sqlite3_step(self.stmnt) - # print("step:", res) if res == SQLITE_DONE: return None if res == SQLITE_ROW: diff --git a/unix-ffi/sqlite3/test_sqlite3.py b/unix-ffi/sqlite3/test_sqlite3.py index 39dc07549..b168f18ff 100644 --- a/unix-ffi/sqlite3/test_sqlite3.py +++ b/unix-ffi/sqlite3/test_sqlite3.py @@ -17,3 +17,6 @@ assert row == e assert expected == [] + +cur.close() +conn.close() diff --git a/unix-ffi/sqlite3/test_sqlite3_2.py b/unix-ffi/sqlite3/test_sqlite3_2.py index 68a2abb86..515f865c3 100644 --- a/unix-ffi/sqlite3/test_sqlite3_2.py +++ b/unix-ffi/sqlite3/test_sqlite3_2.py @@ -10,3 +10,6 @@ cur.execute("SELECT * FROM foo") assert cur.fetchone() == (42,) assert cur.fetchone() is None + +cur.close() +conn.close() From b77f67bd7ccd6701e2bf3333a50c56fa709b68fc Mon Sep 17 00:00:00 2001 From: Robert Klink Date: Tue, 6 Aug 2024 15:24:39 +0200 Subject: [PATCH 5/7] unix-ffi/sqlite3: Add commit and rollback functionality like CPython. 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 --- unix-ffi/sqlite3/sqlite3.py | 133 ++++++++++++++++++++--------- unix-ffi/sqlite3/test_sqlite3_3.py | 42 +++++++++ 2 files changed, 137 insertions(+), 38 deletions(-) create mode 100644 unix-ffi/sqlite3/test_sqlite3_3.py diff --git a/unix-ffi/sqlite3/sqlite3.py b/unix-ffi/sqlite3/sqlite3.py index 35c0c011e..299f8247d 100644 --- a/unix-ffi/sqlite3/sqlite3.py +++ b/unix-ffi/sqlite3/sqlite3.py @@ -12,6 +12,8 @@ sqlite3_open = sq3.func("i", "sqlite3_open", "sp") # int sqlite3_config(int, ...); sqlite3_config = sq3.func("i", "sqlite3_config", "ii") +# int sqlite3_get_autocommit(sqlite3*); +sqlite3_get_autocommit = sq3.func("i", "sqlite3_get_autocommit", "p") # int sqlite3_close_v2(sqlite3*); sqlite3_close = sq3.func("i", "sqlite3_close_v2", "p") # int sqlite3_prepare( @@ -57,6 +59,9 @@ SQLITE_CONFIG_URI = 17 +# For compatibility with CPython sqlite3 driver +LEGACY_TRANSACTION_CONTROL = -1 + class Error(Exception): pass @@ -71,86 +76,138 @@ def get_ptr_size(): return uctypes.sizeof({"ptr": (0 | uctypes.PTR, uctypes.PTR)}) +def __prepare_stmt(db, sql): + # Prepares a statement + stmt_ptr = bytes(get_ptr_size()) + res = sqlite3_prepare(db, sql, -1, stmt_ptr, None) + check_error(db, res) + return int.from_bytes(stmt_ptr, sys.byteorder) + +def __exec_stmt(db, sql): + # Prepares, executes, and finalizes a statement + stmt = __prepare_stmt(db, sql) + sqlite3_step(stmt) + res = sqlite3_finalize(stmt) + check_error(db, res) + +def __is_dml(sql): + # Checks if a sql query is a DML, as these get a BEGIN in LEGACY_TRANSACTION_CONTROL + for dml in ["INSERT", "DELETE", "UPDATE", "MERGE"]: + if dml in sql.upper(): + return True + return False + + class Connections: - def __init__(self, h): - self.h = h + def __init__(self, db, isolation_level, autocommit): + self.db = db + self.isolation_level = isolation_level + self.autocommit = autocommit + + def commit(self): + if self.autocommit == LEGACY_TRANSACTION_CONTROL and not sqlite3_get_autocommit(self.db): + __exec_stmt(self.db, "COMMIT") + elif self.autocommit == False: + __exec_stmt(self.db, "COMMIT") + __exec_stmt(self.db, "BEGIN") + + def rollback(self): + if self.autocommit == LEGACY_TRANSACTION_CONTROL and not sqlite3_get_autocommit(self.db): + __exec_stmt(self.db, "ROLLBACK") + elif self.autocommit == False: + __exec_stmt(self.db, "ROLLBACK") + __exec_stmt(self.db, "BEGIN") def cursor(self): - return Cursor(self.h) + return Cursor(self.db, self.isolation_level, self.autocommit) def close(self): - if self.h: - s = sqlite3_close(self.h) - check_error(self.h, s) - self.h = None + if self.db: + if self.autocommit == False and not sqlite3_get_autocommit(self.db): + __exec_stmt(self.db, "ROLLBACK") + + res = sqlite3_close(self.db) + check_error(self.db, res) + self.db = None class Cursor: - def __init__(self, h): - self.h = h - self.stmnt = None + def __init__(self, db, isolation_level, autocommit): + self.db = db + self.isolation_level = isolation_level + self.autocommit = autocommit + self.stmt = None + + def __quote(val): + if isinstance(val, str): + return "'%s'" % val + return str(val) def execute(self, sql, params=None): - if self.stmnt: + if self.stmt: # If there is an existing statement, finalize that to free it - res = sqlite3_finalize(self.stmnt) - check_error(self.h, res) + res = sqlite3_finalize(self.stmt) + check_error(self.db, res) if params: - params = [quote(v) for v in params] + params = [self.__quote(v) for v in params] sql = sql % tuple(params) - stmnt_ptr = bytes(get_ptr_size()) - res = sqlite3_prepare(self.h, sql, -1, stmnt_ptr, None) - check_error(self.h, res) - self.stmnt = int.from_bytes(stmnt_ptr, sys.byteorder) - self.num_cols = sqlite3_column_count(self.stmnt) + if __is_dml(sql) and self.autocommit == LEGACY_TRANSACTION_CONTROL and sqlite3_get_autocommit(self.db): + # For compatibility with CPython, add functionality for their default transaction + # behavior. Changing autocommit from LEGACY_TRANSACTION_CONTROL will remove this + __exec_stmt(self.db, "BEGIN " + self.isolation_level) + + self.stmt = __prepare_stmt(self.db, sql) + self.num_cols = sqlite3_column_count(self.stmt) if not self.num_cols: v = self.fetchone() # If it's not select, actually execute it here # num_cols == 0 for statements which don't return data (=> modify it) assert v is None - self.lastrowid = sqlite3_last_insert_rowid(self.h) + self.lastrowid = sqlite3_last_insert_rowid(self.db) def close(self): - if self.stmnt: - s = sqlite3_finalize(self.stmnt) - check_error(self.h, s) - self.stmnt = None + if self.stmt: + res = sqlite3_finalize(self.stmt) + check_error(self.db, res) + self.stmt = None - def make_row(self): + def __make_row(self): res = [] for i in range(self.num_cols): - t = sqlite3_column_type(self.stmnt, i) + t = sqlite3_column_type(self.stmt, i) if t == SQLITE_INTEGER: - res.append(sqlite3_column_int(self.stmnt, i)) + res.append(sqlite3_column_int(self.stmt, i)) elif t == SQLITE_FLOAT: - res.append(sqlite3_column_double(self.stmnt, i)) + res.append(sqlite3_column_double(self.stmt, i)) elif t == SQLITE_TEXT: - res.append(sqlite3_column_text(self.stmnt, i)) + res.append(sqlite3_column_text(self.stmt, i)) else: raise NotImplementedError return tuple(res) def fetchone(self): - res = sqlite3_step(self.stmnt) + res = sqlite3_step(self.stmt) if res == SQLITE_DONE: return None if res == SQLITE_ROW: - return self.make_row() - check_error(self.h, res) + return self.__make_row() + check_error(self.db, res) + +def connect(fname, uri=False, isolation_level="", autocommit=LEGACY_TRANSACTION_CONTROL): + if isolation_level not in [None, "", "DEFERRED", "IMMEDIATE", "EXCLUSIVE"]: + raise Error("Invalid option for isolation level") -def connect(fname, uri=False): sqlite3_config(SQLITE_CONFIG_URI, int(uri)) sqlite_ptr = bytes(get_ptr_size()) sqlite3_open(fname, sqlite_ptr) - return Connections(int.from_bytes(sqlite_ptr, sys.byteorder)) + db = int.from_bytes(sqlite_ptr, sys.byteorder) + if autocommit == False: + __exec_stmt(db, "BEGIN") -def quote(val): - if isinstance(val, str): - return "'%s'" % val - return str(val) + return Connections(db, isolation_level, autocommit) diff --git a/unix-ffi/sqlite3/test_sqlite3_3.py b/unix-ffi/sqlite3/test_sqlite3_3.py new file mode 100644 index 000000000..0a6fefc97 --- /dev/null +++ b/unix-ffi/sqlite3/test_sqlite3_3.py @@ -0,0 +1,42 @@ +import sqlite3 + + +def test_autocommit(): + conn = sqlite3.connect(":memory:", autocommit=True) + + # First cursor creates table and inserts value (DML) + cur = conn.cursor() + cur.execute("CREATE TABLE foo(a int)") + cur.execute("INSERT INTO foo VALUES (42)") + cur.close() + + # Second cursor fetches 42 due to the autocommit + cur = conn.cursor() + cur.execute("SELECT * FROM foo") + assert cur.fetchone() == (42,) + assert cur.fetchone() is None + + cur.close() + conn.close() + +def test_manual(): + conn = sqlite3.connect(":memory:", autocommit=False) + + # First cursor creates table, insert rolls back + cur = conn.cursor() + cur.execute("CREATE TABLE foo(a int)") + conn.commit() + cur.execute("INSERT INTO foo VALUES (42)") + cur.close() + conn.rollback() + + # Second connection fetches nothing due to the rollback + cur = conn.cursor() + cur.execute("SELECT * FROM foo") + assert cur.fetchone() is None + + cur.close() + conn.close() + +test_autocommit() +test_manual() From bea5367ce23d9683d15ff19c16e246449827cb4b Mon Sep 17 00:00:00 2001 From: Damien George Date: Thu, 22 Aug 2024 13:05:33 +1000 Subject: [PATCH 6/7] unix-ffi/sqlite3: Bump version to 0.3.0. The previous commits fixed bugs and added new features. Signed-off-by: Damien George --- unix-ffi/sqlite3/manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unix-ffi/sqlite3/manifest.py b/unix-ffi/sqlite3/manifest.py index 63cdf4b9f..5b04d71d3 100644 --- a/unix-ffi/sqlite3/manifest.py +++ b/unix-ffi/sqlite3/manifest.py @@ -1,4 +1,4 @@ -metadata(version="0.2.4") +metadata(version="0.3.0") # Originally written by Paul Sokolovsky. From 66fa62bda10d199be5b24457e044cb863d5d216a Mon Sep 17 00:00:00 2001 From: Damien George Date: Thu, 22 Aug 2024 13:08:29 +1000 Subject: [PATCH 7/7] tools/ci.sh: Add sqlite3 tests to CI. Signed-off-by: Damien George --- tools/ci.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/ci.sh b/tools/ci.sh index c2cf9dbad..07b27d13c 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -66,6 +66,9 @@ function ci_package_tests_run { unix-ffi/gettext/test_gettext.py \ unix-ffi/pwd/test_getpwnam.py \ unix-ffi/re/test_re.py \ + unix-ffi/sqlite3/test_sqlite3.py \ + unix-ffi/sqlite3/test_sqlite3_2.py \ + unix-ffi/sqlite3/test_sqlite3_3.py \ unix-ffi/time/test_strftime.py \ ; do echo "Running test $test"