Skip to content

Commit

Permalink
#56: fetchOne(_:key:Dictionary) and deleteOne(_:key:Dictionary) raise…
Browse files Browse the repository at this point in the history 
… a fatal error when there is no unique index on the key columns.
  • Loading branch information
@groue
groue committed Jul 6, 2016
1 parent baff716 commit 0f27299
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 45 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 @@ -756,6 +756,14 @@
56FF45451D2C23BA00F21EF9 /* DeleteByKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF453F1D2C23BA00F21EF9 /* DeleteByKeyTests.swift */; };
56FF45461D2C23BA00F21EF9 /* DeleteByKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF453F1D2C23BA00F21EF9 /* DeleteByKeyTests.swift */; };
56FF45471D2C23BA00F21EF9 /* DeleteByKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF453F1D2C23BA00F21EF9 /* DeleteByKeyTests.swift */; };
56FF45561D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF45571D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF45581D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF45591D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF455A1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF455B1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF455C1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
56FF455D1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */; };
DC2393C81ABE35F8003FF113 /* GRDB-Bridging.h in Headers */ = {isa = PBXBuildFile; fileRef = DC2393C61ABE35F8003FF113 /* GRDB-Bridging.h */; settings = {ATTRIBUTES = (Public, ); }; };
DC3773F919C8CBB3004FCF85 /* GRDB.h in Headers */ = {isa = PBXBuildFile; fileRef = DC3773F819C8CBB3004FCF85 /* GRDB.h */; settings = {ATTRIBUTES = (Public, ); }; };
DC70AC991AC2331000371524 /* libsqlite3.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = DC37744219C8DC91004FCF85 /* libsqlite3.dylib */; };
Expand Down Expand Up @@ -1434,6 +1442,7 @@
56FC78651BBAEB8C00CA1285 /* ManagedDataControllerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ManagedDataControllerTests.swift; sourceTree = "<group>"; };
56FDECE11BB32DFD009AD709 /* MetalRowTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MetalRowTests.swift; sourceTree = "<group>"; };
56FF453F1D2C23BA00F21EF9 /* DeleteByKeyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DeleteByKeyTests.swift; sourceTree = "<group>"; };
56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UniqueIndexTests.swift; sourceTree = "<group>"; };
DC2393C61ABE35F8003FF113 /* GRDB-Bridging.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "GRDB-Bridging.h"; path = "../GRDB/GRDB-Bridging.h"; sourceTree = "<group>"; };
DC3773F319C8CBB3004FCF85 /* GRDB.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = GRDB.framework; sourceTree = BUILT_PRODUCTS_DIR; };
DC3773F719C8CBB3004FCF85 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1816,6 +1825,7 @@
569531231C90878D00CF1A2B /* DatabaseQueueSchemaCacheTests.swift */,
56EB0AB11BCD787300A3DC55 /* NSDataMemoryTests.swift */,
5605F1861C69111300235C62 /* StatementInformationTests.swift */,
56FF45551D2CDA5200F21EF9 /* UniqueIndexTests.swift */,
);
path = Private;
sourceTree = "<group>";
Expand Down Expand Up @@ -3092,6 +3102,7 @@
560FC5A21CB00B880014AA8E /* FunctionTests.swift in Sources */,
560FC5A41CB00B880014AA8E /* RecordWithColumnNameManglingTests.swift in Sources */,
560FC5A51CB00B880014AA8E /* CGFloatTests.swift in Sources */,
56FF45571D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
560FC5A61CB00B880014AA8E /* StatementInformationTests.swift in Sources */,
560FC5A71CB00B880014AA8E /* GRDBTestCase.swift in Sources */,
569178471CED9B6000E179EA /* DatabaseQueueTests.swift in Sources */,
Expand Down Expand Up @@ -3202,6 +3213,7 @@
5671565F1CB16729007DC145 /* NSDateComponentsTests.swift in Sources */,
567156601CB16729007DC145 /* FunctionTests.swift in Sources */,
567156611CB16729007DC145 /* RecordWithColumnNameManglingTests.swift in Sources */,
56FF45581D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
567156621CB16729007DC145 /* CGFloatTests.swift in Sources */,
567156631CB16729007DC145 /* StatementInformationTests.swift in Sources */,
567156641CB16729007DC145 /* GRDBTestCase.swift in Sources */,
Expand Down Expand Up @@ -3285,6 +3297,7 @@
56AFCA3B1CB1AA9900F48B96 /* DatabaseMigratorTests.swift in Sources */,
56AFCA3D1CB1AA9900F48B96 /* RecordAwakeFromFetchTests.swift in Sources */,
56AFCA3E1CB1AA9900F48B96 /* DatabasePoolCollationTests.swift in Sources */,
56FF455B1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
56AFCA3F1CB1AA9900F48B96 /* PrimaryKeySingleTests.swift in Sources */,
56AFCA401CB1AA9900F48B96 /* RecordFetchTests.swift in Sources */,
56AFCA411CB1AA9900F48B96 /* StatementColumnConvertibleTests.swift in Sources */,
Expand Down Expand Up @@ -3380,6 +3393,7 @@
56AFCA941CB1ABC800F48B96 /* DatabaseMigratorTests.swift in Sources */,
EED6C2E61D00C0E000F8E701 /* NSNullTests.swift in Sources */,
EE50758B1D00E617005D9C5B /* DatabaseValue+FoundationTests.swift in Sources */,
56FF455C1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
56AFCA961CB1ABC800F48B96 /* RecordAwakeFromFetchTests.swift in Sources */,
56AFCA971CB1ABC800F48B96 /* DatabasePoolCollationTests.swift in Sources */,
EED6C2DC1D00B6D500F8E701 /* NSURLTests.swift in Sources */,
Expand Down Expand Up @@ -3592,6 +3606,7 @@
560C97C81BFD0B8400BF8471 /* FunctionTests.swift in Sources */,
56A2383A1B9C74A90082EB20 /* InMemoryDatabaseTests.swift in Sources */,
56A5E40A1BA2BCF900707640 /* RecordWithColumnNameManglingTests.swift in Sources */,
56FF455A1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
56B7F42A1BE14A1900E39BBF /* CGFloatTests.swift in Sources */,
5605F1881C69111300235C62 /* StatementInformationTests.swift in Sources */,
569178491CED9B6000E179EA /* DatabaseQueueTests.swift in Sources */,
Expand All @@ -3608,6 +3623,7 @@
563363BD1C93FD5E000BE133 /* DatabaseQueueConcurrencyTests.swift in Sources */,
56A238551B9C74A90082EB20 /* PrimaryKeyNoneTests.swift in Sources */,
56A2384F1B9C74A90082EB20 /* MinimalPrimaryKeyRowIDTests.swift in Sources */,
56FF45561D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
561667011D08A49900ADD404 /* NSDecimalNumberTests.swift in Sources */,
569C1EB21CF07DDD0042627B /* DatabaseSchedulerTests.swift in Sources */,
56A238451B9C74A90082EB20 /* DictionaryRowTests.swift in Sources */,
Expand Down Expand Up @@ -3885,6 +3901,7 @@
F3BA81141CFB305E003DC1BA /* QueryInterfaceRequestTests.swift in Sources */,
F3BA81271CFB3063003DC1BA /* RecordEditedTests.swift in Sources */,
F3BA81081CFB3056003DC1BA /* DatabaseCoderTests.swift in Sources */,
56FF455D1D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
F3BA80C71CFB2FD8003DC1BA /* DatabaseQueueFileAttributesTests.swift in Sources */,
F3BA80C51CFB2FD8003DC1BA /* DatabaseQueueBackupTests.swift in Sources */,
F3BA80B31CFB2FC9003DC1BA /* InMemoryDatabaseTests.swift in Sources */,
Expand Down Expand Up @@ -3954,6 +3971,7 @@
F3BA81341CFB3064003DC1BA /* RecordCopyTests.swift in Sources */,
F3BA81101CFB3057003DC1BA /* Row+FoundationTests.swift in Sources */,
F3BA812C1CFB3064003DC1BA /* MinimalPrimaryKeyRowIDTests.swift in Sources */,
56FF45591D2CDA5200F21EF9 /* UniqueIndexTests.swift in Sources */,
561667041D08A49900ADD404 /* NSDecimalNumberTests.swift in Sources */,
F3BA80BF1CFB2FD2003DC1BA /* DatabasePoolBackupTests.swift in Sources */,
F3BA80E31CFB300F003DC1BA /* DatabaseValueConvertibleSubclassTests.swift in Sources */,
Expand Down
54 changes: 33 additions & 21 deletions GRDB/Record/TableMapping.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extension RowConvertible where Self: TableMapping {
/// - returns: A sequence of records.
@warn_unused_result
public static func fetch<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) -> DatabaseSequence<Self> {
guard let statement = fetchByPrimaryKeyStatement(db, keys: keys) else {
guard let statement = try! makeFetchByPrimaryKeyStatement(db, keys: keys) else {
return DatabaseSequence.emptySequence(db)
}
return fetch(statement)
Expand All @@ -59,7 +59,7 @@ extension RowConvertible where Self: TableMapping {
/// - returns: An array of records.
@warn_unused_result
public static func fetchAll<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) -> [Self] {
guard let statement = fetchByPrimaryKeyStatement(db, keys: keys) else {
guard let statement = try! makeFetchByPrimaryKeyStatement(db, keys: keys) else {
return []
}
return fetchAll(statement)
Expand All @@ -78,17 +78,17 @@ extension RowConvertible where Self: TableMapping {
guard let key = key else {
return nil
}
return fetchOne(fetchByPrimaryKeyStatement(db, keys: [key])!)
return try! fetchOne(makeFetchByPrimaryKeyStatement(db, keys: [key])!)
}

// Returns "SELECT * FROM table WHERE id IN (?,?,?)"
//
// Returns nil if values is empty.
@warn_unused_result
private static func fetchByPrimaryKeyStatement<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) -> SelectStatement? {
private static func makeFetchByPrimaryKeyStatement<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) throws -> SelectStatement? {
// Fail early if database table does not exist.
let databaseTableName = self.databaseTableName()
let primaryKey = try! db.primaryKey(databaseTableName)
let primaryKey = try db.primaryKey(databaseTableName)

// Fail early if database table has not one column in its primary key
let columns = primaryKey?.columns ?? []
Expand All @@ -103,14 +103,14 @@ extension RowConvertible where Self: TableMapping {
case 1:
// SELECT * FROM table WHERE id = ?
let sql = "SELECT * FROM \(databaseTableName.quotedDatabaseIdentifier) WHERE \(column.quotedDatabaseIdentifier) = ?"
let statement = try! db.selectStatement(sql)
let statement = try db.selectStatement(sql)
statement.arguments = StatementArguments(keys)
return statement
case let count:
// SELECT * FROM table WHERE id IN (?,?,?)
let keysSQL = databaseQuestionMarks(count: count)
let sql = "SELECT * FROM \(databaseTableName.quotedDatabaseIdentifier) WHERE \(column.quotedDatabaseIdentifier) IN (\(keysSQL))"
let statement = try! db.selectStatement(sql)
let statement = try db.selectStatement(sql)
statement.arguments = StatementArguments(keys)
return statement
}
Expand All @@ -130,7 +130,7 @@ extension TableMapping {
/// - keys: A sequence of primary keys.
/// - returns: The number of deleted rows
public static func deleteAll<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) throws -> Int {
guard let statement = deleteByPrimaryKeyStatement(db, keys: keys) else {
guard let statement = try! makeDeleteByPrimaryKeyStatement(db, keys: keys) else {
return 0
}
try statement.execute()
Expand All @@ -156,10 +156,10 @@ extension TableMapping {
//
// Returns nil if keys is empty.
@warn_unused_result
private static func deleteByPrimaryKeyStatement<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) -> UpdateStatement? {
private static func makeDeleteByPrimaryKeyStatement<Sequence: SequenceType where Sequence.Generator.Element: DatabaseValueConvertible>(db: Database, keys: Sequence) throws -> UpdateStatement? {
// Fail early if database table does not exist.
let databaseTableName = self.databaseTableName()
let primaryKey = try! db.primaryKey(databaseTableName)
let primaryKey = try db.primaryKey(databaseTableName)

// Fail early if database table has not one column in its primary key
let columns = primaryKey?.columns ?? []
Expand All @@ -174,14 +174,14 @@ extension TableMapping {
case 1:
// DELETE FROM table WHERE id = ?
let sql = "DELETE FROM \(databaseTableName.quotedDatabaseIdentifier) WHERE \(column.quotedDatabaseIdentifier) = ?"
let statement = try! db.updateStatement(sql)
let statement = try db.updateStatement(sql)
statement.arguments = StatementArguments(keys)
return statement
case let count:
// DELETE FROM table WHERE id IN (?,?,?)
let keysSQL = databaseQuestionMarks(count: count)
let sql = "DELETE FROM \(databaseTableName.quotedDatabaseIdentifier) WHERE \(column.quotedDatabaseIdentifier) IN (\(keysSQL))"
let statement = try! db.updateStatement(sql)
let statement = try db.updateStatement(sql)
statement.arguments = StatementArguments(keys)
return statement
}
Expand All @@ -205,7 +205,7 @@ extension RowConvertible where Self: TableMapping {
/// - returns: A sequence of records.
@warn_unused_result
public static func fetch(db: Database, keys: [[String: DatabaseValueConvertible?]]) -> DatabaseSequence<Self> {
guard let statement = try! fetchByKeyStatement(db, keys: keys) else {
guard let statement = try! makeFetchByKeyStatement(db, keys: keys) else {
return DatabaseSequence.emptySequence(db)
}
return fetch(statement)
Expand All @@ -223,7 +223,7 @@ extension RowConvertible where Self: TableMapping {
/// - returns: An array of records.
@warn_unused_result
public static func fetchAll(db: Database, keys: [[String: DatabaseValueConvertible?]]) -> [Self] {
guard let statement = try! fetchByKeyStatement(db, keys: keys) else {
guard let statement = try! makeFetchByKeyStatement(db, keys: keys) else {
return []
}
return fetchAll(statement)
Expand All @@ -239,14 +239,17 @@ extension RowConvertible where Self: TableMapping {
/// - returns: An optional record.
@warn_unused_result
public static func fetchOne(db: Database, key: [String: DatabaseValueConvertible?]) -> Self? {
return try! fetchOne(fetchByKeyStatement(db, keys: [key])!)
return try! fetchOne(makeFetchByKeyStatement(db, keys: [key])!)
}

// Returns "SELECT * FROM table WHERE (a = ? AND b = ?) OR (a = ? AND b = ?) ...
//
// Returns nil if keys is empty.
//
// If there is no unique index on the columns, the method raises a fatal
// (unless fatalErrorOnMissingUniqueIndex is false, for testability).
@warn_unused_result
private static func fetchByKeyStatement(db: Database, keys: [[String: DatabaseValueConvertible?]]) throws -> SelectStatement? {
static func makeFetchByKeyStatement(db: Database, keys: [[String: DatabaseValueConvertible?]], fatalErrorOnMissingUniqueIndex: Bool = true) throws -> SelectStatement? {
// Avoid performing useless SELECT
guard keys.count > 0 else {
return nil
Expand All @@ -259,7 +262,11 @@ extension RowConvertible where Self: TableMapping {
GRDBPrecondition(dictionary.count > 0, "Invalid empty key dictionary")
let columns = dictionary.keys
guard try db.hasUniqueIndex(on: databaseTableName, columns: columns) else {
throw DatabaseError(code: SQLITE_MISUSE, message: "table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
if fatalErrorOnMissingUniqueIndex {
fatalError("table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
} else {
throw DatabaseError(code: SQLITE_MISUSE, message: "table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
}
}
arguments.appendContentsOf(dictionary.values)
whereClauses.append("(" + (columns.map { "\($0.quotedDatabaseIdentifier) = ?" } as [String]).joinWithSeparator(" AND ") + ")")
Expand Down Expand Up @@ -287,7 +294,7 @@ extension TableMapping {
/// - keys: An array of key dictionaries.
/// - returns: The number of deleted rows
public static func deleteAll(db: Database, keys: [[String: DatabaseValueConvertible?]]) throws -> Int {
guard let statement = try deleteByKeyStatement(db, keys: keys) else {
guard let statement = try makeDeleteByKeyStatement(db, keys: keys) else {
return 0
}
try statement.execute()
Expand All @@ -310,9 +317,10 @@ extension TableMapping {
//
// Returns nil if keys is empty.
//
// - throws SQLITE_MISUSE if table has no unique index of the requested columns
// If there is no unique index on the columns, the method raises a fatal
// (unless fatalErrorOnMissingUniqueIndex is false, for testability).
@warn_unused_result
private static func deleteByKeyStatement(db: Database, keys: [[String: DatabaseValueConvertible?]]) throws -> UpdateStatement? {
static func makeDeleteByKeyStatement(db: Database, keys: [[String: DatabaseValueConvertible?]], fatalErrorOnMissingUniqueIndex: Bool = true) throws -> UpdateStatement? {
// Avoid performing useless SELECT
guard keys.count > 0 else {
return nil
Expand All @@ -325,7 +333,11 @@ extension TableMapping {
GRDBPrecondition(dictionary.count > 0, "Invalid empty key dictionary")
let columns = dictionary.keys
guard try db.hasUniqueIndex(on: databaseTableName, columns: columns) else {
throw DatabaseError(code: SQLITE_MISUSE, message: "table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
if fatalErrorOnMissingUniqueIndex {
fatalError("table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
} else {
throw DatabaseError(code: SQLITE_MISUSE, message: "table \(databaseTableName) has no unique index on column(s) \(columns.joinWithSeparator(", "))")
}
}
arguments.appendContentsOf(dictionary.values)
whereClauses.append("(" + (columns.map { "\($0.quotedDatabaseIdentifier) = ?" } as [String]).joinWithSeparator(" AND ") + ")")
Expand Down
Loading

0 comments on commit 0f27299

Please sign in to comment.