Remove dependency on SQLITE_ENABLE_SNAPSHOT

Address #845 and #846
groue · Oct 6, 2020 · f84e25f · f84e25f
1 parent 6884bf1
commit f84e25f
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 387 deletions.
diff --git a/GRDB/Core/Database.swift b/GRDB/Core/Database.swift
@@ -488,18 +488,17 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
 
     // MARK: - Snapshots
 
-    /// Returns a snapshot that must be freed with `grdb_snapshot_free`.
+    #if SQLITE_ENABLE_SNAPSHOT
+    /// Returns a snapshot that must be freed with `sqlite3_snapshot_free`.
     /// 
     /// See https://www.sqlite.org/c3ref/snapshot.html
     func takeVersionSnapshot() throws -> UnsafeMutablePointer<sqlite3_snapshot> {
         var snapshot: UnsafeMutablePointer<sqlite3_snapshot>?
         let code = withUnsafeMutablePointer(to: &snapshot) {
-            grdb_snapshot_get(sqliteConnection, "main", $0)
+            sqlite3_snapshot_get(sqliteConnection, "main", $0)
         }
         guard code == SQLITE_OK else {
-            // Don't grab `lastErrorMessage`, because grdb_snapshot_get may be a
-            // shim that does not call the missing sqlite3_snapshot_get.
-            throw DatabaseError(resultCode: code)
+            throw DatabaseError(resultCode: code, message: lastErrorMessage)
         }
         if let snapshot = snapshot {
             return snapshot
@@ -511,12 +510,13 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
     func wasChanged(since initialSnapshot: UnsafeMutablePointer<sqlite3_snapshot>) throws -> Bool {
         let secondSnapshot = try takeVersionSnapshot()
         defer {
-            grdb_snapshot_free(secondSnapshot)
+            sqlite3_snapshot_free(secondSnapshot)
         }
-        let cmp = grdb_snapshot_cmp(initialSnapshot, secondSnapshot)
+        let cmp = sqlite3_snapshot_cmp(initialSnapshot, secondSnapshot)
         assert(cmp <= 0, "Unexpected snapshot ordering")
         return cmp < 0
     }
+    #endif
 
     // MARK: - Authorizer
 

diff --git a/GRDB/Core/DatabasePool.swift b/GRDB/Core/DatabasePool.swift
@@ -874,6 +874,7 @@ extension DatabasePool: DatabaseReader {
         //
         // 3. Install the transaction observer.
 
+        #if SQLITE_ENABLE_SNAPSHOT
         if scheduler.immediateInitialValue() {
             do {
                 let initialSnapshot = try makeSnapshot()
@@ -904,10 +905,36 @@ extension DatabasePool: DatabaseReader {
                     }
             }
         }
+        #else
+        if scheduler.immediateInitialValue() {
+            do {
+                let initialValue = try read(observer.fetchInitialValue)
+                onChange(initialValue)
+                addObserver(observer: observer)
+            } catch {
+                observer.complete()
+                observation.events.didFail?(error)
+            }
+        } else {
+            _weakAsyncRead { [weak self] dbResult in
+                guard let self = self, let dbResult = dbResult else { return }
+                if observer.isCompleted { return }
+
+                do {
+                    let initialValue = try observer.fetchInitialValue(dbResult.get())
+                    observer.notifyChange(initialValue)
+                    self.addObserver(observer: observer)
+                } catch {
+                    observer.notifyErrorAndComplete(error)
+                }
+            }
+        }
+        #endif
 
         return observer
     }
 
+    #if SQLITE_ENABLE_SNAPSHOT
     // Support for _addConcurrent(observation:)
     private func add<Reducer: ValueReducer>(
         observer: ValueObserver<Reducer>,
@@ -924,20 +951,11 @@ extension DatabasePool: DatabaseReader {
                     // database versions. It prevents database checkpointing,
                     // and keeps versions (`sqlite3_snapshot`) valid
                     // and comparable.
-                    let fetchNeeded: Bool = withExtendedLifetime(initialSnapshot) {
-                        // Version is nil if SQLite is not compiled with
-                        // SQLITE_ENABLE_SNAPSHOT, or if the DatabaseSnaphot
-                        // could not grab its version. In this case, we don't
-                        // care, and just fetch a fresh value.
+                    let fetchNeeded: Bool = try withExtendedLifetime(initialSnapshot) {
                         guard let initialVersion = initialSnapshot.version else {
                             return true
                         }
-                        do {
-                            return try db.wasChanged(since: initialVersion)
-                        } catch {
-                            // ignore: we'll just re-fetch
-                            return true
-                        }
+                        return try db.wasChanged(since: initialVersion)
                     }
 
                     if fetchNeeded {
@@ -956,6 +974,27 @@ extension DatabasePool: DatabaseReader {
             }
         }
     }
+    #else
+    // Support for _addConcurrent(observation:)
+    private func addObserver<Reducer: ValueReducer>(observer: ValueObserver<Reducer>) {
+        _weakAsyncWriteWithoutTransaction { db in
+            guard let db = db else { return }
+            if observer.isCompleted { return }
+
+            do {
+                observer.events.databaseDidChange?()
+                if let value = try observer.fetchValue(db) {
+                    observer.notifyChange(value)
+                }
+
+                // Now we can start observation
+                db.add(transactionObserver: observer, extent: .observerLifetime)
+            } catch {
+                observer.notifyErrorAndComplete(error)
+            }
+        }
+    }
+    #endif
 }
 
 extension DatabasePool {

diff --git a/GRDB/Core/DatabaseSnapshot.swift b/GRDB/Core/DatabaseSnapshot.swift
@@ -14,8 +14,10 @@ public class DatabaseSnapshot: DatabaseReader {
         serializedDatabase.configuration
     }
 
-    /// Not nil iff SQLite was compiled with `SQLITE_ENABLE_SNAPSHOT`.
+    #if SQLITE_ENABLE_SNAPSHOT
+    // Support for ValueObservation in DatabasePool
     private(set) var version: UnsafeMutablePointer<sqlite3_snapshot>?
+    #endif
 
     init(path: String, configuration: Configuration = Configuration(), defaultLabel: String, purpose: String) throws {
         var configuration = DatabasePool.readerConfiguration(configuration)
@@ -41,17 +43,22 @@ public class DatabaseSnapshot: DatabaseReader {
             // Acquire snapshot isolation
             try db.internalCachedSelectStatement(sql: "SELECT rootpage FROM sqlite_master LIMIT 1").makeCursor().next()
 
-            // Support for ValueObservation in DatabasePool
+            #if SQLITE_ENABLE_SNAPSHOT
+            // We must expect an error: https://www.sqlite.org/c3ref/snapshot_get.html
+            // > At least one transaction must be written to it first.
             version = try? db.takeVersionSnapshot()
+            #endif
         }
     }
 
     deinit {
         // Leave snapshot isolation
         serializedDatabase.reentrantSync { db in
+            #if SQLITE_ENABLE_SNAPSHOT
             if let version = version {
-                grdb_snapshot_free(version)
+                sqlite3_snapshot_free(version)
             }
+            #endif
             try? db.commit()
         }
     }

diff --git a/Makefile b/Makefile
@@ -129,7 +129,7 @@ test_framework_GRDBOSX_maxSwift:
 	  -scheme GRDBOSX \
 	  SWIFT_VERSION=$(MAX_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 
@@ -140,7 +140,7 @@ ifdef MIN_SWIFT_VERSION
 	  -scheme GRDBOSX \
 	  SWIFT_VERSION=$(MIN_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 endif
@@ -164,7 +164,7 @@ test_framework_GRDBiOS_maxTarget_maxSwift:
 	  -destination $(MAX_IOS_DESTINATION) \
 	  SWIFT_VERSION=$(MAX_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 
@@ -176,7 +176,7 @@ ifdef MIN_SWIFT_VERSION
 	  -destination $(MAX_IOS_DESTINATION) \
 	  SWIFT_VERSION=$(MIN_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 endif
@@ -200,7 +200,7 @@ test_framework_GRDBtvOS_maxTarget_maxSwift:
 	  -destination $(MAX_TVOS_DESTINATION) \
 	  SWIFT_VERSION=$(MAX_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 
@@ -212,7 +212,7 @@ ifdef MIN_SWIFT_VERSION
 	  -destination $(MAX_TVOS_DESTINATION) \
 	  SWIFT_VERSION=$(MIN_SWIFT_VERSION) \
 	  'OTHER_SWIFT_FLAGS=$(inherited) -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_PREUPDATE_HOOK' \
-	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1 GRDB_SQLITE_ENABLE_SNAPSHOT=1' \
+	  'GCC_PREPROCESSOR_DEFINITIONS=$(inherited) GRDB_SQLITE_ENABLE_PREUPDATE_HOOK=1' \
 	  $(TEST_ACTIONS) \
 	  $(XCPRETTY)
 endif
@@ -388,10 +388,11 @@ SQLiteCustom: SQLiteCustom/src/sqlite3.h
 	echo '/* Makefile generated */' > SQLiteCustom/GRDBCustomSQLite-USER.h
 	echo '#define SQLITE_ENABLE_PREUPDATE_HOOK' >> SQLiteCustom/GRDBCustomSQLite-USER.h
 	echo '#define SQLITE_ENABLE_FTS5' >> SQLiteCustom/GRDBCustomSQLite-USER.h
+	echo '#define SQLITE_ENABLE_SNAPSHOT' >> SQLiteCustom/GRDBCustomSQLite-USER.h
 	echo '// Makefile generated' > SQLiteCustom/GRDBCustomSQLite-USER.xcconfig
-	echo 'CUSTOM_OTHER_SWIFT_FLAGS = -D SQLITE_ENABLE_PREUPDATE_HOOK -D SQLITE_ENABLE_FTS5' >> SQLiteCustom/GRDBCustomSQLite-USER.xcconfig
+	echo 'CUSTOM_OTHER_SWIFT_FLAGS = -D SQLITE_ENABLE_PREUPDATE_HOOK -D SQLITE_ENABLE_FTS5 -D SQLITE_ENABLE_SNAPSHOT' >> SQLiteCustom/GRDBCustomSQLite-USER.xcconfig
 	echo '// Makefile generated' > SQLiteCustom/src/SQLiteLib-USER.xcconfig
-	echo 'CUSTOM_SQLLIBRARY_CFLAGS = -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_FTS5' >> SQLiteCustom/src/SQLiteLib-USER.xcconfig
+	echo 'CUSTOM_SQLLIBRARY_CFLAGS = -DSQLITE_ENABLE_PREUPDATE_HOOK -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_SNAPSHOT' >> SQLiteCustom/src/SQLiteLib-USER.xcconfig
 
 # Makes sure the SQLiteCustom/src submodule has been downloaded
 SQLiteCustom/src/sqlite3.h:

diff --git a/SQLiteCustom/grdb_config.h b/SQLiteCustom/grdb_config.h
@@ -27,65 +27,4 @@ static inline void enableDoubleQuotedStringLiterals(sqlite3 *db) {
 static inline void disableDoubleQuotedStringLiterals(sqlite3 *db) { }
 static inline void enableDoubleQuotedStringLiterals(sqlite3 *db) { }
 #endif
-
-/*
- Snapshots
- =========
- 
- We have a linker/C-interop difficulty here:
-
- - Not all SQLite versions ship with the sqlite3_snapshot_get function.
- - Not all iOS/macOS versions ship a <sqlite3.h> header that contains
-   the declaration for sqlite3_snapshot_get(), even when SQLite is
-   actually compiled with SQLITE_ENABLE_SNAPSHOT.
-
- This makes it really difficult to deal with system SQLite, custom
- SQLite builds, SQLCipher, and SPM.
-
- To avoid those problems, we add grdb_snapshot_xxx shim functions in the
- following header files:
-
- - SQLiteCustom/grdb_config.h
- - Sources/CSQLite/shim.h
- - Support/grdb_config.h
- */
-#ifdef SQLITE_ENABLE_SNAPSHOT
-static inline int grdb_snapshot_get(
-  sqlite3 *db,
-  const char *zSchema,
-  sqlite3_snapshot **ppSnapshot)
-{
-    return sqlite3_snapshot_get(db, zSchema, ppSnapshot);
-}
-
-static inline void grdb_snapshot_free(sqlite3_snapshot* ppSnapshot) {
-    sqlite3_snapshot_free(ppSnapshot);
-}
-
-static inline int grdb_snapshot_cmp(
-  sqlite3_snapshot *p1,
-  sqlite3_snapshot *p2)
-{
-    return sqlite3_snapshot_cmp(p1, p2);
-}
-#else
-static inline int grdb_snapshot_get(
-  sqlite3 *db,
-  const char *zSchema,
-  sqlite3_snapshot **ppSnapshot)
-{
-    return SQLITE_MISUSE;
-}
-
-static inline void grdb_snapshot_free(sqlite3_snapshot* ppSnapshot) {
-}
-
-static inline int grdb_snapshot_cmp(
-  sqlite3_snapshot *p1,
-  sqlite3_snapshot *p2)
-{
-    return 0;
-}
-#endif /* SQLITE_ENABLE_SNAPSHOT */
-
 #endif /* grdb_config_h */
diff --git a/Sources/CSQLite/shim.h b/Sources/CSQLite/shim.h
@@ -26,82 +26,3 @@ static inline void enableDoubleQuotedStringLiterals(sqlite3 *db) {
 static inline void disableDoubleQuotedStringLiterals(sqlite3 *db) { }
 static inline void enableDoubleQuotedStringLiterals(sqlite3 *db) { }
 #endif
-
-/*
- Snapshots
- =========
- 
- We have a linker/C-interop difficulty here:
-
- - Not all SQLite versions ship with the sqlite3_snapshot_get function.
- - Not all iOS/macOS versions ship a <sqlite3.h> header that contains
-   the declaration for sqlite3_snapshot_get(), even when SQLite is
-   actually compiled with SQLITE_ENABLE_SNAPSHOT.
-
- This makes it really difficult to deal with system SQLite, custom
- SQLite builds, SQLCipher, and SPM.
-
- To avoid those problems, we add grdb_snapshot_xxx shim functions in the
- following header files:
-
- - SQLiteCustom/grdb_config.h
- - Sources/CSQLite/shim.h
- - Support/grdb_config.h
- */
-#ifdef SQLITE_ENABLE_SNAPSHOT
-static inline int grdb_snapshot_get(
-  sqlite3 *db,
-  const char *zSchema,
-  sqlite3_snapshot **ppSnapshot)
-{
-    return sqlite3_snapshot_get(db, zSchema, ppSnapshot);
-}
-
-static inline void grdb_snapshot_free(sqlite3_snapshot* ppSnapshot) {
-    sqlite3_snapshot_free(ppSnapshot);
-}
-
-static inline int grdb_snapshot_cmp(
-  sqlite3_snapshot *p1,
-  sqlite3_snapshot *p2)
-{
-    return sqlite3_snapshot_cmp(p1, p2);
-}
-#else
-// Assume snapshot apis *are* defined, but not exposed.
-typedef struct sqlite3_snapshot {
-  unsigned char hidden[48];
-} sqlite3_snapshot;
-
-SQLITE_API SQLITE_EXPERIMENTAL int sqlite3_snapshot_get(
-  sqlite3 *db,
-  const char *zSchema,
-  sqlite3_snapshot **ppSnapshot
-);
-
-SQLITE_API SQLITE_EXPERIMENTAL void sqlite3_snapshot_free(sqlite3_snapshot*);
-
-SQLITE_API SQLITE_EXPERIMENTAL int sqlite3_snapshot_cmp(
-  sqlite3_snapshot *p1,
-  sqlite3_snapshot *p2
-);
-
-static inline int grdb_snapshot_get(
-  sqlite3 *db,
-  const char *zSchema,
-  sqlite3_snapshot **ppSnapshot)
-{
-    return sqlite3_snapshot_get(db, zSchema, ppSnapshot);
-}
-
-static inline void grdb_snapshot_free(sqlite3_snapshot* ppSnapshot) {
-    sqlite3_snapshot_free(ppSnapshot);
-}
-
-static inline int grdb_snapshot_cmp(
-  sqlite3_snapshot *p1,
-  sqlite3_snapshot *p2)
-{
-    return sqlite3_snapshot_cmp(p1, p2);
-}
-#endif /* SQLITE_ENABLE_SNAPSHOT */