Skip to content

Commit

Permalink
Merge pull request #384 from groue/conversionDebugInfo
Browse files Browse the repository at this point in the history
Improve database value decoding diagnostics
  • Loading branch information
groue authored Aug 13, 2018
2 parents 566dae0 + 9bc68cd commit 8eee52d
Show file tree
Hide file tree
Showing 44 changed files with 1,511 additions and 277 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,23 @@ Release Notes

## Next Version

- Cursors of optimized values (Strint, Int, Date, etc.) have been renamed: use FastDatabaseValueCursor and FastNullableDatabaseValueCursor instead of the deprecated ColumnCursor and NullableColumnCursor.
- [#384](https://github.com/groue/GRDB.swift/pull/384): Improve database value decoding diagnostics
- [#393](https://github.com/groue/GRDB.swift/pull/393): Upgrade SQLCipher to 3.4.2, enable FTS5 on GRDBCipher and new pod GRDBPlus.

### API diff

```diff
+final class FastDatabaseValueCursor<Value: DatabaseValueConvertible & StatementColumnConvertible> : Cursor { }
+@available(*, deprecated, renamed: "FastDatabaseValueCursor")
+typealias ColumnCursor<Value: DatabaseValueConvertible & StatementColumnConvertible> = FastDatabaseValueCursor<Value>

+final class FastNullableDatabaseValueCursor<Value: DatabaseValueConvertible & StatementColumnConvertible> : Cursor { }
+@available(*, deprecated, renamed: "FastNullableDatabaseValueCursor")
+typealias NullableColumnCursor<Value: DatabaseValueConvertible & StatementColumnConvertible> = FastNullableDatabaseValueCursor<Value>

```

### Documentation Diff

- [Enabling FTS5 Support](README.md#enabling-fts5-support): Procedure for enabling FTS5 support in GRDB
Expand Down
38 changes: 32 additions & 6 deletions GRDB.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion GRDB/Core/DatabasePool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ extension DatabasePool : DatabaseReader {
do {
try db.beginTransaction(.deferred)
assert(db.isInsideTransaction)
try db.makeSelectStatement("SELECT rootpage FROM sqlite_master").cursor().next()
try db.makeSelectStatement("SELECT rootpage FROM sqlite_master").makeCursor().next()
} catch {
readError = error
semaphore.signal() // Release the writer queue and rethrow error
Expand Down
2 changes: 1 addition & 1 deletion GRDB/Core/DatabaseSnapshot.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class DatabaseSnapshot : DatabaseReader {

// Take snapshot
// See DatabasePool.readFromCurrentState for a complete discussion
try db.makeSelectStatement("SELECT rootpage FROM sqlite_master").cursor().next()
try db.makeSelectStatement("SELECT rootpage FROM sqlite_master").makeCursor().next()
}
}

Expand Down
35 changes: 4 additions & 31 deletions GRDB/Core/DatabaseValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ extension DatabaseValue {
// MARK: - Lossless conversions

extension DatabaseValue {
// TODO: deprecate and rename to DatabaseValue.decode(_:sql:arguments:)
/// Converts the database value to the type T.
///
/// let dbValue = "foo".databaseValue
Expand All @@ -226,20 +227,10 @@ extension DatabaseValue {
/// - arguments: Optional statement arguments that enhances the eventual
/// conversion error
public func losslessConvert<T>(sql: String? = nil, arguments: StatementArguments? = nil) -> T where T : DatabaseValueConvertible {
if let value = T.fromDatabaseValue(self) {
return value
}
// Failed conversion: this is data loss, a programmer error.
var error = "could not convert database value \(self) to \(T.self)"
if let sql = sql {
error += " with statement `\(sql)`"
}
if let arguments = arguments, !arguments.isEmpty {
error += " arguments \(arguments)"
}
fatalError(error)
return T.decode(from: self, conversionContext: sql.map { ValueConversionContext(sql: $0, arguments: arguments) })
}

// TODO: deprecate and rename to DatabaseValue.decodeIfPresent(_:sql:arguments:)
/// Converts the database value to the type Optional<T>.
///
/// let dbValue = "foo".databaseValue
Expand All @@ -260,25 +251,7 @@ extension DatabaseValue {
/// - arguments: Optional statement arguments that enhances the eventual
/// conversion error
public func losslessConvert<T>(sql: String? = nil, arguments: StatementArguments? = nil) -> T? where T : DatabaseValueConvertible {
// Use fromDatabaseValue first: this allows DatabaseValue to convert NULL to .null.
if let value = T.fromDatabaseValue(self) {
return value
}
if isNull {
// Failed conversion from null: ok
return nil
} else {
// Failed conversion from a non-null database value: this is data
// loss, a programmer error.
var error = "could not convert database value \(self) to \(T.self)"
if let sql = sql {
error += " with statement `\(sql)`"
}
if let arguments = arguments, !arguments.isEmpty {
error += " arguments \(arguments)"
}
fatalError(error)
}
return T.decodeIfPresent(from: self, conversionContext: sql.map { ValueConversionContext(sql: $0, arguments: arguments) })
}
}

Expand Down
182 changes: 182 additions & 0 deletions GRDB/Core/DatabaseValueConversion.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
#if SWIFT_PACKAGE
import CSQLite
#elseif !GRDBCUSTOMSQLITE && !GRDBCIPHER
import SQLite3
#endif

// MARK: - Conversion Context and Errors

/// A type that helps the user understanding value conversion errors
struct ValueConversionContext {
private enum Column {
case columnIndex(Int)
case columnName(String)
}
var row: Row?
var sql: String?
var arguments: StatementArguments?
private var column: Column?

func atColumn(_ columnIndex: Int) -> ValueConversionContext {
var result = self
result.column = .columnIndex(columnIndex)
return result
}

func atColumn(_ columnName: String) -> ValueConversionContext {
var result = self
result.column = .columnName(columnName)
return result
}

var columnIndex: Int? {
guard let column = column else { return nil }
switch column {
case .columnIndex(let index):
return index
case .columnName(let name):
return row?.index(ofColumn: name)
}
}

var columnName: String? {
guard let column = column else { return nil }
switch column {
case .columnIndex(let index):
guard let row = row else { return nil }
let rowIndex = row.index(row.startIndex, offsetBy: index)
return row[rowIndex].0
case .columnName(let name):
return name
}
}
}

extension ValueConversionContext {
init(_ statement: SelectStatement) {
self.init(
row: Row(statement: statement).copy(),
sql: statement.sql,
arguments: statement.arguments,
column: nil)
}

init(_ row: Row) {
if let statement = row.statement {
self.init(
row: row.copy(),
sql: statement.sql,
arguments: statement.arguments,
column: nil)
} else {
self.init(
row: row.copy(),
sql: nil,
arguments: nil,
column: nil)
}
}

init(sql: String, arguments: StatementArguments?) {
self.init(
row: nil,
sql: sql,
arguments: arguments,
column: nil)
}
}

/// The canonical conversion error message
///
/// - parameter dbValue: nil means "missing column"
func conversionErrorMessage<T>(to: T.Type, from dbValue: DatabaseValue?, conversionContext: ValueConversionContext?) -> String {
var message: String
var extras: [String] = []

if let dbValue = dbValue {
message = "could not convert database value \(dbValue) to \(T.self)"
if let columnName = conversionContext?.columnName {
extras.append("column: `\(columnName)`")
}
if let columnIndex = conversionContext?.columnIndex {
extras.append("column index: \(columnIndex)")
}
} else {
message = "could not read \(T.self) from missing column"
if let columnName = conversionContext?.columnName {
message += " `\(columnName)`"
}
}

if let row = conversionContext?.row {
extras.append("row: \(row)")
}

if let sql = conversionContext?.sql {
extras.append("sql: `\(sql)`")
if let arguments = conversionContext?.arguments, arguments.isEmpty == false {
extras.append("arguments: \(arguments)")
}
}

if extras.isEmpty == false {
message += " (" + extras.joined(separator: ", ") + ")"
}
return message
}

/// The canonical conversion fatal error
///
/// - parameter dbValue: nil means "missing column", for consistency with (row["missing"] as DatabaseValue? == nil)
func fatalConversionError<T>(to: T.Type, from dbValue: DatabaseValue?, conversionContext: ValueConversionContext?, file: StaticString = #file, line: UInt = #line) -> Never {
fatalError(conversionErrorMessage(to: T.self, from: dbValue, conversionContext: conversionContext), file: file, line: line)
}

// MARK: - DatabaseValueConvertible

extension DatabaseValueConvertible {
/// Performs lossless conversion from a database value.
@inline(__always)
static func decode(from dbValue: DatabaseValue, conversionContext: @autoclosure () -> ValueConversionContext?) -> Self {
if let value = fromDatabaseValue(dbValue) {
return value
} else {
fatalConversionError(to: Self.self, from: dbValue, conversionContext: conversionContext())
}
}

/// Performs lossless conversion from a database value.
@inline(__always)
static func decodeIfPresent(from dbValue: DatabaseValue, conversionContext: @autoclosure () -> ValueConversionContext?) -> Self? {
// Use fromDatabaseValue before checking for null: this allows DatabaseValue to convert NULL to .null.
if let value = fromDatabaseValue(dbValue) {
return value
} else if dbValue.isNull {
return nil
} else {
fatalConversionError(to: Self.self, from: dbValue, conversionContext: conversionContext())
}
}
}

// MARK: - DatabaseValueConvertible & StatementColumnConvertible

extension DatabaseValueConvertible where Self: StatementColumnConvertible {
/// Performs lossless conversion from a statement value.
@inline(__always)
static func fastDecode(from sqliteStatement: SQLiteStatement, index: Int32, conversionContext: @autoclosure () -> ValueConversionContext?) -> Self {
if sqlite3_column_type(sqliteStatement, index) == SQLITE_NULL {
fatalConversionError(to: Self.self, from: .null, conversionContext: conversionContext())
}
return self.init(sqliteStatement: sqliteStatement, index: index)
}

/// Performs lossless conversion from a statement value.
@inline(__always)
static func fastDecodeIfPresent(from sqliteStatement: SQLiteStatement, index: Int32) -> Self? {
if sqlite3_column_type(sqliteStatement, index) == SQLITE_NULL {
return nil
}
return self.init(sqliteStatement: sqliteStatement, index: index)
}
}
52 changes: 40 additions & 12 deletions GRDB/Core/DatabaseValueConvertible.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,25 +55,39 @@ public final class DatabaseValueCursor<Value: DatabaseValueConvertible> : Cursor

init(statement: SelectStatement, arguments: StatementArguments? = nil, adapter: RowAdapter? = nil) throws {
self.statement = statement
// We'll read from leftmost column at index 0, unless adapter mangles columns
self.columnIndex = try Int32(adapter?.baseColumnIndex(atIndex: 0, layout: statement) ?? 0)
self.sqliteStatement = statement.sqliteStatement
statement.cursorReset(arguments: arguments)
if let adapter = adapter {
// adapter may redefine the index of the leftmost column
self.columnIndex = try Int32(adapter.baseColumnIndex(atIndex: 0, layout: statement))
} else {
self.columnIndex = 0
}
statement.reset(withArguments: arguments)
}

/// :nodoc:
public func next() throws -> Value? {
if done { return nil }
if done {
// make sure this instance never yields a value again, even if the
// statement is reset by another cursor.
return nil
}
switch sqlite3_step(sqliteStatement) {
case SQLITE_DONE:
done = true
return nil
case SQLITE_ROW:
let dbValue = DatabaseValue(sqliteStatement: sqliteStatement, index: columnIndex)
return dbValue.losslessConvert() as Value
return Value.decode(
from: dbValue,
conversionContext: ValueConversionContext(statement).atColumn(Int(columnIndex)))
case let code:
statement.database.selectStatementDidFail(statement)
throw DatabaseError(resultCode: code, message: statement.database.lastErrorMessage, sql: statement.sql, arguments: statement.arguments)
throw DatabaseError(
resultCode: code,
message: statement.database.lastErrorMessage,
sql: statement.sql,
arguments: statement.arguments)
}
}
}
Expand All @@ -95,25 +109,39 @@ public final class NullableDatabaseValueCursor<Value: DatabaseValueConvertible>

init(statement: SelectStatement, arguments: StatementArguments? = nil, adapter: RowAdapter? = nil) throws {
self.statement = statement
// We'll read from leftmost column at index 0, unless adapter mangles columns
self.columnIndex = try Int32(adapter?.baseColumnIndex(atIndex: 0, layout: statement) ?? 0)
self.sqliteStatement = statement.sqliteStatement
statement.cursorReset(arguments: arguments)
if let adapter = adapter {
// adapter may redefine the index of the leftmost column
self.columnIndex = try Int32(adapter.baseColumnIndex(atIndex: 0, layout: statement))
} else {
self.columnIndex = 0
}
statement.reset(withArguments: arguments)
}

/// :nodoc:
public func next() throws -> Value?? {
if done { return nil }
if done {
// make sure this instance never yields a value again, even if the
// statement is reset by another cursor.
return nil
}
switch sqlite3_step(sqliteStatement) {
case SQLITE_DONE:
done = true
return nil
case SQLITE_ROW:
let dbValue = DatabaseValue(sqliteStatement: sqliteStatement, index: columnIndex)
return dbValue.losslessConvert() as Value?
return Value.decodeIfPresent(
from: dbValue,
conversionContext: ValueConversionContext(statement).atColumn(Int(columnIndex)))
case let code:
statement.database.selectStatementDidFail(statement)
throw DatabaseError(resultCode: code, message: statement.database.lastErrorMessage, sql: statement.sql, arguments: statement.arguments)
throw DatabaseError(
resultCode: code,
message: statement.database.lastErrorMessage,
sql: statement.sql,
arguments: statement.arguments)
}
}
}
Expand Down
Loading

0 comments on commit 8eee52d

Please sign in to comment.