Skip to content

Commit

Permalink
Merge pull request #956 from groue/dev/observation-case-insensitivity
Browse files Browse the repository at this point in the history 
Fix case-sensitivity of region-based database observation
  • Loading branch information
@groue
groue authored Apr 10, 2021
2 parents 47199ed + a44c518 commit c0f537c
Show file tree
Hide file tree
Showing 11 changed files with 465 additions and 53 deletions.
18 changes: 18 additions & 0 deletions GRDB.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@
564CE5AE21B8FAB400652B19 /* DatabaseRegionObservation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564CE5AB21B8FAB400652B19 /* DatabaseRegionObservation.swift */; };
564CE5BE21B8FFA300652B19 /* DatabaseRegionObservationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564CE5BD21B8FFA300652B19 /* DatabaseRegionObservationTests.swift */; };
564CE5BF21B8FFA300652B19 /* DatabaseRegionObservationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564CE5BD21B8FFA300652B19 /* DatabaseRegionObservationTests.swift */; };
564D4F7E261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564D4F7D261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift */; };
564D4F7F261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564D4F7D261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift */; };
564D4F80261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564D4F7D261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift */; };
564E73DF203D50B9000C443C /* JoinSupportTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564E73DE203D50B9000C443C /* JoinSupportTests.swift */; };
564E73E0203D50B9000C443C /* JoinSupportTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564E73DE203D50B9000C443C /* JoinSupportTests.swift */; };
564F9C1E1F069B4E00877A00 /* DatabaseAggregateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 564F9C1D1F069B4E00877A00 /* DatabaseAggregateTests.swift */; };
Expand Down Expand Up @@ -469,6 +472,10 @@
56703297212B5450007D270F /* DatabaseUUIDEncodingStrategyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56703290212B544F007D270F /* DatabaseUUIDEncodingStrategyTests.swift */; };
56703298212B5450007D270F /* DatabaseUUIDEncodingStrategyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56703290212B544F007D270F /* DatabaseUUIDEncodingStrategyTests.swift */; };
567156181CB142AA007DC145 /* DatabaseQueueReadOnlyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 567156151CB142AA007DC145 /* DatabaseQueueReadOnlyTests.swift */; };
56717271261C68E900423B6F /* CaseInsensitiveIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */; };
56717272261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */; };
56717273261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */; };
56717274261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */; };
5671FC201DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5671FC1F1DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift */; };
5671FC231DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5671FC1F1DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift */; };
5671FC261DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5671FC1F1DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift */; };
Expand Down Expand Up @@ -1348,6 +1355,7 @@
564CE59621B7A8B500652B19 /* RemoveDuplicates.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoveDuplicates.swift; sourceTree = "<group>"; };
564CE5AB21B8FAB400652B19 /* DatabaseRegionObservation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseRegionObservation.swift; sourceTree = "<group>"; };
564CE5BD21B8FFA300652B19 /* DatabaseRegionObservationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseRegionObservationTests.swift; sourceTree = "<group>"; };
564D4F7D261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaseInsensitiveIdentifierTests.swift; sourceTree = "<group>"; };
564E73DE203D50B9000C443C /* JoinSupportTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JoinSupportTests.swift; sourceTree = "<group>"; };
564F9C1D1F069B4E00877A00 /* DatabaseAggregateTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseAggregateTests.swift; sourceTree = "<group>"; };
564F9C2C1F075DD200877A00 /* DatabaseFunction.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseFunction.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1419,6 +1427,7 @@
56703290212B544F007D270F /* DatabaseUUIDEncodingStrategyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseUUIDEncodingStrategyTests.swift; sourceTree = "<group>"; };
567156151CB142AA007DC145 /* DatabaseQueueReadOnlyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseQueueReadOnlyTests.swift; sourceTree = "<group>"; };
567156701CB18050007DC145 /* EncryptionTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EncryptionTests.swift; sourceTree = "<group>"; };
56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaseInsensitiveIdentifier.swift; sourceTree = "<group>"; };
5671FC1F1DA3CAC9003BF4FF /* FTS3TokenizerDescriptor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FTS3TokenizerDescriptor.swift; sourceTree = "<group>"; };
5672DE581CDB72520022BA81 /* DatabaseQueueBackupTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseQueueBackupTests.swift; sourceTree = "<group>"; };
5672DE661CDB751D0022BA81 /* DatabasePoolBackupTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabasePoolBackupTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2072,6 +2081,7 @@
5659F4861EA8D94E004A4992 /* Utils */ = {
isa = PBXGroup;
children = (
56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */,
563EF4492161F179007DAACD /* Inflections.swift */,
569BBA482291707D00478429 /* Inflections+English.swift */,
566BE7172342542F00A8254B /* LockedBox.swift */,
Expand Down Expand Up @@ -2150,6 +2160,7 @@
569978D31B539038005EBEED /* Private */ = {
isa = PBXGroup;
children = (
564D4F7D261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift */,
563363CF1C943D13000BE133 /* DatabasePoolReleaseMemoryTests.swift */,
569531281C908A5B00CF1A2B /* DatabasePoolSchemaCacheTests.swift */,
563363D41C94484E000BE133 /* DatabaseQueueReleaseMemoryTests.swift */,
Expand Down Expand Up @@ -2980,6 +2991,7 @@
565490BC1D5AE236005622CB /* DatabaseSchemaCache.swift in Sources */,
563EF42F2161180D007DAACD /* AssociationAggregate.swift in Sources */,
565490BA1D5AE236005622CB /* DatabaseQueue.swift in Sources */,
56717273261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */,
56781B0D243F86E600650A83 /* Refinable.swift in Sources */,
56256EDB25D1B316008C2BDD /* ForeignKey.swift in Sources */,
565490CD1D5AE252005622CB /* Date.swift in Sources */,
Expand Down Expand Up @@ -3119,6 +3131,7 @@
560D92481C672C4B00F4F92B /* PersistableRecord.swift in Sources */,
5613ED4521A95B2C00DC7A68 /* ValueReducer.swift in Sources */,
560D92431C672C3E00F4F92B /* StatementColumnConvertible.swift in Sources */,
56717272261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */,
5613ED3621A95A5C00DC7A68 /* Map.swift in Sources */,
56E9FAD8221053DD00C703A8 /* SQL.swift in Sources */,
56781B0C243F86E600650A83 /* Refinable.swift in Sources */,
Expand Down Expand Up @@ -3269,6 +3282,7 @@
56419C5924A51999004967E1 /* Finished.swift in Sources */,
56A2386A1B9C74A90082EB20 /* RecordSubClassTests.swift in Sources */,
56A2383E1B9C74A90082EB20 /* DatabaseValueTests.swift in Sources */,
564D4F7F261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */,
567156181CB142AA007DC145 /* DatabaseQueueReadOnlyTests.swift in Sources */,
56EA86951C91DFE7002BB4DF /* DatabaseReaderTests.swift in Sources */,
56A8C2471D1918F00096E9D4 /* FoundationNSUUIDTests.swift in Sources */,
Expand Down Expand Up @@ -3499,6 +3513,7 @@
56FEB8F8248403000081AF83 /* DatabaseTraceTests.swift in Sources */,
56419C5124A51998004967E1 /* Finished.swift in Sources */,
56176C5E1EACCCC7000F3F2B /* FTS5WrapperTokenizerTests.swift in Sources */,
564D4F7E261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */,
56FEE7FB1F47253700D930EA /* TableRecordTests.swift in Sources */,
56D496641D81304E008276D7 /* FoundationUUIDTests.swift in Sources */,
56D496921D81316E008276D7 /* RowFromDictionaryLiteralTests.swift in Sources */,
Expand Down Expand Up @@ -3709,6 +3724,7 @@
AAA4DCDF230F1E0600C74B15 /* PersistableRecord.swift in Sources */,
AAA4DCE0230F1E0600C74B15 /* ValueReducer.swift in Sources */,
AAA4DCE2230F1E0600C74B15 /* StatementColumnConvertible.swift in Sources */,
56717274261C68EA00423B6F /* CaseInsensitiveIdentifier.swift in Sources */,
AAA4DCE3230F1E0600C74B15 /* Map.swift in Sources */,
AAA4DCE5230F1E0600C74B15 /* SQL.swift in Sources */,
56781B0E243F86E600650A83 /* Refinable.swift in Sources */,
Expand Down Expand Up @@ -3859,6 +3875,7 @@
56419C6124A5199B004967E1 /* Finished.swift in Sources */,
AAA4DD79230F262000C74B15 /* RecordSubClassTests.swift in Sources */,
AAA4DD7A230F262000C74B15 /* DatabaseValueTests.swift in Sources */,
564D4F80261C6DC200F55856 /* CaseInsensitiveIdentifierTests.swift in Sources */,
AAA4DD7B230F262000C74B15 /* DatabaseQueueReadOnlyTests.swift in Sources */,
AAA4DD7C230F262000C74B15 /* DatabaseReaderTests.swift in Sources */,
AAA4DD7D230F262000C74B15 /* FoundationNSUUIDTests.swift in Sources */,
Expand Down Expand Up @@ -4069,6 +4086,7 @@
56A238831B9C75030082EB20 /* DatabaseQueue.swift in Sources */,
5605F1671C672E4000235C62 /* NSNumber.swift in Sources */,
56E9FADA221053DD00C703A8 /* SQL.swift in Sources */,
56717271261C68E900423B6F /* CaseInsensitiveIdentifier.swift in Sources */,
C96C0F2B2084A442006B2981 /* SQLiteDateParser.swift in Sources */,
56781B0B243F86E600650A83 /* Refinable.swift in Sources */,
56A238871B9C75030082EB20 /* Row.swift in Sources */,
Expand Down
10 changes: 6 additions & 4 deletions GRDB/Core/Database+Schema.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,14 @@ extension Database {
return foreignKeys
}

/// Returns the actual name of the database table, in the main or temp schema.
/// Returns the actual name of the database table, in the main or temp
/// schema, or nil if the table does not exist.
///
/// - throws: A DatabaseError if table does not exist.
func canonicalTableName(_ tableName: String) throws -> String {
func canonicalTableName(_ tableName: String) throws -> String? {
// SQLite has temporary tables shadow main ones
try schema(.temp).canonicalName(tableName, ofType: .table)
?? schema(.main).canonicalName(tableName, ofType: .table)
?? { throw DatabaseError.noSuchTable(tableName) }()
}

func schema(_ schemaID: SchemaIdentifier) throws -> SchemaInfo {
Expand Down Expand Up @@ -865,7 +865,9 @@ struct SchemaInfo: Equatable {
/// try db.schema().canonicalName("foobar", ofType: .table) // "FooBar"
func canonicalName(_ name: String, ofType type: SchemaObjectType) -> String? {
let name = name.lowercased()
return objects.first { $0.name.lowercased() == name }?.name
return objects
.first { $0.type == type.rawValue && $0.name.lowercased() == name }?
.name
}

private struct SchemaObject: Codable, Hashable, FetchableRecord {
Expand Down
66 changes: 42 additions & 24 deletions GRDB/Core/DatabaseRegion.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
/// let request = Player.filter(key: 1)
/// let region = try request.databaseRegion(db)
public struct DatabaseRegion: CustomStringConvertible, Equatable {
private let tableRegions: [String: TableRegion]?
private let tableRegions: [CaseInsensitiveIdentifier: TableRegion]?

private init(tableRegions: [String: TableRegion]?) {
private init(tableRegions: [CaseInsensitiveIdentifier: TableRegion]?) {
self.tableRegions = tableRegions
}

Expand Down Expand Up @@ -64,16 +64,20 @@ public struct DatabaseRegion: CustomStringConvertible, Equatable {
///
/// - parameter table: A table name.
public init(table: String) {
let table = CaseInsensitiveIdentifier(rawValue: table)
self.init(tableRegions: [table: TableRegion(columns: nil, rowIds: nil)])
}

/// Full columns in a table: (some columns in a table) × (all rows)
init(table: String, columns: Set<String>) {
let table = CaseInsensitiveIdentifier(rawValue: table)
let columns = Set(columns.map(CaseInsensitiveIdentifier.init))
self.init(tableRegions: [table: TableRegion(columns: columns, rowIds: nil)])
}

/// Full rows in a table: (all columns in a table) × (some rows)
init(table: String, rowIds: Set<Int64>) {
let table = CaseInsensitiveIdentifier(rawValue: table)
self.init(tableRegions: [table: TableRegion(columns: nil, rowIds: rowIds)])
}

Expand All @@ -86,7 +90,7 @@ public struct DatabaseRegion: CustomStringConvertible, Equatable {
guard let tableRegions = tableRegions else { return other }
guard let otherTableRegions = other.tableRegions else { return self }

var tableRegionsIntersection: [String: TableRegion] = [:]
var tableRegionsIntersection: [CaseInsensitiveIdentifier: TableRegion] = [:]
for (table, tableRegion) in tableRegions {
guard let otherTableRegion = otherTableRegions
.first(where: { (otherTable, _) in otherTable == table })?
Expand All @@ -105,6 +109,7 @@ public struct DatabaseRegion: CustomStringConvertible, Equatable {
return DatabaseRegion(table: table, rowIds: rowIds)
}

let table = CaseInsensitiveIdentifier(rawValue: table)
guard let tableRegion = tableRegions[table] else {
return self
}
Expand All @@ -123,7 +128,7 @@ public struct DatabaseRegion: CustomStringConvertible, Equatable {
guard let tableRegions = tableRegions else { return .fullDatabase }
guard let otherTableRegions = other.tableRegions else { return .fullDatabase }

var tableRegionsUnion: [String: TableRegion] = [:]
var tableRegionsUnion: [CaseInsensitiveIdentifier: TableRegion] = [:]
let tableNames = Set(tableRegions.keys).union(Set(otherTableRegions.keys))
for table in tableNames {
let tableRegion = tableRegions[table]
Expand All @@ -148,26 +153,39 @@ public struct DatabaseRegion: CustomStringConvertible, Equatable {
self = union(other)
}

/// Returns a region suitable for database observation by removing views.
/// Returns a region suitable for database observation
func observableRegion(_ db: Database) throws -> DatabaseRegion {
// SQLite does not expose schema changes to the
// TransactionObserver protocol. By removing internal SQLite tables from
// the observed region, we optimize database observation.
//
// And by canonicalizing table names, we remove views, and help the
// `isModified` methods.
try ignoringInternalSQLiteTables().canonicalTables(db)
}

/// Returns a region only made of actual tables with their canonical names.
/// Canonical names help the `isModified` methods.
///
/// We can do it because modifications only happen in actual tables. And we
/// want to do it because we have a fast path for regions that span a
/// single table.
func ignoringViews(_ db: Database) throws -> DatabaseRegion {
/// This method removes views (assuming no table exists with the same name
/// as a view).
private func canonicalTables(_ db: Database) throws -> DatabaseRegion {
guard let tableRegions = tableRegions else { return .fullDatabase }
let mainViewNames = try db.schema(.main).names(ofType: .view)
let tempViewNames = try db.schema(.temp).names(ofType: .view)
let viewNames = mainViewNames.union(tempViewNames)
guard viewNames.isEmpty == false else { return self }
let filteredRegions = tableRegions.filter { viewNames.contains($0.key) == false }
return DatabaseRegion(tableRegions: filteredRegions)
var region = DatabaseRegion()
for (table, tableRegion) in tableRegions {
if let canonicalTableName = try db.canonicalTableName(table.rawValue) {
let table = CaseInsensitiveIdentifier(rawValue: canonicalTableName)
region.formUnion(DatabaseRegion(tableRegions: [table: tableRegion]))
}
}
return region
}

/// Returns a region which doesn't contain any SQLite internal table.
func ignoringInternalSQLiteTables() -> DatabaseRegion {
private func ignoringInternalSQLiteTables() -> DatabaseRegion {
guard let tableRegions = tableRegions else { return .fullDatabase }
let filteredRegions = tableRegions.filter {
!Database.isSQLiteInternalTable($0.key)
!Database.isSQLiteInternalTable($0.key.rawValue)
}
return DatabaseRegion(tableRegions: filteredRegions)
}
Expand All @@ -194,7 +212,7 @@ extension DatabaseRegion {
return true
}

guard let tableRegion = tableRegions[event.tableName] else {
guard let tableRegion = tableRegions[CaseInsensitiveIdentifier(rawValue: event.tableName)] else {
// FTS4 (and maybe other virtual tables) perform unadvertised
// changes. For example, an "INSERT INTO document ..." statement
// advertises an insertion in the `document` table, but the
Expand Down Expand Up @@ -242,11 +260,11 @@ extension DatabaseRegion {
return "empty"
}
return tableRegions
.sorted(by: { (l, r) in l.key < r.key })
.sorted(by: { (l, r) in l.key.rawValue < r.key.rawValue })
.map({ (table, tableRegion) in
var desc = table
var desc = table.rawValue
if let columns = tableRegion.columns {
desc += "(" + columns.sorted().joined(separator: ",") + ")"
desc += "(" + columns.map(\.rawValue).sorted().joined(separator: ",") + ")"
} else {
desc += "(*)"
}
Expand All @@ -260,7 +278,7 @@ extension DatabaseRegion {
}

private struct TableRegion: Equatable {
var columns: Set<String>? // nil means "all columns"
var columns: Set<CaseInsensitiveIdentifier>? // nil means "all columns"
var rowIds: Set<Int64>? // nil means "all rowids"

var isEmpty: Bool {
Expand All @@ -270,7 +288,7 @@ private struct TableRegion: Equatable {
}

func intersection(_ other: TableRegion) -> TableRegion {
let columnsIntersection: Set<String>?
let columnsIntersection: Set<CaseInsensitiveIdentifier>?
switch (self.columns, other.columns) {
case let (nil, columns), let (columns, nil):
columnsIntersection = columns
Expand All @@ -290,7 +308,7 @@ private struct TableRegion: Equatable {
}

func union(_ other: TableRegion) -> TableRegion {
let columnsUnion: Set<String>?
let columnsUnion: Set<CaseInsensitiveIdentifier>?
switch (self.columns, other.columns) {
case (nil, _), (_, nil):
columnsUnion = nil
Expand Down
Loading

0 comments on commit c0f537c

Please sign in to comment.