Skip to content

Commit 84bee19

Browse files
grouegroue
authored
Merge pull request groue#1248 from groue/dev/WALSnapshot
Avoid double notification of the initial value for ValueObservation on DatabasePool
2 parents 0e97663 + 0aa561e commit 84bee19

File tree

8 files changed

+179
-258
lines changed

8 files changed

+179
-258
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ GRDB adheres to [Semantic Versioning](https://semver.org/), with one exception:
9292

9393
---
9494

95+
## Next Version
96+
97+
- **New**: [#1248](https://github.com/groue/GRDB.swift/pull/1248) by [@groue](https://github.com/groue): Avoid double notification of the initial value for ValueObservation on DatabasePool
98+
99+
95100
## 5.25.0
96101

97102
Released June 11, 2022 • [diff](https://github.com/groue/GRDB.swift/compare/v5.24.1...v5.25.0)

‎GRDB.xcodeproj/project.pbxproj‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,10 @@
733733
56B14E831D4DAE54000BF4A3 /* RowFromDictionaryLiteralTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B14E7E1D4DAE54000BF4A3 /* RowFromDictionaryLiteralTests.swift */; };
734734
56B6EF56208CB4E3002F0ACB /* ColumnExpressionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B6EF55208CB4E3002F0ACB /* ColumnExpressionTests.swift */; };
735735
56B6EF57208CB4E3002F0ACB /* ColumnExpressionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B6EF55208CB4E3002F0ACB /* ColumnExpressionTests.swift */; };
736+
56B7EE832863781300C0525F /* WALSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7EE822863781300C0525F /* WALSnapshot.swift */; };
737+
56B7EE842863781300C0525F /* WALSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7EE822863781300C0525F /* WALSnapshot.swift */; };
738+
56B7EE852863781300C0525F /* WALSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7EE822863781300C0525F /* WALSnapshot.swift */; };
739+
56B7EE862863781300C0525F /* WALSnapshot.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7EE822863781300C0525F /* WALSnapshot.swift */; };
736740
56B7F42A1BE14A1900E39BBF /* CGFloatTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7F4291BE14A1900E39BBF /* CGFloatTests.swift */; };
737741
56B7F43A1BEB42D500E39BBF /* Migration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7F4391BEB42D500E39BBF /* Migration.swift */; };
738742
56B7F43B1BEB42D500E39BBF /* Migration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56B7F4391BEB42D500E39BBF /* Migration.swift */; };
@@ -1595,6 +1599,7 @@
15951599
56B021C81D8C0D3900B239BB /* MutablePersistableRecordPersistenceConflictPolicyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MutablePersistableRecordPersistenceConflictPolicyTests.swift; sourceTree = "<group>"; };
15961600
56B14E7E1D4DAE54000BF4A3 /* RowFromDictionaryLiteralTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RowFromDictionaryLiteralTests.swift; sourceTree = "<group>"; };
15971601
56B6EF55208CB4E3002F0ACB /* ColumnExpressionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ColumnExpressionTests.swift; sourceTree = "<group>"; };
1602+
56B7EE822863781300C0525F /* WALSnapshot.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WALSnapshot.swift; sourceTree = "<group>"; };
15981603
56B7F4291BE14A1900E39BBF /* CGFloatTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CGFloatTests.swift; sourceTree = "<group>"; };
15991604
56B7F4391BEB42D500E39BBF /* Migration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Migration.swift; sourceTree = "<group>"; };
16001605
56B86E72220FF4E000524C16 /* SQLLiteralTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SQLLiteralTests.swift; sourceTree = "<group>"; };
@@ -2375,8 +2380,9 @@
23752380
56A238781B9C75030082EB20 /* Statement.swift */,
23762381
566B912A1FA4D0CC0012D5B0 /* StatementAuthorizer.swift */,
23772382
560D923F1C672C3E00F4F92B /* StatementColumnConvertible.swift */,
2378-
5605F1471C672E4000235C62 /* Support */,
23792383
566B91321FA4D3810012D5B0 /* TransactionObserver.swift */,
2384+
56B7EE822863781300C0525F /* WALSnapshot.swift */,
2385+
5605F1471C672E4000235C62 /* Support */,
23802386
);
23812387
path = Core;
23822388
sourceTree = "<group>";
@@ -2976,6 +2982,7 @@
29762982
565490D81D5AE252005622CB /* DatabaseMigrator.swift in Sources */,
29772983
5656A8AF2295BFD7001FF3FF /* TableRecord+Association.swift in Sources */,
29782984
565490C21D5AE236005622CB /* Row.swift in Sources */,
2985+
56B7EE852863781300C0525F /* WALSnapshot.swift in Sources */,
29792986
565490C71D5AE236005622CB /* StatementColumnConvertible.swift in Sources */,
29802987
565490D91D5AE252005622CB /* Migration.swift in Sources */,
29812988
56CEB5001EAA2F4D00BFAF62 /* FTS3.swift in Sources */,
@@ -3145,6 +3152,7 @@
31453152
5671FC231DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift in Sources */,
31463153
56D51D031EA789FA0074638A /* FetchableRecord+TableRecord.swift in Sources */,
31473154
56DDDDF5242F87C60025E381 /* ValueObservationScheduler.swift in Sources */,
3155+
56B7EE842863781300C0525F /* WALSnapshot.swift in Sources */,
31483156
560D924C1C672C4B00F4F92B /* TableRecord.swift in Sources */,
31493157
56A2FA3B24424F4700E97D23 /* Export.swift in Sources */,
31503158
569BBA4F229170F900478429 /* Inflections+English.swift in Sources */,
@@ -3753,6 +3761,7 @@
37533761
AAA4DCA5230F1E0600C74B15 /* FTS3TokenizerDescriptor.swift in Sources */,
37543762
AAA4DCA6230F1E0600C74B15 /* FetchableRecord+TableRecord.swift in Sources */,
37553763
56DDDDF7242F87C70025E381 /* ValueObservationScheduler.swift in Sources */,
3764+
56B7EE862863781300C0525F /* WALSnapshot.swift in Sources */,
37563765
AAA4DCA7230F1E0600C74B15 /* TableRecord.swift in Sources */,
37573766
56A2FA3D24424F4800E97D23 /* Export.swift in Sources */,
37583767
AAA4DCA8230F1E0600C74B15 /* Inflections+English.swift in Sources */,
@@ -4125,6 +4134,7 @@
41254134
569BBA4E229170F800478429 /* Inflections+English.swift in Sources */,
41264135
5656A81E2295B12F001FF3FF /* SQLAssociation.swift in Sources */,
41274136
5617294E223533F40006E219 /* EncodableRecord.swift in Sources */,
4137+
56B7EE832863781300C0525F /* WALSnapshot.swift in Sources */,
41284138
5698AD181DAAD17A0056AF8C /* FTS5Tokenizer.swift in Sources */,
41294139
56A2FA3624424D2A00E97D23 /* Export.swift in Sources */,
41304140
56B964B11DA51D010002DA19 /* FTS5TokenizerDescriptor.swift in Sources */,

‎GRDB/Core/Database.swift‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -553,38 +553,6 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
553553
_readOnlyDepth > 0 || configuration.readonly
554554
}
555555

556-
// MARK: - Snapshots
557-
558-
#if SQLITE_ENABLE_SNAPSHOT
559-
/// Returns a snapshot that must be freed with `sqlite3_snapshot_free`.
560-
///
561-
/// See <https://www.sqlite.org/c3ref/snapshot.html>
562-
func takeVersionSnapshot() throws -> UnsafeMutablePointer<sqlite3_snapshot> {
563-
var snapshot: UnsafeMutablePointer<sqlite3_snapshot>?
564-
let code = withUnsafeMutablePointer(to: &snapshot) {
565-
sqlite3_snapshot_get(sqliteConnection, "main", $0)
566-
}
567-
guard code == SQLITE_OK else {
568-
throw DatabaseError(resultCode: code, message: lastErrorMessage)
569-
}
570-
if let snapshot = snapshot {
571-
return snapshot
572-
} else {
573-
throw DatabaseError(resultCode: .SQLITE_INTERNAL) // WTF SQLite?
574-
}
575-
}
576-
577-
func wasChanged(since initialSnapshot: UnsafeMutablePointer<sqlite3_snapshot>) throws -> Bool {
578-
let secondSnapshot = try takeVersionSnapshot()
579-
defer {
580-
sqlite3_snapshot_free(secondSnapshot)
581-
}
582-
let cmp = sqlite3_snapshot_cmp(initialSnapshot, secondSnapshot)
583-
assert(cmp <= 0, "Unexpected snapshot ordering")
584-
return cmp < 0
585-
}
586-
#endif
587-
588556
// MARK: - Recording of the selected region
589557

590558
func recordingSelection<T>(_ region: inout DatabaseRegion, _ block: () throws -> T) rethrows -> T {

‎GRDB/Core/DatabaseSnapshot.swift‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,6 @@ public final class DatabaseSnapshot: DatabaseReader {
1414
serializedDatabase.configuration
1515
}
1616

17-
#if SQLITE_ENABLE_SNAPSHOT
18-
typealias Version = UnsafeMutablePointer<sqlite3_snapshot>
19-
// Support for ValueObservation in DatabasePool
20-
let version: Version?
21-
#else
22-
typealias Version = Void
23-
#endif
24-
2517
init(path: String, configuration: Configuration = Configuration(), defaultLabel: String, purpose: String) throws {
2618
var configuration = DatabasePool.readerConfiguration(configuration)
2719
configuration.allowsUnsafeTransactions = true // Snaphost keeps a long-lived transaction
@@ -31,8 +23,8 @@ public final class DatabaseSnapshot: DatabaseReader {
3123
configuration: configuration,
3224
defaultLabel: defaultLabel,
3325
purpose: purpose)
34-
35-
let version: Version? = try serializedDatabase.sync { db in
26+
27+
try serializedDatabase.sync { db in
3628
// Assert WAL mode
3729
let journalMode = try String.fetchOne(db, sql: "PRAGMA journal_mode")
3830
guard journalMode == "wal" else {
@@ -44,29 +36,12 @@ public final class DatabaseSnapshot: DatabaseReader {
4436

4537
// Acquire snapshot isolation
4638
try db.internalCachedStatement(sql: "SELECT rootpage FROM sqlite_master LIMIT 1").makeCursor().next()
47-
48-
#if SQLITE_ENABLE_SNAPSHOT
49-
// We must expect an error: https://www.sqlite.org/c3ref/snapshot_get.html
50-
// > At least one transaction must be written to it first.
51-
return try? db.takeVersionSnapshot()
52-
#else
53-
return nil
54-
#endif
5539
}
56-
57-
#if SQLITE_ENABLE_SNAPSHOT
58-
self.version = version
59-
#endif
6040
}
6141

6242
deinit {
6343
// Leave snapshot isolation
6444
serializedDatabase.reentrantSync { db in
65-
#if SQLITE_ENABLE_SNAPSHOT
66-
if let version = version {
67-
sqlite3_snapshot_free(version)
68-
}
69-
#endif
7045
try? db.commit()
7146
}
7247
}

‎GRDB/Core/WALSnapshot.swift‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/// An instance of WALSnapshot records the state of a WAL mode database for some
2+
/// specific point in history.
3+
///
4+
/// We use `WALSnapshot` to help `ValueObservation` check for changes
5+
/// that would happen between the initial fetch, and the start of the
6+
/// actual observation. This class has no other purpose, and is not intended to
7+
/// become public.
8+
///
9+
/// It does not work with SQLCipher, because SQLCipher does not support
10+
/// `SQLITE_ENABLE_SNAPSHOT` correctly: we have linker errors.
11+
/// See <https://github.com/ericsink/SQLitePCL.raw/issues/452>.
12+
///
13+
/// With custom SQLite builds, it only works if `SQLITE_ENABLE_SNAPSHOT`
14+
/// is defined.
15+
///
16+
/// With system SQLite, it can only work when the SDK exposes the C apis and
17+
/// their availability, which means XCode 14 (identified with Swift 5.7).
18+
///
19+
/// Yes, this is an awfully complex logic.
20+
///
21+
/// See <https://www.sqlite.org/c3ref/snapshot.html>.
22+
final class WALSnapshot {
23+
#if GRDBCIPHER || (GRDBCUSTOMSQLITE && !SQLITE_ENABLE_SNAPSHOT) || compiler(<5.7)
24+
init?(_ db: Database) {
25+
return nil
26+
}
27+
28+
func compare(_ other: WALSnapshot) -> CInt {
29+
preconditionFailure("snapshots are not available")
30+
}
31+
#else
32+
private let snapshot: UnsafeMutablePointer<sqlite3_snapshot>?
33+
34+
/// Returns nil if `SQLITE_ENABLE_SNAPSHOT` is not enabled, or if an
35+
/// error occurs.
36+
init?(_ db: Database) {
37+
var snapshot: UnsafeMutablePointer<sqlite3_snapshot>?
38+
let code: CInt = withUnsafeMutablePointer(to: &snapshot) {
39+
#if GRDBCUSTOMSQLITE
40+
return sqlite3_snapshot_get(db.sqliteConnection, "main", $0)
41+
#else
42+
// iOS 10.0 is always true because our minimum requirement is iOS 11.
43+
if #available(macOS 10.12, watchOS 3.0, tvOS 10.0, *) {
44+
return sqlite3_snapshot_get(db.sqliteConnection, "main", $0)
45+
} else {
46+
return SQLITE_ERROR
47+
}
48+
#endif
49+
}
50+
guard code == SQLITE_OK, let s = snapshot else {
51+
return nil
52+
}
53+
self.snapshot = s
54+
}
55+
56+
deinit {
57+
#if GRDBCUSTOMSQLITE
58+
sqlite3_snapshot_free(snapshot)
59+
#else
60+
// iOS 10.0 is always true because our minimum requirement is iOS 11.
61+
if #available(macOS 10.12, watchOS 3.0, tvOS 10.0, *) {
62+
sqlite3_snapshot_free(snapshot)
63+
}
64+
#endif
65+
}
66+
67+
/// Compares two WAL snapshots.
68+
///
69+
/// `a.compare(b) < 0` iff a is older than b.
70+
///
71+
/// See <https://www.sqlite.org/c3ref/snapshot_cmp.html>.
72+
func compare(_ other: WALSnapshot) -> CInt {
73+
#if GRDBCUSTOMSQLITE
74+
return sqlite3_snapshot_cmp(snapshot, other.snapshot)
75+
#else
76+
// iOS 10.0 is always true because our minimum requirement is iOS 11.
77+
if #available(macOS 10.12, watchOS 3.0, tvOS 10.0, *) {
78+
return sqlite3_snapshot_cmp(snapshot, other.snapshot)
79+
} else {
80+
preconditionFailure("snapshots are not available")
81+
}
82+
#endif
83+
}
84+
#endif // GRDBCIPHER || (GRDBCUSTOMSQLITE && !SQLITE_ENABLE_SNAPSHOT) || compiler(<5.7)
85+
}

0 commit comments

Comments
 (0)