Make column ordering explicit in createRecord return.

The Nim [Row][nim-row] implementation only supports positional
identification of columns. In other words, there is nothing to tell us
which column is in which position. Because of this, we always create SQL
statements which explicitly name the columns we wish to receive so that
we know the order of columns and can rebuild models appropriately.

`createRule` wasn't doing this but naively using `RETURNING *`. This
still works as long as the field ordering in the Nim model class match
the default column ordering returned by the database, but confuses
columns otherwise. This fixes that by specifying explicitly the column
ordering as we do in other places.

[nim-row]: https://nim-lang.org/docs/db_postgres.html#Row

fiber_orm.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version       = "0.3.4"
+version       = "1.0.1"
 author        = "Jonathan Bernard"
 description   = "Lightweight Postgres ORM for Nim."
 license       = "GPL-3.0"
@@ -11,3 +11,4 @@ srcDir        = "src"
 # Dependencies
 
 requires "nim >= 1.4.0"
+requires "https://git.jdb-software.com/jdb/nim-namespaced-logging.git"

src/fiber_orm.nim

@@ -1,9 +1,13 @@
-import db_postgres, macros, options, sequtils, strutils, uuids
+import std/db_postgres, std/macros, std/options, std/sequtils, std/strutils
+import namespaced_logging, uuids
 
-from unicode import capitalize
+from std/unicode import capitalize
 
+import ./fiber_orm/pool
 import ./fiber_orm/util
+
 export
+  pool,
   util.columnNamesForModel,
   util.dbFormat,
   util.dbNameToIdent,
@@ -14,6 +18,12 @@ export
 
 type NotFoundError* = object of CatchableError
 
+var logNs {.threadvar.}: LoggingNamespace
+
+template log(): untyped =
+  if logNs.isNil: logNs = initLoggingNamespace(name = "fiber_orm", level = lvlNotice)
+  logNs
+
 proc newMutateClauses(): MutateClauses =
   return MutateClauses(
     columns: @[],
@@ -24,13 +34,14 @@ proc createRecord*[T](db: DbConn, rec: T): T =
   var mc = newMutateClauses()
   populateMutateClauses(rec, true, mc)
 
-  # Confusingly, getRow allows inserts and updates. We use it to get back the ID
+  let sqlStmt =
-  # we want from the row.
-  let newRow = db.getRow(sql(
     "INSERT INTO " & tableName(rec) &
     " (" & mc.columns.join(",") & ") " &
     " VALUES (" & mc.placeholders.join(",") & ") " &
-    " RETURNING *"), mc.values)
+    " RETURNING " & columnNamesForModel(rec).join(",")
+
+  log().debug "createRecord: [" & sqlStmt & "]"
+  let newRow = db.getRow(sql(sqlStmt), mc.values)
 
   result = rowToModel(T, newRow)
 
@@ -39,24 +50,34 @@ proc updateRecord*[T](db: DbConn, rec: T): bool =
   populateMutateClauses(rec, false, mc)
 
   let setClause = zip(mc.columns, mc.placeholders).mapIt(it[0] & " = " & it[1]).join(",")
-  let numRowsUpdated = db.execAffectedRows(sql(
+  let sqlStmt =
     "UPDATE " & tableName(rec) &
     " SET " & setClause &
-    " WHERE id = ? "), mc.values.concat(@[$rec.id]))
+    " WHERE id = ? "
+
+  log().debug "updateRecord: [" & sqlStmt & "] id: " & $rec.id
+  let numRowsUpdated = db.execAffectedRows(sql(sqlStmt), mc.values.concat(@[$rec.id]))
 
   return numRowsUpdated > 0;
 
 template deleteRecord*(db: DbConn, modelType: type, id: typed): untyped =
-  db.tryExec(sql("DELETE FROM " & tableName(modelType) & " WHERE id = ?"), $id)
+  let sqlStmt = "DELETE FROM " & tableName(modelType) & " WHERE id = ?"
+  log().debug "deleteRecord: [" & sqlStmt & "] id: " & $id
+  db.tryExec(sql(sqlStmt), $id)
 
 proc deleteRecord*[T](db: DbConn, rec: T): bool =
-  return db.tryExec(sql("DELETE FROM " & tableName(rec) & " WHERE id = ?"), $rec.id)
+  let sqlStmt = "DELETE FROM " & tableName(rec) & " WHERE id = ?"
+  log().debug "deleteRecord: [" & sqlStmt & "] id: " & $rec.id
+  return db.tryExec(sql(sqlStmt), $rec.id)
 
 template getRecord*(db: DbConn, modelType: type, id: typed): untyped =
-  let row = db.getRow(sql(
+  let sqlStmt =
     "SELECT " & columnNamesForModel(modelType).join(",") &
     " FROM " & tableName(modelType) &
-    " WHERE id = ?"), @[$id])
+    " WHERE id = ?"
+
+  log().debug "getRecord: [" & sqlStmt & "] id: " & $id
+  let row = db.getRow(sql(sqlStmt), @[$id])
 
   if allIt(row, it.len == 0):
     raise newException(NotFoundError, "no " & modelName(modelType) & " record for id " & $id)
@@ -64,25 +85,31 @@ template getRecord*(db: DbConn, modelType: type, id: typed): untyped =
   rowToModel(modelType, row)
 
 template findRecordsWhere*(db: DbConn, modelType: type, whereClause: string, values: varargs[string, dbFormat]): untyped =
-  db.getAllRows(sql(
+  let sqlStmt =
     "SELECT " & columnNamesForModel(modelType).join(",") &
     " FROM " & tableName(modelType) &
-    " WHERE " & whereClause), values)
+    " WHERE " & whereClause
-    .mapIt(rowToModel(modelType, it))
+
+  log().debug "findRecordsWhere: [" & sqlStmt & "] values: (" & values.join(", ") & ")"
+  db.getAllRows(sql(sqlStmt), values).mapIt(rowToModel(modelType, it))
 
 template getAllRecords*(db: DbConn, modelType: type): untyped =
-  db.getAllRows(sql(
+  let sqlStmt =
     "SELECT " & columnNamesForModel(modelType).join(",") &
-    " FROM " & tableName(modelType)))
+    " FROM " & tableName(modelType)
-    .mapIt(rowToModel(modelType, it))
+
+  log().debug "getAllRecords: [" & sqlStmt & "]"
+  db.getAllRows(sql(sqlStmt)).mapIt(rowToModel(modelType, it))
 
 template findRecordsBy*(db: DbConn, modelType: type, lookups: seq[tuple[field: string, value: string]]): untyped =
-  db.getAllRows(sql(
+  let sqlStmt =
     "SELECT " & columnNamesForModel(modelType).join(",") &
     " FROM " & tableName(modelType) &
-    " WHERE " & lookups.mapIt(it.field & " = ?").join(" AND ")),
+    " WHERE " & lookups.mapIt(it.field & " = ?").join(" AND ")
-    lookups.mapIt(it.value))
+  let values = lookups.mapIt(it.value)
-  .mapIt(rowToModel(modelType, it))
+
+  log().debug "findRecordsBy: [" & sqlStmt & "] values (" & values.join(", ") & ")"
+  db.getAllRows(sql(sqlStmt), values).mapIt(rowToModel(modelType, it))
 
 macro generateProcsForModels*(dbType: type, modelTypes: openarray[type]): untyped =
   result = newStmtList()
@@ -97,14 +124,27 @@ macro generateProcsForModels*(dbType: type, modelTypes: openarray[type]): untype
     let deleteName = ident("delete" & modelName)
     let idType = typeOfColumn(t, "id")
     result.add quote do:
-      proc `getName`*(db: `dbType`, id: `idType`): `t` = getRecord(db.conn, `t`, id)
+      proc `getName`*(db: `dbType`, id: `idType`): `t` =
-      proc `getAllName`*(db: `dbType`): seq[`t`] = getAllRecords(db.conn, `t`)
+        db.withConn: result = getRecord(conn, `t`, id)
+
+      proc `getAllName`*(db: `dbType`): seq[`t`] =
+        db.withConn: result = getAllRecords(conn, `t`)
+
       proc `findWhereName`*(db: `dbType`, whereClause: string, values: varargs[string, dbFormat]): seq[`t`] =
-        return findRecordsWhere(db.conn, `t`, whereClause, values)
+        db.withConn:
-      proc `createName`*(db: `dbType`, rec: `t`): `t` = createRecord(db.conn, rec)
+          result = findRecordsWhere(conn, `t`, whereClause, values)
-      proc `updateName`*(db: `dbType`, rec: `t`): bool = updateRecord(db.conn, rec)
+
-      proc `deleteName`*(db: `dbType`, rec: `t`): bool = deleteRecord(db.conn, rec)
+      proc `createName`*(db: `dbType`, rec: `t`): `t` =
-      proc `deleteName`*(db: `dbType`, id: `idType`): bool = deleteRecord(db.conn, `t`, id)
+        db.withConn: result = createRecord(conn, rec)
+
+      proc `updateName`*(db: `dbType`, rec: `t`): bool =
+        db.withConn: result = updateRecord(conn, rec)
+
+      proc `deleteName`*(db: `dbType`, rec: `t`): bool =
+        db.withConn: result = deleteRecord(conn, rec)
+
+      proc `deleteName`*(db: `dbType`, id: `idType`): bool =
+        db.withConn: result = deleteRecord(conn, `t`, id)
 
 macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyped =
   let fieldNames = fields[1].mapIt($it)
@@ -113,7 +153,7 @@ macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyp
   # Create proc skeleton
   result = quote do:
     proc `procName`*(db: `dbType`): seq[`modelType`] =
-      return findRecordsBy(db.conn, `modelType`)
+      db.withConn: result = findRecordsBy(conn, `modelType`)
 
   var callParams = quote do: @[]
 
@@ -123,10 +163,19 @@ macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyp
     paramTuple.add(newColonExpr(ident("field"), newLit(identNameToDb(n))))
     paramTuple.add(newColonExpr(ident("value"), ident(n)))
 
+    # Add the parameter to the outer call (the generated proc)
+    # result[3] is ProcDef -> [3]: FormalParams
     result[3].add(newIdentDefs(ident(n), ident("string")))
+
+    # Build up the AST for the inner procedure call
     callParams[1].add(paramTuple)
 
-  result[6][0][0].add(callParams)
+  # Add the call params to the inner procedure call
+  # result[6][0][1][0][1] is
+  #   ProcDef -> [6]: StmtList (body) -> [0]: Call ->
+  #     [1]: StmtList (withConn body) -> [0]: Asgn (result =) ->
+  #     [1]: Call (inner findRecords invocation)
+  result[6][0][1][0][1].add(callParams)
 
 macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tuple[t: type, fields: seq[string]]]): untyped =
   result = newStmtList()
@@ -140,7 +189,7 @@ macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tup
     # Create proc skeleton
     let procDefAST = quote do:
       proc `procName`*(db: `dbType`): seq[`modelType`] =
-        return findRecordsBy(db.conn, `modelType`)
+        db.withConn: result = findRecordsBy(conn, `modelType`)
 
     var callParams = quote do: @[]
 
@@ -153,6 +202,28 @@ macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tup
       procDefAST[3].add(newIdentDefs(ident(n), ident("string")))
       callParams[1].add(paramTuple)
 
-    procDefAST[6][0][0].add(callParams)
+    procDefAST[6][0][1][0][1].add(callParams)
 
     result.add procDefAST
+
+proc initPool*(
+    connect: proc(): DbConn,
+    poolSize = 10,
+    hardCap = false,
+    healthCheckQuery = "SELECT 'true' AS alive"): DbConnPool =
+
+  initDbConnPool(DbConnPoolConfig(
+    connect: connect,
+    poolSize: poolSize,
+    hardCap: hardCap,
+    healthCheckQuery: healthCheckQuery))
+
+template inTransaction*(db: DbConnPool, body: untyped) =
+  pool.withConn(db):
+    conn.exec(sql"BEGIN TRANSACTION")
+    try:
+      body
+      conn.exec(sql"COMMIT")
+    except:
+      conn.exec(sql"ROLLBACK")
+      raise getCurrentException()

src/fiber_orm/pool.nim

@@ -0,0 +1,97 @@
+import std/db_postgres, std/sequtils, std/strutils, std/sugar
+
+import namespaced_logging
+
+
+type
+  DbConnPoolConfig* = object
+    connect*: () -> DbConn
+    poolSize*: int
+    hardCap*: bool
+    healthCheckQuery*: string
+
+  PooledDbConn = ref object
+    conn: DbConn
+    id: int
+    free: bool
+
+  DbConnPool* = ref object
+    conns: seq[PooledDbConn]
+    cfg: DbConnPoolConfig
+    lastId: int
+
+var logNs {.threadvar.}: LoggingNamespace
+
+template log(): untyped =
+  if logNs.isNil: logNs = initLoggingNamespace(name = "fiber_orm/pool", level = lvlNotice)
+  logNs
+
+proc initDbConnPool*(cfg: DbConnPoolConfig): DbConnPool =
+  log().debug("Initializing new pool (size: " & $cfg.poolSize)
+  result = DbConnPool(
+    conns: @[],
+    cfg: cfg)
+
+proc newConn(pool: DbConnPool): PooledDbConn =
+  log().debug("Creating a new connection to add to the pool.")
+  pool.lastId += 1
+  let conn = pool.cfg.connect()
+  result = PooledDbConn(
+    conn: conn,
+    id: pool.lastId,
+    free: true)
+  pool.conns.add(result)
+
+proc maintain(pool: DbConnPool): void =
+  log().debug("Maintaining pool. $# connections." % [$pool.conns.len])
+  pool.conns.keepIf(proc (pc: PooledDbConn): bool =
+    if not pc.free: return true
+
+    try:
+      discard getRow(pc.conn, sql(pool.cfg.healthCheckQuery), [])
+      return true
+    except:
+      try: pc.conn.close()  # try to close the connection
+      except: discard ""
+      return false
+  )
+  log().debug(
+    "Pruned dead connections. $# connections remaining." %
+    [$pool.conns.len])
+
+  let freeConns = pool.conns.filterIt(it.free)
+  if pool.conns.len > pool.cfg.poolSize and freeConns.len > 0:
+    let numToCull = min(freeConns.len, pool.conns.len - pool.cfg.poolSize)
+    let toCull = freeConns[0..numToCull]
+    pool.conns.keepIf((pc) => toCull.allIt(it.id != pc.id))
+    for culled in toCull:
+      try: culled.conn.close()
+      except: discard ""
+    log().debug(
+      "Trimming pool size. Culled $# free connections. $# connections remaining." %
+      [$toCull.len, $pool.conns.len])
+
+proc take*(pool: DbConnPool): tuple[id: int, conn: DbConn] =
+  pool.maintain
+  let freeConns = pool.conns.filterIt(it.free)
+
+  log().debug(
+    "Providing a new connection ($# currently free)." % [$freeConns.len])
+
+  let reserved =
+    if freeConns.len > 0: freeConns[0]
+    else: pool.newConn()
+
+  reserved.free = false
+  log().debug("Reserve connection $#" % [$reserved.id])
+  return (id: reserved.id, conn: reserved.conn)
+
+proc release*(pool: DbConnPool, connId: int): void =
+  log().debug("Reclaiming released connaction $#" % [$connId])
+  let foundConn = pool.conns.filterIt(it.id == connId)
+  if foundConn.len > 0: foundConn[0].free = true
+
+template withConn*(pool: DbConnPool, stmt: untyped): untyped =
+  let (connId, conn {.inject.}) = take(pool)
+  stmt
+  release(pool, connId)