Skip to content
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.

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

add basic query implementation #19

Merged
merged 1 commit into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 7 additions & 6 deletions datastore/datastore.nim
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import pkg/questionable/results
import pkg/upraises

import ./key
import ./query

export key
export key, query

push: {.upraises: [].}

Expand Down Expand Up @@ -37,8 +38,8 @@ method put*(

raiseAssert("Not implemented!")

# method query*(
# self: Datastore,
# query: ...): Future[?!(?...)] {.async, base, locks: "unknown".} =
#
# raiseAssert("Not implemented!")
iterator query*(
self: Datastore,
query: Query): Future[QueryResponse] =

raiseAssert("Not implemented!")
2 changes: 1 addition & 1 deletion datastore/key.nim
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ template `[]`*(
proc len*(self: Key): int =
self.namespaces.len

iterator items*(key: Key): Namespace {.inline.} =
iterator items*(key: Key): Namespace =
var
i = 0

Expand Down
10 changes: 5 additions & 5 deletions datastore/null_datastore.nim
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ method put*(

return success()

# method query*(
# self: NullDatastore,
# query: ...): Future[?!(?...)] {.async, locks: "unknown".} =
#
# return success ....none
iterator query*(
self: NullDatastore,
query: Query): Future[QueryResponse] =

discard
18 changes: 18 additions & 0 deletions datastore/query.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import ./key

type
Query* = object
key: QueryKey

QueryKey* = Key

QueryResponse* = tuple[key: Key, data: seq[byte]]

proc init*(
T: type Query,
key: QueryKey): T =

T(key: key)

proc key*(self: Query): QueryKey =
self.key
11 changes: 9 additions & 2 deletions datastore/sqlite.nim
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ type

SQLiteStmt*[Params, Res] = distinct RawStmtPtr

# see https://github.com/arnetheduck/nim-sqlite3-abi/issues/4
sqlite3_destructor_type_gcsafe =
proc (a1: pointer) {.cdecl, gcsafe, upraises: [].}

const
SQLITE_TRANSIENT_GCSAFE* = cast[sqlite3_destructor_type_gcsafe](-1)

proc bindParam(
s: RawStmtPtr,
n: int,
Expand Down Expand Up @@ -248,8 +255,8 @@ proc sqlite3_column_text_not_null*(
# https://www.sqlite.org/c3ref/column_blob.html
# a null pointer here implies an out-of-memory error
let
code = sqlite3_errcode(sqlite3_db_handle(s))
v = sqlite3_errcode(sqlite3_db_handle(s))

raise (ref Defect)(msg: $sqlite3_errstr(code))
raise (ref Defect)(msg: $sqlite3_errstr(v))

text
84 changes: 75 additions & 9 deletions datastore/sqlite_datastore.nim
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import pkg/questionable
import pkg/questionable/results
import pkg/sqlite3_abi
import pkg/stew/byteutils
from pkg/stew/results as stewResults import get, isErr
from pkg/stew/results as stewResults import isErr
import pkg/upraises

import ./datastore
Expand Down Expand Up @@ -34,6 +34,8 @@ type

PutStmt = SQLiteStmt[(string, seq[byte], int64), void]

QueryStmt = SQLiteStmt[(string), void]

SQLiteDatastore* = ref object of Datastore
dbPath: string
containsStmt: ContainsStmt
Expand Down Expand Up @@ -97,6 +99,14 @@ const
) VALUES (?, ?, ?);
"""

queryStmtStr = """
SELECT """ & idColName & """, """ & dataColName & """ FROM """ & tableName &
""" WHERE """ & idColName & """ GLOB ?;
"""

queryStmtIdCol = 0
queryStmtDataCol = 1

proc checkColMetadata(s: RawStmtPtr, i: int, expectedName: string) =
let
colName = sqlite3_column_origin_name(s, i.cint)
Expand Down Expand Up @@ -140,10 +150,10 @@ proc dataCol*(
# result code is an error code
if blob.isNil:
let
code = sqlite3_errcode(sqlite3_db_handle(s))
v = sqlite3_errcode(sqlite3_db_handle(s))

if not (code in [SQLITE_OK, SQLITE_ROW, SQLITE_DONE]):
raise (ref Defect)(msg: $sqlite3_errstr(code))
if not (v in [SQLITE_OK, SQLITE_ROW, SQLITE_DONE]):
raise (ref Defect)(msg: $sqlite3_errstr(v))

let
dataLen = sqlite3_column_bytes(s, i)
Expand Down Expand Up @@ -339,8 +349,64 @@ method put*(

return await self.put(key, data, timestamp())

# method query*(
# self: SQLiteDatastore,
# query: ...): Future[?!(?...)] {.async, locks: "unknown".} =
#
# return success ....some
iterator query*(
self: SQLiteDatastore,
query: Query): Future[QueryResponse] =

let
queryStmt = QueryStmt.prepare(
self.env, queryStmtStr).expect("should not fail")

s = RawStmtPtr(queryStmt)

defer:
discard sqlite3_reset(s)
discard sqlite3_clear_bindings(s)
s.dispose

let
v = sqlite3_bind_text(s, 1.cint, query.key.id.cstring, -1.cint,
SQLITE_TRANSIENT_GCSAFE)

if not (v == SQLITE_OK):
raise (ref Defect)(msg: $sqlite3_errstr(v))
Copy link
Contributor

Choose a reason for hiding this comment

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

As I said before, we should discuss when raise Defect and when return failure, but at this moment let's accept the already implemented approach

Copy link
Contributor Author

@michaelsbradleyjr michaelsbradleyjr Jul 20, 2022

Choose a reason for hiding this comment

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

In this case I wasn't sure of the best approach because we can't return from an inline iterator. We could yield a failure and break the internal loop, or leave it to the user to break in their loop over the iterator when an iteration-step yields a failure. Either way the cleanup in the defer block will be triggered.

Since I wasn't sure, for now I chose to raise Defect on any error, either directly or via nim-result's expect.


while true:
let
v = sqlite3_step(s)

case v
of SQLITE_ROW:
let
key = Key.init($sqlite3_column_text_not_null(
s, queryStmtIdCol)).expect("should not fail")

blob = sqlite3_column_blob(s, queryStmtDataCol)

# detect out-of-memory error
# see the conversion table and final paragraph of:
# https://www.sqlite.org/c3ref/column_blob.html
# see also https://www.sqlite.org/rescode.html

# the "data" column can be NULL so in order to detect an out-of-memory
# error it is necessary to check that the result is a null pointer and
# that the result code is an error code
if blob.isNil:
let
v = sqlite3_errcode(sqlite3_db_handle(s))

if not (v in [SQLITE_OK, SQLITE_ROW, SQLITE_DONE]):
raise (ref Defect)(msg: $sqlite3_errstr(v))
Copy link
Contributor

Choose a reason for hiding this comment

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

This became repetitive pattern, may be replaced sometime in future


let
dataLen = sqlite3_column_bytes(s, queryStmtDataCol)
dataBytes = cast[ptr UncheckedArray[byte]](blob)
data = @(toOpenArray(dataBytes, 0, dataLen - 1))
fut = newFuture[QueryResponse]()
Copy link
Contributor

Choose a reason for hiding this comment

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

will it work with blob==nil ? your testing code for queries doesn't include empty blobs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add some tests for that. Should already work since the approach is roughly the same for method get, which supports empty blobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've accounted for empty blobs in 32596eb.


fut.complete((key, data))
yield fut
of SQLITE_DONE:
break
else:
raise (ref Defect)(msg: $sqlite3_errstr(v))
8 changes: 4 additions & 4 deletions tests/datastore/test_datastore.nim
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ suite "Datastore (base)":
let
key = Key.init("a").get
ds = Datastore()
oneByte = @[1.byte]

asyncTest "put":
expect Defect: discard ds.put(key, oneByte)
expect Defect: discard ds.put(key, @[1.byte])

asyncTest "delete":
expect Defect: discard ds.delete(key)
Expand All @@ -25,5 +24,6 @@ suite "Datastore (base)":
asyncTest "get":
expect Defect: discard ds.get(key)

# asyncTest "query":
# expect Defect: discard ds.query(...)
asyncTest "query":
expect Defect:
for n in ds.query(Query.init(key)): discard
15 changes: 11 additions & 4 deletions tests/datastore/test_null_datastore.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ suite "NullDatastore":
(await ds.get(key)).isOk
(await ds.get(key)).get.isNone

# asyncTest "query":
# check:
# (await ds.query(...)).isOk
# (await ds.query(...)).get.isNone
asyncTest "query":
var
x = true

for n in ds.query(Query.init(key)):
# `iterator query` for NullDatastore never yields so the following lines
# are not run (else the test would hang)
x = false
discard (await n)

check: x
Loading