From d5708854376230956d79699a60a33e806a7ea2cb Mon Sep 17 00:00:00 2001 From: Phil Krylov Date: Wed, 4 Jan 2023 14:15:59 +0100 Subject: [PATCH 1/4] Nim 2.0 fixes --- src/tiny_sqlite.nim | 75 ++++++++++++++++-------------- src/tiny_sqlite/sqlite_wrapper.nim | 6 +-- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/src/tiny_sqlite.nim b/src/tiny_sqlite.nim index b9dce98..9f25fb5 100644 --- a/src/tiny_sqlite.nim +++ b/src/tiny_sqlite.nim @@ -13,7 +13,7 @@ when not declared(tupleLen): export options.get, options.isSome, options.isNone type - DbConnImpl = ref object + DbConnImpl = ref object handle: sqlite.Sqlite3 ## The underlying SQLite3 handle cache: StmtCache @@ -66,9 +66,9 @@ const SqliteRcOk = [ sqlite.SQLITE_OK, sqlite.SQLITE_DONE, sqlite.SQLITE_ROW ] # Forward declarations -proc isInTransaction*(db: DbConn): bool {.noSideEffect.} -proc isOpen*(db: DbConn): bool {.noSideEffect, inline.} -proc isAlive*(statement: SqlStatement): bool {.noSideEffect.} +func isInTransaction*(db: DbConn): bool {.raises: [].} +func isOpen*(db: DbConn): bool {.inline, raises: [].} +func isAlive*(statement: SqlStatement): bool {.raises: [].} template handle(db: DbConn): sqlite.Sqlite3 = DbConnImpl(db).handle template handle(statement: SqlStatement): sqlite.Stmt = SqlStatementImpl(statement).handle @@ -105,7 +105,7 @@ proc skipLeadingWhiteSpaceAndComments(sql: var cstring) = let original = sql template `&+`(s: cstring, offset: int): cstring = - cast[cstring](cast[ByteAddress](sql) + offset) + cast[cstring](cast[uint](sql) + offset) while true: case sql[0] @@ -173,7 +173,7 @@ proc toDbValue*[T: type(nil)](val: T): DbValue = proc toDbValues*(values: varargs[DbValue, toDbValue]): seq[DbValue] = ## Convert several values to a sequence of DbValue's. runnableExamples: - doAssert toDbValues("string", 23) == @[toDbValue("string"), toDbValue(23)] + doAssert toDbValues("string", 23) == @[toDbValue("string"), toDbValue(23)] @values proc fromDbValue*(value: DbValue, T: typedesc[Ordinal]): T = @@ -204,9 +204,9 @@ proc fromDbValue*(value: DbValue, T: typedesc[DbValue]): T = ## The purpose of this overload is to do partial unpacking. ## For example, if the type of one column in a result row is unknown, ## the DbValue type can be kept just for that column. - ## + ## ## .. code-block:: nim - ## + ## ## for row in db.iterate("SELECT name, extra FROM Person"): ## # Type of 'extra' is unknown, so we don't unpack it. ## # The 'extra' variable will be of type 'DbValue' @@ -241,7 +241,7 @@ proc `==`*(a, b: DbValue): bool = proc bindParams(db: DbConn, stmtHandle: sqlite.Stmt, params: varargs[DbValue]): Rc = result = sqlite.SQLITE_OK - let expectedParamsLen = sqlite.bind_parameter_count(stmtHandle) + let expectedParamsLen = sqlite.bind_parameter_count(stmtHandle) if expectedParamsLen != params.len: raise newSqliteError("SQL statement contains " & $expectedParamsLen & " parameters but only " & $params.len & " was provided.") @@ -256,7 +256,7 @@ proc bindParams(db: DbConn, stmtHandle: sqlite.Stmt, params: varargs[DbValue]): sqlite.bind_int64(stmtHandle, idx, value.intval) of sqliteReal: sqlite.bind_double(stmtHandle, idx, value.floatVal) - of sqliteText: + of sqliteText: sqlite.bind_text(stmtHandle, idx, value.strVal.cstring, value.strVal.len.int32, sqlite.SQLITE_TRANSIENT) of sqliteBlob: sqlite.bind_blob(stmtHandle, idx.int32, cast[string](value.blobVal).cstring, @@ -266,7 +266,8 @@ proc bindParams(db: DbConn, stmtHandle: sqlite.Stmt, params: varargs[DbValue]): return rc idx.inc -proc prepareSql(db: DbConn, sql: string): sqlite.Stmt = +proc prepareSql(db: DbConn, sql: string): sqlite.Stmt + {.raises: [SqliteError].} = var tail: cstring let rc = sqlite.prepare_v2(db.handle, sql.cstring, sql.len.cint + 1, result, tail) db.checkRc(rc) @@ -338,7 +339,8 @@ iterator iterate(db: DbConn, stmtOrHandle: sqlite.Stmt | SqlStatement, params: v # DbConn # -proc exec*(db: DbConn, sql: string, params: varargs[DbValue, toDbValue]) = +proc exec*(db: DbConn, sql: string, params: varargs[DbValue, toDbValue]) + {.raises: [SqliteError].} = ## Executes ``sql``, which must be a single SQL statement. runnableExamples: let db = openDatabase(":memory:") @@ -361,17 +363,12 @@ template transaction*(db: DbConn, body: untyped) = body else: db.exec("BEGIN") - var ok = true + var ok = false try: - try: - body - except Exception: - ok = false - db.exec("ROLLBACK") - raise + body + ok = true finally: - if ok: - db.exec("COMMIT") + db.exec(if ok: "COMMIT" else: "ROLLBACK") proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) = ## Executes ``sql``, which must be a single SQL statement, repeatedly using each element of @@ -381,7 +378,7 @@ proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) = for p in params: db.exec(sql, p) -proc execScript*(db: DbConn, sql: string) = +proc execScript*(db: DbConn, sql: string) {.raises: [SqliteError].} = ## Executes ``sql``, which can consist of multiple SQL statements. ## The statements are executed inside a transaction. assertCanUseDb db @@ -399,7 +396,8 @@ proc execScript*(db: DbConn, sql: string) = remaining.skipLeadingWhiteSpaceAndComments() iterator iterate*(db: DbConn, sql: string, - params: varargs[DbValue, toDbValue]): ResultRow = + params: varargs[DbValue, toDbValue]): ResultRow + {.raises: [SqliteError].} = ## Executes ``sql``, which must be a single SQL statement, and yields each result row one by one. assertCanUseDb db let stmtHandle = db.prepareSql(sql, @params) @@ -437,7 +435,7 @@ proc value*(db: DbConn, sql: string, for row in db.iterate(sql, params): return some(row.values[0]) -proc close*(db: DbConn) = +proc close*(db: DbConn) {.raises: [SqliteError].} = ## Closes the database connection. This should be called once the connection will no longer be used ## to avoid leaking memory. Closing an already closed database is a harmless no-op. if not db.isOpen: @@ -480,7 +478,7 @@ proc isReadonly*(db: DbConn): bool = assertCanUseDb db sqlite.db_readonly(db.handle, "main") == 1 -proc isOpen*(db: DbConn): bool {.inline.} = +func isOpen*(db: DbConn): bool {.inline, raises: [].} = ## Returns true if `db` has been opened and not yet closed. runnableExamples: var db: DbConn @@ -491,7 +489,7 @@ proc isOpen*(db: DbConn): bool {.inline.} = doAssert not db.isOpen (not DbConnImpl(db).isNil) and (not db.handle.isNil) -proc isInTransaction*(db: DbConn): bool = +func isInTransaction*(db: DbConn): bool {.raises: [].} = ## Returns true if a transaction is currently active. runnableExamples: let db = openDatabase(":memory:") @@ -517,8 +515,9 @@ proc stmt*(db: DbConn, sql: string): SqlStatement = assertCanUseDb db let handle = prepareSql(db, sql) SqlStatementImpl(handle: handle, db: db).SqlStatement - -proc exec*(statement: SqlStatement, params: varargs[DbValue, toDbValue]) = + +proc exec*(statement: SqlStatement, params: varargs[DbValue, toDbValue]) + {.raises: [SqliteError].} = ## Executes `statement` with `params` as parameters. assertCanUseStatement statement var rc = statement.db.bindParams(statement.handle, params) @@ -538,7 +537,9 @@ proc execMany*(statement: SqlStatement, params: seq[seq[DbValue]]) = for p in params: statement.exec(p) -iterator iterate*(statement: SqlStatement, params: varargs[DbValue, toDbValue]): ResultRow = +iterator iterate*(statement: SqlStatement, + params: varargs[DbValue, toDbValue]): ResultRow + {.raises: [SqliteError].} = ## Executes ``statement`` and yields each result row one by one. assertCanUseStatement statement var errorRc: int32 @@ -568,7 +569,7 @@ proc one*(statement: SqlStatement, proc value*(statement: SqlStatement, params: varargs[DbValue, toDbValue]): Option[DbValue] = - ## Executes `statement` and returns the first column of the first row found. + ## Executes `statement` and returns the first column of the first row found. ## Returns `none(DbValue)` if no result was found. assertCanUseStatement statement for row in statement.iterate(params): @@ -582,12 +583,15 @@ proc finalize*(statement: SqlStatement): void = discard sqlite.finalize(statement.handle) SqlStatementImpl(statement).handle = nil -proc isAlive*(statement: SqlStatement): bool = +func isAlive*(statement: SqlStatement): bool + {.raises: [].} = ## Returns true if ``statement`` has been initialized and not yet finalized. (not SqlStatementImpl(statement).isNil) and (not statement.handle.isNil) and (not statement.db.handle.isNil) -proc openDatabase*(path: string, mode = dbReadWrite, cacheSize: Natural = 100): DbConn = +proc openDatabase*(path: string, mode = dbReadWrite, + cacheSize: Natural = 100): DbConn + {.raises: [SqliteError].} = ## Open a new database connection to a database file. To create an ## in-memory database the special path `":memory:"` can be used. ## If the database doesn't already exist and ``mode`` is ``dbReadWrite``, @@ -615,7 +619,8 @@ proc openDatabase*(path: string, mode = dbReadWrite, cacheSize: Natural = 100): result.exec("PRAGMA foreign_keys = ON") when not defined(macosx): - proc loadExtension*(db: DbConn, path: string) = + proc loadExtension*(db: DbConn, path: string) + {.raises: [SqliteError].} = ## Load an SQLite extension. Will raise a ``SqliteError`` exception if loading fails. db.checkRc sqlite.db_config(db.handle, sqlite.SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, 0); var err: cstring @@ -672,11 +677,11 @@ proc unpack*[T: tuple](row: ResultRow, _: typedesc[T]): T = proc rows*(db: DbConn, sql: string, params: varargs[DbValue, toDbValue]): seq[seq[DbValue]] {.deprecated: "use 'all' instead".} = db.all(sql, params).mapIt(it.values) - + iterator rows*(db: DbConn, sql: string, params: varargs[DbValue, toDbValue]): seq[DbValue] {.deprecated: "use 'iterate' instead".} = for row in db.all(sql, params): yield row.values proc unpack*[T: tuple](row: seq[DbValue], _: typedesc[T]): T {.deprecated.} = - ResultRow(values: row).unpack(T) \ No newline at end of file + ResultRow(values: row).unpack(T) diff --git a/src/tiny_sqlite/sqlite_wrapper.nim b/src/tiny_sqlite/sqlite_wrapper.nim index 4c0c690..762de6c 100644 --- a/src/tiny_sqlite/sqlite_wrapper.nim +++ b/src/tiny_sqlite/sqlite_wrapper.nim @@ -21,7 +21,7 @@ type {.cdecl, raises: [].} SqliteDestructor* = proc (p: pointer) - {.cdecl, locks: 0, tags: [], raises: [], gcsafe.} + {.cdecl, tags: [], raises: [], gcsafe.} const SQLITE_OK* = 0.cint @@ -96,7 +96,7 @@ const SQLITE_ALTER_TABLE* = 26.cint SQLITE_REINDEX* = 27.cint SQLITE_DENY* = 1.cint - SQLITE_IGNORE* = 2.cint + SQLITE_IGNORE* = 2.cint SQLITE_DETERMINISTIC* = 0x800.cint const @@ -291,4 +291,4 @@ proc free*(z: cstring) when not defined(macosx): proc load_extension*(db: Sqlite3, filename: cstring, entry: cstring, error: var cstring): cint - {.cdecl, dynlib: Lib, importc: "sqlite3_load_extension".} \ No newline at end of file + {.cdecl, dynlib: Lib, importc: "sqlite3_load_extension".} From f4e826ad5045151b1fb796f7f731aeae3d0ea687 Mon Sep 17 00:00:00 2001 From: Phil Krylov Date: Tue, 10 Jan 2023 19:21:15 +0100 Subject: [PATCH 2/4] Revert transaction logic change; handle transaction exceptions more explicitly to pacify Nim exception tracking --- src/tiny_sqlite.nim | 55 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/src/tiny_sqlite.nim b/src/tiny_sqlite.nim index 9f25fb5..05bbacb 100644 --- a/src/tiny_sqlite.nim +++ b/src/tiny_sqlite.nim @@ -97,6 +97,10 @@ proc newSqliteError(msg: string): ref SqliteError = ## Raises a SqliteError exception. (ref SqliteError)(msg: msg) +proc newSqliteError(exc: ref Exception): ref SqliteError = + ## Raises a SqliteError exception on an internal error. + (ref SqliteError)(msg: "Unexpected error: " & exc.msg) + template checkRc(db: DbConn, rc: Rc) = if rc notin SqliteRcOk: raise newSqliteError(db) @@ -363,26 +367,54 @@ template transaction*(db: DbConn, body: untyped) = body else: db.exec("BEGIN") - var ok = false + var ok = true try: - body - ok = true + # Rollback on anything we can catch. It's unclear what the future + # of Nim exception handling must be, but the transaction should not + # live on. + {.push warning[BareExcept]: off.} + try: + body + except: + ok = false + db.exec("ROLLBACK") + raise + {.pop.} finally: - db.exec(if ok: "COMMIT" else: "ROLLBACK") - -proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) = + if ok: + db.exec("COMMIT") + +template transactionInternal(db: DbConn, body: untyped) = + ## Use this for internal transactions when it is evident that body only raises SqliteError. + ## Any unexpected CatchableError gets converted to SqliteError to pacify exception tracking. + {.push warning[BareExcept]: off.} + # We handle all possible cases explicitly here so disable warnings + try: + db.transaction: + body + except Defect as e: + raise e + except SqliteError as e: + raise e + except Exception as e: # should not happen if used as prescribed + raise newSqliteError(e) + {.pop.} + +proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) + {.raises: [SqliteError].} = ## Executes ``sql``, which must be a single SQL statement, repeatedly using each element of ## ``params`` as parameters. The statements are executed inside a transaction. assertCanUseDb db - db.transaction: + db.transactionInternal: for p in params: db.exec(sql, p) -proc execScript*(db: DbConn, sql: string) {.raises: [SqliteError].} = +proc execScript*(db: DbConn, sql: string) + {.raises: [SqliteError].} = ## Executes ``sql``, which can consist of multiple SQL statements. ## The statements are executed inside a transaction. assertCanUseDb db - db.transaction: + db.transactionInternal: var remaining = sql.cstring while remaining.len > 0: var tail: cstring @@ -529,11 +561,12 @@ proc exec*(statement: SqlStatement, params: varargs[DbValue, toDbValue]) resetStmt(statement.handle) statement.db.checkRc(rc) -proc execMany*(statement: SqlStatement, params: seq[seq[DbValue]]) = +proc execMany*(statement: SqlStatement, params: seq[seq[DbValue]]) + {.raises: [SqliteError].} = ## Executes ``statement`` repeatedly using each element of ``params`` as parameters. ## The statements are executed inside a transaction. assertCanUseStatement statement - statement.db.transaction: + statement.db.transactionInternal: for p in params: statement.exec(p) From e0bb18879a2099965ac3bb87122cacedee81d764 Mon Sep 17 00:00:00 2001 From: Phil Krylov Date: Tue, 10 Jan 2023 19:32:34 +0100 Subject: [PATCH 3/4] Comment typo fix --- src/tiny_sqlite.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tiny_sqlite.nim b/src/tiny_sqlite.nim index 05bbacb..9f3a85b 100644 --- a/src/tiny_sqlite.nim +++ b/src/tiny_sqlite.nim @@ -386,7 +386,7 @@ template transaction*(db: DbConn, body: untyped) = template transactionInternal(db: DbConn, body: untyped) = ## Use this for internal transactions when it is evident that body only raises SqliteError. - ## Any unexpected CatchableError gets converted to SqliteError to pacify exception tracking. + ## Any unexpected Exception gets converted to SqliteError to pacify exception tracking. {.push warning[BareExcept]: off.} # We handle all possible cases explicitly here so disable warnings try: From eb6cbba8c54a7777b21773ad71e046e45d657600 Mon Sep 17 00:00:00 2001 From: Phil Krylov Date: Tue, 10 Jan 2023 19:40:47 +0100 Subject: [PATCH 4/4] Simplify transaction handling --- src/tiny_sqlite.nim | 42 +++++------------------------------------- 1 file changed, 5 insertions(+), 37 deletions(-) diff --git a/src/tiny_sqlite.nim b/src/tiny_sqlite.nim index 9f3a85b..8ac1391 100644 --- a/src/tiny_sqlite.nim +++ b/src/tiny_sqlite.nim @@ -97,10 +97,6 @@ proc newSqliteError(msg: string): ref SqliteError = ## Raises a SqliteError exception. (ref SqliteError)(msg: msg) -proc newSqliteError(exc: ref Exception): ref SqliteError = - ## Raises a SqliteError exception on an internal error. - (ref SqliteError)(msg: "Unexpected error: " & exc.msg) - template checkRc(db: DbConn, rc: Rc) = if rc notin SqliteRcOk: raise newSqliteError(db) @@ -367,45 +363,17 @@ template transaction*(db: DbConn, body: untyped) = body else: db.exec("BEGIN") - var ok = true try: - # Rollback on anything we can catch. It's unclear what the future - # of Nim exception handling must be, but the transaction should not - # live on. - {.push warning[BareExcept]: off.} - try: - body - except: - ok = false - db.exec("ROLLBACK") - raise - {.pop.} - finally: - if ok: - db.exec("COMMIT") - -template transactionInternal(db: DbConn, body: untyped) = - ## Use this for internal transactions when it is evident that body only raises SqliteError. - ## Any unexpected Exception gets converted to SqliteError to pacify exception tracking. - {.push warning[BareExcept]: off.} - # We handle all possible cases explicitly here so disable warnings - try: - db.transaction: body - except Defect as e: - raise e - except SqliteError as e: - raise e - except Exception as e: # should not happen if used as prescribed - raise newSqliteError(e) - {.pop.} + finally: + db.exec(if getCurrentException() != nil: "ROLLBACK" else: "COMMIT") proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) {.raises: [SqliteError].} = ## Executes ``sql``, which must be a single SQL statement, repeatedly using each element of ## ``params`` as parameters. The statements are executed inside a transaction. assertCanUseDb db - db.transactionInternal: + db.transaction: for p in params: db.exec(sql, p) @@ -414,7 +382,7 @@ proc execScript*(db: DbConn, sql: string) ## Executes ``sql``, which can consist of multiple SQL statements. ## The statements are executed inside a transaction. assertCanUseDb db - db.transactionInternal: + db.transaction: var remaining = sql.cstring while remaining.len > 0: var tail: cstring @@ -566,7 +534,7 @@ proc execMany*(statement: SqlStatement, params: seq[seq[DbValue]]) ## Executes ``statement`` repeatedly using each element of ``params`` as parameters. ## The statements are executed inside a transaction. assertCanUseStatement statement - statement.db.transactionInternal: + statement.db.transaction: for p in params: statement.exec(p)