From 4442ad0bbd06d2e843a5bb280cc96070aafec2c8 Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Thu, 21 Jul 2022 15:01:04 -0500 Subject: [PATCH 01/10] Adding public method to reset a prepared statement. This is needed so that clients can reset any prepared statements they keep in memory, which allows for transactions to completely finish. without closing transactions (regardless of commit), the wal file cannot be checkpointed and will grow unbounded. --- Sources/SQLite/Core/Statement.swift | 6 ++- Tests/SQLiteTests/StatementTests.swift | 57 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/Sources/SQLite/Core/Statement.swift b/Sources/SQLite/Core/Statement.swift index 5ec430cf..7ba4026c 100644 --- a/Sources/SQLite/Core/Statement.swift +++ b/Sources/SQLite/Core/Statement.swift @@ -185,7 +185,11 @@ public final class Statement { try connection.sync { try connection.check(sqlite3_step(handle)) == SQLITE_ROW } } - fileprivate func reset(clearBindings shouldClear: Bool = true) { + public func reset() { + reset(clearBindings: true) + } + + fileprivate func reset(clearBindings shouldClear: Bool) { sqlite3_reset(handle) if shouldClear { sqlite3_clear_bindings(handle) } } diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index 3286b792..77bc09c1 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -34,4 +34,61 @@ class StatementTests: SQLiteTestCase { XCTAssertEqual(names.map({ "\($0)@example.com" }), emails.sorted()) } + + /// Check that a statement reset will close the implicit transaction, allowing wal file to checkpoint + func test_reset_statement() throws { + // Remove old test db if any + let path = "\(NSTemporaryDirectory())/SQLite.swift Tests.sqlite3" + try? FileManager.default.removeItem(atPath: path) + try? FileManager.default.removeItem(atPath: path + "-shm") + try? FileManager.default.removeItem(atPath: path + "-wal") + + // create new db on disk in wal mode + let db = try Connection(.uri(path)) + let url = URL(fileURLWithPath: db.description) + XCTAssertEqual(url.lastPathComponent, "SQLite.swift Tests.sqlite3") + try db.run("PRAGMA journal_mode=WAL;") + + // create users table + try db.execute(""" + CREATE TABLE users ( + id INTEGER PRIMARY KEY, + email TEXT NOT NULL UNIQUE, + age INTEGER, + salary REAL, + admin BOOLEAN NOT NULL DEFAULT 0 CHECK (admin IN (0, 1)), + manager_id INTEGER, + created_at DATETIME, + FOREIGN KEY(manager_id) REFERENCES users(id) + ) + """ + ) + + // insert single row + try db.run("INSERT INTO \"users\" (email, age, admin) values (?, ?, ?)", + "alice@example.com", 1.datatypeValue, false.datatypeValue) + + // prepare a statement and read a single row. This will incremeent the cursor which + // prevents the implicit transaction from closing. + // https://www.sqlite.org/lang_transaction.html#implicit_versus_explicit_transactions + let statement = try db.prepare("SELECT email FROM users") + XCTAssert(try statement.step()) + let blob = statement.row[0] as Blob + XCTAssertEqual("alice@example.com", String(bytes: blob.bytes, encoding: .utf8)!) + + // verify that the transaction is not closed, which prevents wal_checkpoints (both explicit and auto) + do { + try db.run("pragma wal_checkpoint(truncate)") + XCTFail("Database should be locked") + } catch { + // pass + } + + // reset the prepared statement, allowing the implicit transaction to close + statement.reset() + + // truncate succeeds + try db.run("pragma wal_checkpoint(truncate)") + } + } From ac1988c9663e8c5f64d21b255d91c154a30e2f35 Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 14:12:11 -0500 Subject: [PATCH 02/10] Test cleanup for statement reset --- Tests/SQLiteTests/StatementTests.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index 77bc09c1..c12fe8cb 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -38,15 +38,13 @@ class StatementTests: SQLiteTestCase { /// Check that a statement reset will close the implicit transaction, allowing wal file to checkpoint func test_reset_statement() throws { // Remove old test db if any - let path = "\(NSTemporaryDirectory())/SQLite.swift Tests.sqlite3" + let path = temporaryFile() + ".sqlite3" try? FileManager.default.removeItem(atPath: path) try? FileManager.default.removeItem(atPath: path + "-shm") try? FileManager.default.removeItem(atPath: path + "-wal") // create new db on disk in wal mode let db = try Connection(.uri(path)) - let url = URL(fileURLWithPath: db.description) - XCTAssertEqual(url.lastPathComponent, "SQLite.swift Tests.sqlite3") try db.run("PRAGMA journal_mode=WAL;") // create users table From 012194b271dc32a0a9d1182edb4bea5201d8d59d Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 15:43:14 -0500 Subject: [PATCH 03/10] Adding documentation for Statement.reset(). --- Documentation/Index.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/Index.md b/Documentation/Index.md index b71cdc4f..ed79f33e 100644 --- a/Documentation/Index.md +++ b/Documentation/Index.md @@ -1872,6 +1872,14 @@ let stmt = try db.prepare("SELECT * FROM attachments WHERE typeConformsTo(UTI, ? for row in stmt.bind(kUTTypeImage) { /* ... */ } ``` +> _Note:_ Prepared queries can be reused, and long lived prepared queries should be `reset()` after each use. Otherwise, the transaction (either implicit or explicit) might be held open until the query is reset or finalized. This can affect performance. Statements are reset automatically during `deinit`. +> +> ```swift +> someObj.statement = try db.prepare("SELECT * FROM attachments WHERE typeConformsTo(UTI, ?)") +> for row in someObj.statement.bind(kUTTypeImage) { /* ... */ } +> someObj.statement.reset() +> ``` + [UTTypeConformsTo]: https://developer.apple.com/documentation/coreservices/1444079-uttypeconformsto ## Custom Aggregations From 58fd9dc1b6db7544135af10ced07f720e2dc36ac Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 15:43:42 -0500 Subject: [PATCH 04/10] Fix close of markdown code block --- Documentation/Index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Index.md b/Documentation/Index.md index ed79f33e..2a877a71 100644 --- a/Documentation/Index.md +++ b/Documentation/Index.md @@ -828,7 +828,7 @@ let query = try db.prepare(users) for user in query { // 💥 can throw an error here } -```` +``` #### Failable iteration From 7b3de3d24b3eaea797a3db1b5a6373196edd4561 Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 15:48:15 -0500 Subject: [PATCH 05/10] Add link to relevant sqlite.org documentation page --- Documentation/Index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Index.md b/Documentation/Index.md index 2a877a71..29b18c96 100644 --- a/Documentation/Index.md +++ b/Documentation/Index.md @@ -1872,7 +1872,7 @@ let stmt = try db.prepare("SELECT * FROM attachments WHERE typeConformsTo(UTI, ? for row in stmt.bind(kUTTypeImage) { /* ... */ } ``` -> _Note:_ Prepared queries can be reused, and long lived prepared queries should be `reset()` after each use. Otherwise, the transaction (either implicit or explicit) might be held open until the query is reset or finalized. This can affect performance. Statements are reset automatically during `deinit`. +> _Note:_ Prepared queries can be reused, and long lived prepared queries should be `reset()` after each use. Otherwise, the transaction (either [implicit or explicit](https://www.sqlite.org/lang_transaction.html#implicit_versus_explicit_transactions)) will be held open until the query is reset or finalized. This can affect performance. Statements are reset automatically during `deinit`. > > ```swift > someObj.statement = try db.prepare("SELECT * FROM attachments WHERE typeConformsTo(UTI, ?)") From 5a30f299c41f335a0745b0eab773db4f3ee184af Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 17:45:52 -0500 Subject: [PATCH 06/10] cleanup test for Statement.reset() --- Tests/SQLiteTests/StatementTests.swift | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index c12fe8cb..b3048211 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -37,13 +37,8 @@ class StatementTests: SQLiteTestCase { /// Check that a statement reset will close the implicit transaction, allowing wal file to checkpoint func test_reset_statement() throws { - // Remove old test db if any - let path = temporaryFile() + ".sqlite3" - try? FileManager.default.removeItem(atPath: path) - try? FileManager.default.removeItem(atPath: path + "-shm") - try? FileManager.default.removeItem(atPath: path + "-wal") - // create new db on disk in wal mode + let path = temporaryFile() + ".sqlite3" let db = try Connection(.uri(path)) try db.run("PRAGMA journal_mode=WAL;") @@ -70,9 +65,7 @@ class StatementTests: SQLiteTestCase { // prevents the implicit transaction from closing. // https://www.sqlite.org/lang_transaction.html#implicit_versus_explicit_transactions let statement = try db.prepare("SELECT email FROM users") - XCTAssert(try statement.step()) - let blob = statement.row[0] as Blob - XCTAssertEqual("alice@example.com", String(bytes: blob.bytes, encoding: .utf8)!) + _ = try statement.step() // verify that the transaction is not closed, which prevents wal_checkpoints (both explicit and auto) do { From efa5fcb5c24352d266e9a87075dbfb3e1c5fd7ea Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 17:47:46 -0500 Subject: [PATCH 07/10] Better check for throwing error in test --- Tests/SQLiteTests/StatementTests.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index b3048211..e41fa714 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -68,12 +68,7 @@ class StatementTests: SQLiteTestCase { _ = try statement.step() // verify that the transaction is not closed, which prevents wal_checkpoints (both explicit and auto) - do { - try db.run("pragma wal_checkpoint(truncate)") - XCTFail("Database should be locked") - } catch { - // pass - } + XCTAssertThrowsError(try db.run("pragma wal_checkpoint(truncate)")) // reset the prepared statement, allowing the implicit transaction to close statement.reset() From da0cd09219fe02d22c721637bc55d0ce600821ff Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Fri, 22 Jul 2022 19:25:52 -0500 Subject: [PATCH 08/10] Remove unneeded wal mode --- Tests/SQLiteTests/StatementTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index e41fa714..3cc68aca 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -40,7 +40,6 @@ class StatementTests: SQLiteTestCase { // create new db on disk in wal mode let path = temporaryFile() + ".sqlite3" let db = try Connection(.uri(path)) - try db.run("PRAGMA journal_mode=WAL;") // create users table try db.execute(""" From 210303f7b21f829bf82e17f41a8f912742ea2be3 Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Sat, 23 Jul 2022 15:19:10 -0500 Subject: [PATCH 09/10] Use in memory database from test setup, verify error when statement isn't reset, update comments --- Tests/SQLiteTests/StatementTests.swift | 46 ++++++++++++-------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index 3cc68aca..bc100cd2 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -1,6 +1,16 @@ import XCTest import SQLite +#if SQLITE_SWIFT_STANDALONE +import sqlite3 +#elseif SQLITE_SWIFT_SQLCIPHER +import SQLCipher +#elseif os(Linux) +import CSQLite +#else +import SQLite3 +#endif + class StatementTests: SQLiteTestCase { override func setUpWithError() throws { try super.setUpWithError() @@ -37,28 +47,8 @@ class StatementTests: SQLiteTestCase { /// Check that a statement reset will close the implicit transaction, allowing wal file to checkpoint func test_reset_statement() throws { - // create new db on disk in wal mode - let path = temporaryFile() + ".sqlite3" - let db = try Connection(.uri(path)) - - // create users table - try db.execute(""" - CREATE TABLE users ( - id INTEGER PRIMARY KEY, - email TEXT NOT NULL UNIQUE, - age INTEGER, - salary REAL, - admin BOOLEAN NOT NULL DEFAULT 0 CHECK (admin IN (0, 1)), - manager_id INTEGER, - created_at DATETIME, - FOREIGN KEY(manager_id) REFERENCES users(id) - ) - """ - ) - // insert single row - try db.run("INSERT INTO \"users\" (email, age, admin) values (?, ?, ?)", - "alice@example.com", 1.datatypeValue, false.datatypeValue) + try insertUsers("bob") // prepare a statement and read a single row. This will incremeent the cursor which // prevents the implicit transaction from closing. @@ -66,14 +56,20 @@ class StatementTests: SQLiteTestCase { let statement = try db.prepare("SELECT email FROM users") _ = try statement.step() - // verify that the transaction is not closed, which prevents wal_checkpoints (both explicit and auto) - XCTAssertThrowsError(try db.run("pragma wal_checkpoint(truncate)")) + // verify implicit transaction is not closed, and the users table is still locked + XCTAssertThrowsError(try db.run("DROP TABLE users")) { error in + if case let Result.error(_, code, _) = error { + XCTAssertEqual(code, SQLITE_LOCKED) + } else { + XCTFail("unexpected error") + } + } - // reset the prepared statement, allowing the implicit transaction to close + // reset the prepared statement, unlocking the table and allowing the implicit transaction to close statement.reset() // truncate succeeds - try db.run("pragma wal_checkpoint(truncate)") + try db.run("DROP TABLE users") } } From d970a11bd66747fd3b67d8ce072d944fca03b341 Mon Sep 17 00:00:00 2001 From: Adam Wulf Date: Sat, 23 Jul 2022 15:21:03 -0500 Subject: [PATCH 10/10] typo --- Tests/SQLiteTests/StatementTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SQLiteTests/StatementTests.swift b/Tests/SQLiteTests/StatementTests.swift index bc100cd2..eeb513fe 100644 --- a/Tests/SQLiteTests/StatementTests.swift +++ b/Tests/SQLiteTests/StatementTests.swift @@ -50,7 +50,7 @@ class StatementTests: SQLiteTestCase { // insert single row try insertUsers("bob") - // prepare a statement and read a single row. This will incremeent the cursor which + // prepare a statement and read a single row. This will increment the cursor which // prevents the implicit transaction from closing. // https://www.sqlite.org/lang_transaction.html#implicit_versus_explicit_transactions let statement = try db.prepare("SELECT email FROM users")