Files
Baradb/BARADB_DEFICIENCIES.md
T
dimgigov 9756d93cb8
CI / test (push) Has been cancelled
CI / verify (push) Has been cancelled
docs: add deficiency audit and ID generators plan
2026-05-16 03:07:11 +03:00

7.0 KiB

BaraDB Deficiencies Fixed During NimForum Migration

This document summarizes the bugs and design deficiencies discovered in BaraDB while porting NimForum from SQLite to BaraDB's TCP wire protocol.


1. Comma Join Not Supported in Parser

Problem: The BaraQL parser did not recognize comma-separated table lists in the FROM clause.

SELECT * FROM thread t, category c WHERE t.category = c.id

This would fail with "unknown tokens" after the first comma.

Root Cause: parseSelect() only parsed a single table after FROM and ignored everything after a comma.

Fix: Modified database/src/barabadb/query/parser.nim to loop over comma-separated tables and treat each additional table as an implicit CROSS JOIN.

while p.peek().kind == tkComma:
  discard p.advance()
  let nextTableTok = p.expect(tkIdent)
  # ... build joinNode with jkCross and add to result.selJoins

2. DEFAULT Constraints Not Evaluated at Schema Creation Time

Problem: CREATE TABLE with DEFAULT <expr> stored the raw AST node instead of the evaluated string value. During INSERT, applyDefaultValues() checked colDef.defaultVal.len > 0, but defaultVal was empty because the AST was never evaluated to a string.

Result: Explicitly omitted columns in INSERT raised NOT NULL constraint violation even when a DEFAULT was defined.

Fix: Added evalNodeToString() in database/src/barabadb/query/executor.nim to evaluate DEFAULT AST nodes to their string representation during schema restoration.

proc evalNodeToString(node: Node): string =
  let ir = lowerExpr(node)
  return evalExpr(ir, initTable[string, string](), nil)

3. Join Column Resolution Overwrites Duplicate Column Names

Problem: Row is defined as Table[string, string]. When a query selects columns with identical names from joined tables:

SELECT t.id, c.id FROM thread t INNER JOIN category c ON t.category = c.id

The Project operator in executePlan() builds newRow as a Table. The second id overwrites the first, so both columns end up with the value from the right-hand table.

Result: t.id returns 0 (category id) instead of 1 (thread id).

Fix: Two changes in database/src/barabadb/query/executor.nim:

  1. lowerSelect() — for nkPath expressions, use the full path (t.id) as the alias instead of just the last segment (id).
  2. Added uniqueness logic for all aliases: if a duplicate alias is detected, append _1, _2, etc. This also fixes strftime() appearing multiple times in the same select list.
# Before
projectPlan.projectAliases.add(e.pathParts[^1])        # -> "id"
# After
projectPlan.projectAliases.add(e.pathParts.join(".")) # -> "t.id"

4. Empty Result Sets Do Not Send Column Metadata

Problem: In database/src/barabadb/core/server.nim, the server only sent the mkData message when result.rows.len > 0:

if result.rows.len > 0:
  let dataMsg = serializeResult(result, header.requestId)
  await client.send(...)

For queries with zero matching rows (e.g. WHERE id IN (SELECT ...) with no subquery matches), the client received only mkComplete and no mkData.

Result: The client saw columns: @[] and rows: @[]. When getRow() fell back to newSeq[string](qr.columns.len), it returned an empty seq, causing "index out of bounds" errors when the application tried to access row[0].

Fix: Removed the if result.rows.len > 0 guard. serializeResult() is now always sent, including the correct column names even when rowCount = 0.


5. GROUP BY Returns Empty Values for Non-Aggregated Columns

Problem: Unlike SQLite, BaraDB does not automatically pick an arbitrary row value for columns that are neither in GROUP BY nor inside an aggregate function.

SELECT u.id, u.name, count(*)
FROM person u, post p
WHERE p.author = u.id AND p.thread = ?
GROUP BY name

Result: u.id, u.email, u.usrStatus, etc. return empty strings, while name and count(*) are correct.

Fix: (Workaround in forum code) Replaced GROUP BY queries with DISTINCT + separate subqueries where ordering by count is not critical:

SELECT DISTINCT u.id, u.name, u.email, ...
FROM person u, post p
WHERE p.author = u.id AND p.thread = ?
LIMIT 5

6. Inconsistent Aggregate Column Names

Problem: Aggregate functions produce column names that omit the argument expression:

  • SELECT count(*) → column name count()
  • SELECT max(id) → column name max()
  • SELECT min(creation) → column name min()

Code that relies on exact column names (e.g. getValue looking up count(*)) can be confused.

Fix: (Workaround in forum code) Avoided name-dependent lookup and rewrote queries to use positional access via getRow() / getAllRows().


7. Async Client + waitFor in Async Context = Connection Instability

Problem: The original baradb_client.nim used AsyncSocket wrapped in SyncClient via waitFor. When called from inside Jester's async handlers, nested waitFor + poll() created race conditions on the single socket. recv(12) occasionally returned 0 bytes, causing "Connection closed" exceptions.

Result: Login and other routes crashed intermittently with 502 Bad Gateway.

Fix: Built a new synchronous client (forum/src/baradb_sync_client.nim) using blocking net.Socket from Nim's standard library. This eliminates all async event loop interactions.

Key differences from the async client:

  • Uses net.Socket instead of asyncnet.AsyncSocket
  • Uses blocking recv() with an explicit recvExact() helper
  • No dependency on asyncdispatch or waitFor
  • Fully compatible with the existing wire protocol

8. Lack of Thread Safety in the Adapter

Problem: A single SyncClient instance was shared across all HTTP requests. NimForum runs on Jester's async event loop, which can interleave request handlers in the same thread. Without synchronization, two handlers could send queries over the same socket simultaneously, corrupting the wire protocol stream.

Fix: Added a global Lock and withDbLock template in forum/src/baradb_sqlite.nim:

var dbLock: Lock
initLock(dbLock)

template withDbLock(body: untyped) =
  acquire(dbLock)
  try: body
  finally: release(dbLock)

All DB operations (query, getRow, exec, etc.) are wrapped in this lock.


Summary Table

# Issue Location Type
1 Comma join parsing query/parser.nim Parser bug
2 DEFAULT not evaluated query/executor.nim Schema bug
3 Duplicate column overwrite query/executor.nim Data structure bug
4 Empty result = no columns core/server.nim Protocol bug
5 GROUP BY empty values N/A (engine behavior) Semantic difference
6 Inconsistent agg names N/A (engine behavior) Naming convention
7 Async client unstable forum/src/baradb_client.nim Client design
8 No thread safety forum/src/baradb_sqlite.nim Adapter design

Document version: 2026-05-15