check column metadata in id/data/timestampCol

The goal is to detect mismatches between caller-supplied indexes and original
column names, and in that case crash the process by raising Defect. This should
help avoid harder to debug errors for such mismatches.

These helper procs are now higher-order, which allows column metadata checks to
be run only once, i.e. when setting up derivative helpers to be used in an
`onData` callback.

Use compile-time constants for column names and default indexes.

Adjust callback proc annotations to be more precise, and remove unnecessary
annotations.

datastore/sqlite.nim

@@ -3,16 +3,22 @@ import pkg/questionable/results
 import pkg/sqlite3_abi
 import pkg/upraises
 
-push: {.upraises: [].}
-
 # Adapted from:
 # https://github.com/status-im/nwaku/blob/master/waku/v2/node/storage/sqlite.nim
 
+# see https://www.sqlite.org/c3ref/column_database_name.html
+# can pass `--forceBuild:on` to the Nim compiler if a SQLite build without
+# `-DSQLITE_ENABLE_COLUMN_METADATA` option is stuck in the build cache,
+# e.g. `nimble test --forceBuild:on`
+{.passC: "-DSQLITE_ENABLE_COLUMN_METADATA".}
+
+push: {.upraises: [].}
+
 type
   AutoDisposed*[T: ptr|ref] = object
     val*: T
 
-  DataProc* = proc(s: RawStmtPtr) {.closure.}
+  DataProc* = proc(s: RawStmtPtr) {.closure, gcsafe.}
 
   NoParams* = tuple # empty tuple
 

datastore/sqlite_datastore.nim

@@ -17,6 +17,12 @@ export datastore, sqlite
 push: {.upraises: [].}
 
 type
+  BoundIdCol = proc (): string {.closure, gcsafe, upraises: [].}
+
+  BoundDataCol = proc (): seq[byte] {.closure, gcsafe, upraises: [].}
+
+  BoundTimestampCol = proc (): int64 {.closure, gcsafe, upraises: [].}
+
   # feels odd to use `void` for prepared statements corresponding to SELECT
   # queries but it fits with the rest of the SQLite wrapper adapted from
   # status-im/nwaku, at least in its current form in ./sqlite
@@ -33,17 +39,26 @@ type
     containsStmt: ContainsStmt
     deleteStmt: DeleteStmt
     env: SQLite
+    getDataCol: BoundDataCol
     getStmt: GetStmt
     putStmt: PutStmt
     readOnly: bool
 
 const
-  IdType = "TEXT"
-  DataType = "BLOB"
-  TimestampType = "INTEGER"
-
   dbExt* = ".sqlite3"
-  tableTitle* = "Store"
+  tableName* = "Store"
+
+  idColName = "id"
+  dataColName = "data"
+  timestampColName = "timestamp"
+
+  idColIndex = 0
+  dataColIndex = 1
+  timestampColIndex = 2
+
+  idColType = "TEXT"
+  dataColType = "BLOB"
+  timestampColType = "INTEGER"
 
   # https://stackoverflow.com/a/9756276
   # EXISTS returns a boolean value represented by an integer:
@@ -51,35 +66,128 @@ const
   # https://sqlite.org/lang_expr.html#the_exists_operator
   containsStmtStr = """
     SELECT EXISTS(
-      SELECT 1 FROM """ & tableTitle & """
+      SELECT 1 FROM """ & tableName & """
-      WHERE id = ?
+      WHERE """ & idColName & """ = ?
     );
   """
 
   createStmtStr = """
-    CREATE TABLE IF NOT EXISTS """ & tableTitle & """ (
+    CREATE TABLE IF NOT EXISTS """ & tableName & """ (
-      id """ & IdType & """ NOT NULL PRIMARY KEY,
+      """ & idColName & """ """ & idColType & """ NOT NULL PRIMARY KEY,
-      data """ & DataType & """,
+      """ & dataColName & """ """ & dataColType & """,
-      timestamp """ & TimestampType & """ NOT NULL
+      """ & timestampColName & """ """ & timestampColType & """ NOT NULL
     ) WITHOUT ROWID;
   """
 
   deleteStmtStr = """
-    DELETE FROM """ & tableTitle & """
+    DELETE FROM """ & tableName & """
-    WHERE id = ?;
+    WHERE """ & idColName & """ = ?;
   """
 
   getStmtStr = """
-    SELECT data FROM """ & tableTitle & """
+    SELECT """ & dataColName & """ FROM """ & tableName & """
-    WHERE id = ?;
+    WHERE """ & idColName & """ = ?;
   """
 
   putStmtStr = """
-    REPLACE INTO """ & tableTitle & """ (
+    REPLACE INTO """ & tableName & """ (
-      id, data, timestamp
+      """ & idColName & """,
+      """ & dataColName & """,
+      """ & timestampColName & """
     ) VALUES (?, ?, ?);
   """
 
+proc idCol*(
+  s: RawStmtPtr,
+  index = idColIndex): BoundIdCol =
+
+  let
+    i = index.cint
+    colName = sqlite3_column_origin_name(s, i)
+
+  if colName.isNil or $colName != idColName:
+    raise (ref Defect)(msg: "original column name for index " & $index &
+      " was not \"" & idColName & "\"")
+
+  return proc (): string =
+    let
+      text = sqlite3_column_text(s, i)
+
+    # detect out-of-memory error
+    # see the conversion table and final paragraph of:
+    # https://www.sqlite.org/c3ref/column_blob.html
+
+    # the "id" column is NOT NULL PRIMARY KEY so an out-of-memory error can be
+    # inferred from a null pointer result
+    if text.isNil:
+      let
+        code = sqlite3_errcode(sqlite3_db_handle(s))
+
+      raise (ref Defect)(msg: $sqlite3_errstr(code))
+
+    $text.cstring
+
+proc dataCol*(
+  s: RawStmtPtr,
+  index = dataColIndex): BoundDataCol =
+
+  let
+    i = index.cint
+    colName = sqlite3_column_origin_name(s, i)
+
+  if colName.isNil or $colName != dataColName:
+    raise (ref Defect)(msg: "original column name for index " & $index &
+      " was not \"" & dataColName & "\"")
+
+  return proc (): seq[byte] =
+    let
+      blob = sqlite3_column_blob(s, i)
+
+    # 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
+        code = sqlite3_errcode(sqlite3_db_handle(s))
+
+      if not (code in [SQLITE_OK, SQLITE_ROW, SQLITE_DONE]):
+        raise (ref Defect)(msg: $sqlite3_errstr(code))
+
+    let
+      dataLen = sqlite3_column_bytes(s, i)
+
+    # an out-of-memory error can be inferred from a null pointer result
+    if (unsafeAddr dataLen).isNil:
+      let
+        code = sqlite3_errcode(sqlite3_db_handle(s))
+
+      raise (ref Defect)(msg: $sqlite3_errstr(code))
+
+    let
+      dataBytes = cast[ptr UncheckedArray[byte]](blob)
+
+    @(toOpenArray(dataBytes, 0, dataLen - 1))
+
+proc timestampCol*(
+  s: RawStmtPtr,
+  index = timestampColIndex): BoundTimestampCol =
+
+  let
+    i = index.cint
+    colName = sqlite3_column_origin_name(s, i)
+
+  if colName.isNil or $colName != timestampColName:
+    raise (ref Defect)(msg: "original column name for index " & $index &
+      " was not \"" & timestampColName & "\"")
+
+  return proc (): int64 =
+    sqlite3_column_int64(s, i)
+
 proc new*(
   T: type SQLiteDatastore,
   basePath = "data",
@@ -157,9 +265,12 @@ proc new*(
   # `pepare()` will fail and `new` will return an error with message
   # "SQL logic error"
 
+  let
+    getDataCol = dataCol(RawStmtPtr(getStmt), 0)
+
   success T(dbPath: dbPath, containsStmt: containsStmt, deleteStmt: deleteStmt,
-            env: env.release, getStmt: getStmt, putStmt: putStmt,
+            env: env.release, getStmt: getStmt, getDataCol: getDataCol,
-            readOnly: readOnly)
+            putStmt: putStmt, readOnly: readOnly)
 
 proc dbPath*(self: SQLiteDatastore): string =
   self.dbPath
@@ -181,71 +292,6 @@ proc close*(self: SQLiteDatastore) =
 proc timestamp*(t = epochTime()): int64 =
   (t * 1_000_000).int64
 
-proc idCol*(
-  s: RawStmtPtr,
-  index = 0): string =
-
-  let
-    text = sqlite3_column_text(s, index.cint).cstring
-
-  # detect out-of-memory error
-  # see the conversion table and final paragraph of:
-  # https://www.sqlite.org/c3ref/column_blob.html
-
-  # the "id" column is NOT NULL PRIMARY KEY so an out-of-memory error can be
-  # inferred from a null pointer result
-  if text.isNil:
-    let
-      code = sqlite3_errcode(sqlite3_db_handle(s))
-
-    raise (ref Defect)(msg: $sqlite3_errstr(code))
-
-  $text
-
-proc dataCol*(
-  s: RawStmtPtr,
-  index = 1): seq[byte] =
-
-  let
-    i = index.cint
-    blob = sqlite3_column_blob(s, i)
-
-  # 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
-      code = sqlite3_errcode(sqlite3_db_handle(s))
-
-    if not (code in [SQLITE_OK, SQLITE_ROW, SQLITE_DONE]):
-      raise (ref Defect)(msg: $sqlite3_errstr(code))
-
-  let
-    dataLen = sqlite3_column_bytes(s, i)
-
-  # an out-of-memory error can be inferred from a null pointer result
-  if (unsafeAddr dataLen).isNil:
-    let
-      code = sqlite3_errcode(sqlite3_db_handle(s))
-
-    raise (ref Defect)(msg: $sqlite3_errstr(code))
-
-  let
-    dataBytes = cast[ptr UncheckedArray[byte]](blob)
-
-  @(toOpenArray(dataBytes, 0, dataLen - 1))
-
-proc timestampCol*(
-  s: RawStmtPtr,
-  index = 2): int64 =
-
-  sqlite3_column_int64(s, index.cint)
-
 method contains*(
   self: SQLiteDatastore,
   key: Key): Future[?!bool] {.async, locks: "unknown".} =
@@ -253,7 +299,7 @@ method contains*(
   var
     exists = false
 
-  proc onData(s: RawStmtPtr) {.closure.} =
+  proc onData(s: RawStmtPtr) =
     exists = sqlite3_column_int64(s, 0).bool
 
   let
@@ -283,8 +329,11 @@ method get*(
   var
     bytes: ?seq[byte]
 
-  proc onData(s: RawStmtPtr) {.closure.} =
+  let
-    bytes = dataCol(s, 0).some
+    dataCol = self.getDataCol
+
+  proc onData(s: RawStmtPtr) =
+    bytes = dataCol().some
 
   let
     queryRes = self.getStmt.query((key.id), onData)

tests/datastore/test_sqlite_datastore.nim

@@ -136,7 +136,17 @@ suite "SQLiteDatastore":
     check: putRes.isOk
 
     let
-      query = "SELECT * FROM " & tableTitle & ";"
+      prequeryRes = NoParamsStmt.prepare(
+        ds.env, "SELECT timestamp as foo, id as baz, data as bar FROM " &
+          tableName & ";")
+
+    assert prequeryRes.isOk
+
+    let
+      prequery = prequeryRes.get
+      idCol = idCol(RawStmtPtr(prequery), 1)
+      dataCol = dataCol(RawStmtPtr(prequery), 2)
+      timestampCol = timestampCol(RawStmtPtr(prequery), 0)
 
     var
       qId: string
@@ -145,13 +155,13 @@ suite "SQLiteDatastore":
       rowCount = 0
 
     proc onData(s: RawStmtPtr) {.closure.} =
-      qId = idCol(s)
+      qId = idCol()
-      qData = dataCol(s)
+      qData = dataCol()
-      qTimestamp = timestampCol(s)
+      qTimestamp = timestampCol()
       inc rowCount
 
     var
-      qRes = ds.env.query(query, onData)
+      qRes = prequery.query((), onData)
 
     assert qRes.isOk
 
@@ -169,7 +179,7 @@ suite "SQLiteDatastore":
     check: putRes.isOk
 
     rowCount = 0
-    qRes = ds.env.query(query, onData)
+    qRes = prequery.query((), onData)
     assert qRes.isOk
 
     check:
@@ -186,7 +196,7 @@ suite "SQLiteDatastore":
     check: putRes.isOk
 
     rowCount = 0
-    qRes = ds.env.query(query, onData)
+    qRes = prequery.query((), onData)
     assert qRes.isOk
 
     check:
@@ -196,6 +206,8 @@ suite "SQLiteDatastore":
       qTimestamp == timestamp
       rowCount == 1
 
+    prequery.dispose
+
   asyncTest "delete":
     let
       bytes = @[1.byte, 2.byte, 3.byte]
@@ -225,7 +237,7 @@ suite "SQLiteDatastore":
     assert putRes.isOk
 
     let
-      query = "SELECT * FROM " & tableTitle & ";"
+      query = "SELECT * FROM " & tableName & ";"
 
     var
       rowCount = 0