Skip to content

Nim 2.0 fixes #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tuffnatty wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Nim 2.0 fixes #23

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d570885
Nim 2.0 fixes
tuffnatty Jan 4, 2023
f4e826a
Revert transaction logic change; handle transaction exceptions more e…
tuffnatty Jan 10, 2023
e0bb188
Comment typo fix
tuffnatty Jan 10, 2023
eb6cbba
Simplify transaction handling
tuffnatty Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions src/tiny_sqlite.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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.")
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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:")
Expand All @@ -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")
Copy link
Copy Markdown
Owner

@GULPF GULPF Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is why the tests are failing. The body is allowed to contain a return, break, or similar so ok = true might not execute. That the old code is no longer valid seems like an oversight in Nims exception tracking since a bare raise statement cannot raise anything that couldn't already be raised by the code in the try block :/

Copy link
Copy Markdown
Author

@tuffnatty tuffnatty Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! I reverted this change, but had to add more stuff so that compiler would not mark every execMany/execScript call as possibly raising an Exception. Now the tests are passing.

Copy link
Copy Markdown
Author

@tuffnatty tuffnatty Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then I thought if we could understand in the finally block whether an exception is being raised, then we would not have to mess with except/raise; and so it seems, getCurrentException() can be used. So the last version is much simpler.


proc execMany*(db: DbConn, sql: string, params: seq[seq[DbValue]]) =
## Executes ``sql``, which must be a single SQL statement, repeatedly using each element of
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:")
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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``,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
ResultRow(values: row).unpack(T)
6 changes: 3 additions & 3 deletions src/tiny_sqlite/sqlite_wrapper.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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".}
{.cdecl, dynlib: Lib, importc: "sqlite3_load_extension".}