diff --git a/.gitignore b/.gitignore index bb7aca5..05fc769 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ benchmarks/bench_all benchmarks/compare clients/nim/tests/test_client src/baradadb +src/barabadb/client/client src/barabadb/core/raft # Temp diff --git a/BARADB_DEFICIENCIES.md b/BARADB_DEFICIENCIES.md index 82f5c50..b70e872 100644 --- a/BARADB_DEFICIENCIES.md +++ b/BARADB_DEFICIENCIES.md @@ -89,7 +89,7 @@ For queries with zero matching rows (e.g. `WHERE id IN (SELECT ...)` with no sub ## 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. +**Problem:** Unlike SQLite, BaraDB did not automatically pick an arbitrary row value for columns that are neither in `GROUP BY` nor inside an aggregate function. ```sql SELECT u.id, u.name, count(*) @@ -98,30 +98,40 @@ 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. +**Result:** `u.id`, `u.email`, `u.usrStatus`, etc. returned empty strings, while `name` and `count(*)` were correct. -**Fix:** (Workaround in forum code) Replaced `GROUP BY` queries with `DISTINCT` + separate subqueries where ordering by count is not critical: +**Fix:** Modified `query/executor.nim` — the `irpkGroupBy` execution path now populates non-aggregated columns from the first row in each group (SQLite behavior). The forum workaround using `DISTINCT` is no longer necessary. -```sql -SELECT DISTINCT u.id, u.name, u.email, ... -FROM person u, post p -WHERE p.author = u.id AND p.thread = ? -LIMIT 5 +```nim +# Populate non-aggregated columns from first row in group +if groupRows.len > 0: + for k, v in groupRows[0]: + if not k.startsWith("$") and k notin aggRow: + aggRow[k] = v ``` --- ## 6. Inconsistent Aggregate Column Names -**Problem:** Aggregate functions produce column names that omit the argument expression: +**Problem:** Aggregate functions produced column names that omitted 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. +Code that relies on exact column names (e.g. `getValue` looking up `count(*)`) could be confused. -**Fix:** (Workaround in forum code) Avoided name-dependent lookup and rewrote queries to use positional access via `getRow()` / `getAllRows()`. +**Fix:** Modified `query/executor.nim` — `lowerSelect()` now builds aliases using `exprToSql(arg)` for function arguments: + +```nim +elif e.kind == nkFuncCall: + var aliasArgs: seq[string] = @[] + for arg in e.funcArgs: aliasArgs.add(exprToSql(arg)) + projectPlan.projectAliases.add(e.funcName & "(" & aliasArgs.join(", ") & ")") +``` + +Result: `SELECT count(*)` now produces column name `count(*)`, matching user expectations. --- @@ -131,12 +141,13 @@ Code that relies on exact column names (e.g. `getValue` looking up `count(*)`) c **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. +**Fix:** Rewrote `SyncClient` in both `clients/nim/src/baradb/client.nim` and `src/barabadb/client/client.nim` to use 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` +Key changes: +- `SyncClient.socket` is now `net.Socket` instead of `AsyncSocket` +- `connect()` uses blocking `net.connect()` instead of `waitFor asyncClient.connect()` +- `query()` uses blocking `recv()` via a `recvExact()` helper instead of `waitFor` +- No dependency on `asyncdispatch` or `waitFor` in sync path - Fully compatible with the existing wire protocol --- @@ -145,19 +156,26 @@ Key differences from the async client: **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`: +**Fix:** Integrated a `Lock` directly into `SyncClient` in both client libraries. Every public operation (`query`, `exec`, `auth`, `ping`, `close`) acquires the lock before touching the socket: ```nim -var dbLock: Lock -initLock(dbLock) +type + SyncClient* = ref object + config: ClientConfig + socket: net.Socket + connected: bool + requestId: uint32 + lock: Lock -template withDbLock(body: untyped) = - acquire(dbLock) - try: body - finally: release(dbLock) +proc query*(client: SyncClient, sql: string): QueryResult = + acquire(client.lock) + try: + ... socket I/O ... + finally: + release(client.lock) ``` -All DB operations (`query`, `getRow`, `exec`, etc.) are wrapped in this lock. +The external `withDbLock` workaround in the forum adapter is no longer necessary because the client itself is now thread-safe. --- @@ -219,10 +237,10 @@ This allows empty strings to be stored in NOT NULL columns while still properly | 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 | +| 5 | GROUP BY empty values | `query/executor.nim` | Semantic difference | +| 6 | Inconsistent agg names | `query/executor.nim` | Naming convention | +| 7 | Async client unstable | `clients/nim/src/baradb/client.nim` | Client design | +| 8 | No thread safety | `clients/nim/src/baradb/client.nim` | Adapter design | | 9 | `key` reserved keyword | `query/lexer.nim` | Parser limitation | | 10 | Empty string = NULL | `query/executor.nim` | Data handling bug | diff --git a/PLAN.md b/PLAN.md index 473296e..aee953e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -137,9 +137,9 @@ |---|--------|--------|--------| | 9.1.1 | Почистване на 9-те build warnings (ResultShadowed + UnusedImport) | 1ч | ✅ | | 9.1.2 | Issue #6: Aggregate column names (`count(*)` → `count(*)`, `max(id)` → `max(id)`) | 2ч | ✅ | -| 9.1.3 | Issue #5: GROUP BY bare columns — първи ред от групата за non-aggregated колони | 4-6ч | 🔄 | -| 9.1.4 | Issue #7+8: Решение за async vs sync client + thread safety | 2ч | 🔄 | -| 9.1.5 | Regression тестове за всички 10 deficiencies | 2ч | 🔄 | +| 9.1.3 | Issue #5: GROUP BY bare columns — първи ред от групата за non-aggregated колони | 4-6ч | ✅ | +| 9.1.4 | Issue #7+8: Решение за async vs sync client + thread safety | 2ч | ✅ | +| 9.1.5 | Regression тестове за всички 10 deficiencies | 2ч | ✅ | **Метрика:** NimForum миграционният код маха всички `DISTINCT` workaround-и за GROUP BY. @@ -149,10 +149,10 @@ | # | Задача | Оценка | Статус | |---|--------|--------|--------| -| 9.2.1 | `IRExpr` носи `expectedType` — всеки AST node знае дали е INT, FLOAT, TEXT, NULL | 4-6ч | 🔄 | -| 9.2.2 | `evalExpr` връща discriminated union (`Value(kind: vkInt64/Float64/String/Null)`) вместо само `string` | 6-8ч | 🔄 | -| 9.2.3 | `irAdd`/`irSub`/`irMul`/`irDiv` използват типовата информация (INT+INT → INT, INT+FLOAT → FLOAT) | 3ч | 🔄 | -| 9.2.4 | `validateType` използва `Value.kind` вместо `parseInt`/`parseFloat` на string | 2ч | 🔄 | +| 9.2.1 | `IRExpr` носи `valueKind` — всеки AST node знае дали е INT, FLOAT, TEXT, NULL | 4-6ч | ✅ | +| 9.2.2 | `evalExprValue` връща discriminated union (`Value(kind: vkInt64/Float64/String/Null)`) вместо само `string` | 6-8ч | ✅ | +| 9.2.3 | `irAdd`/`irSub`/`irMul`/`irDiv` използват типовата информация (INT+INT → INT, INT+FLOAT → FLOAT) | 3ч | ✅ | +| 9.2.4 | `validateType` използва `Value.kind` вместо `parseInt`/`parseFloat` на string | 2ч | ✅ | **Метрика:** Премахваме всички `try: parseFloat catch: return fallback` евристики от `evalExpr`. @@ -194,11 +194,13 @@ --- -### Текущи метрики (преди сесия 9) +### Текущи метрики (след сесия 9 — Sedmica 1+2) | Метрика | Стойност | |---------|----------| -| **Тестове** | 294 — 0 failures | -| **Build warnings** | 9 (3× ResultShadowed + 6× UnusedImport) | -| **BARADB_DEFICIENCIES** | 4 непоправени (#5, #6, #7, #8) | -| **Workaround-и в NimForum** | 2 (GROUP BY → DISTINCT, aggregate positional access) | +| **Тестове** | 311 — 0 failures | +| **Build warnings** | 0 | +| **BARADB_DEFICIENCIES** | 0 непоправени (всички 10 поправени) | +| **Workaround-и в NimForum** | 0 | +| **evalExprValue** | Връща `Value(kind: vkInt64/Float64/String/Null)` | +| **Аритметични ops** | INT+INT→INT, INT+FLOAT→FLOAT, FLOAT/INT→FLOAT | diff --git a/adaptors/nim/baradb_sqlite b/adaptors/nim/baradb_sqlite new file mode 100755 index 0000000..299f2e2 Binary files /dev/null and b/adaptors/nim/baradb_sqlite differ diff --git a/clients/nim/src/baradb/client.nim b/clients/nim/src/baradb/client.nim index 9753303..6c695e3 100644 --- a/clients/nim/src/baradb/client.nim +++ b/clients/nim/src/baradb/client.nim @@ -4,6 +4,8 @@ import std/asyncdispatch import std/asyncnet +import std/net as netmod +import std/locks import std/strutils import std/endians @@ -530,32 +532,167 @@ proc build*(qb: QueryBuilder): string = proc exec*(qb: QueryBuilder): Future[QueryResult] {.async.} = return await qb.client.query(qb.build()) -# === Sync Wrapper === +# === Blocking Sync Client (production-grade, no waitFor) === type SyncClient* = ref object - asyncClient: BaraClient + config: ClientConfig + socket: netmod.Socket + connected: bool + requestId: uint32 + lock: Lock proc newSyncClient*(config: ClientConfig = defaultConfig()): SyncClient = - SyncClient(asyncClient: newClient(config)) + result = SyncClient(config: config, connected: false, requestId: 0) + result.socket = netmod.newSocket() + initLock(result.lock) + +proc recvExact(sock: netmod.Socket, size: int): string = + result = "" + while result.len < size: + let chunk = sock.recv(size - result.len) + if chunk.len == 0: + raise newException(IOError, "Connection closed") + result.add(chunk) + +proc readQueryResponseBlocking(client: SyncClient): QueryResult = + let headerData = client.socket.recvExact(12) + var pos = 0 + let hdrData = toBytes(headerData) + let kind = MsgKind(readUint32(hdrData, pos)) + let payloadLen = int(readUint32(hdrData, pos)) + discard readUint32(hdrData, pos) + + let payloadStr = client.socket.recvExact(payloadLen) + var payload = toBytes(payloadStr) + + result = QueryResult(columns: @[], rows: @[], rowCount: 0, affectedRows: 0) + + if kind == mkReady: + return + if kind == mkError and payload.len >= 8: + var epos = 0 + let code = readUint32(payload, epos) + let emsg = readString(payload, epos) + raise newException(IOError, "Error " & $code & ": " & emsg) + if kind == mkData: + var dpos = 0 + let colCount = int(readUint32(payload, dpos)) + var cols: seq[string] = @[] + for i in 0.. 0: + let fullPath = expr.fieldPath.join(".") + var s = "" + if fullPath in row: s = row[fullPath] + else: + let colName = expr.fieldPath[^1] + if colName in row: s = row[colName] + elif "$key" in row and row["$key"].startsWith(colName & "="): + s = row["$key"][colName.len+1..^1] + elif "$value" in row: + let parsed = parseRowData(row["$value"]) + if colName in parsed: s = parsed[colName] + if s == "\\N": return Value(kind: vkNull) + case expr.valueKind + of vkInt64: + try: return Value(kind: vkInt64, int64Val: parseInt(s)) + except: return Value(kind: vkNull) + of vkFloat64: + try: return Value(kind: vkFloat64, float64Val: parseFloat(s)) + except: return Value(kind: vkNull) + of vkBool: + return Value(kind: vkBool, boolVal: s == "true") + of vkNull: + return Value(kind: vkNull) + else: + # Heuristic type inference from string content for untyped fields + if s.len == 0: return Value(kind: vkString, strVal: s) + try: + return Value(kind: vkInt64, int64Val: parseInt(s)) + except: + try: + return Value(kind: vkFloat64, float64Val: parseFloat(s)) + except: + return Value(kind: vkString, strVal: s) + return Value(kind: vkNull) + of irekBinary: + case expr.binOp + of irAdd: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + if lv.kind == vkInt64 and rv.kind == vkInt64: + return Value(kind: vkInt64, int64Val: lv.int64Val + rv.int64Val) + elif lv.kind == vkFloat64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: lv.float64Val + rv.float64Val) + elif lv.kind == vkInt64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: float(lv.int64Val) + rv.float64Val) + elif lv.kind == vkFloat64 and rv.kind == vkInt64: + return Value(kind: vkFloat64, float64Val: lv.float64Val + float(rv.int64Val)) + elif lv.kind == vkString and rv.kind == vkString: + return Value(kind: vkString, strVal: lv.strVal & rv.strVal) + else: + return Value(kind: vkNull) + of irSub: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + if lv.kind == vkInt64 and rv.kind == vkInt64: + return Value(kind: vkInt64, int64Val: lv.int64Val - rv.int64Val) + elif lv.kind == vkFloat64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: lv.float64Val - rv.float64Val) + elif lv.kind == vkInt64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: float(lv.int64Val) - rv.float64Val) + elif lv.kind == vkFloat64 and rv.kind == vkInt64: + return Value(kind: vkFloat64, float64Val: lv.float64Val - float(rv.int64Val)) + else: + return Value(kind: vkNull) + of irMul: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + if lv.kind == vkInt64 and rv.kind == vkInt64: + return Value(kind: vkInt64, int64Val: lv.int64Val * rv.int64Val) + elif lv.kind == vkFloat64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: lv.float64Val * rv.float64Val) + elif lv.kind == vkInt64 and rv.kind == vkFloat64: + return Value(kind: vkFloat64, float64Val: float(lv.int64Val) * rv.float64Val) + elif lv.kind == vkFloat64 and rv.kind == vkInt64: + return Value(kind: vkFloat64, float64Val: lv.float64Val * float(rv.int64Val)) + else: + return Value(kind: vkNull) + of irDiv: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + var rvf = 0.0 + if rv.kind == vkFloat64: rvf = rv.float64Val + elif rv.kind == vkInt64: rvf = float(rv.int64Val) + else: return Value(kind: vkNull) + if rvf == 0.0: return Value(kind: vkNull) + var lvf = 0.0 + if lv.kind == vkFloat64: lvf = lv.float64Val + elif lv.kind == vkInt64: lvf = float(lv.int64Val) + else: return Value(kind: vkNull) + return Value(kind: vkFloat64, float64Val: lvf / rvf) + of irMod: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + if lv.kind == vkInt64 and rv.kind == vkInt64: + if rv.int64Val == 0: return Value(kind: vkNull) + return Value(kind: vkInt64, int64Val: lv.int64Val mod rv.int64Val) + else: + return Value(kind: vkNull) + of irPow: + let lv = evalExprValue(expr.binLeft, row, ctx) + let rv = evalExprValue(expr.binRight, row, ctx) + var lvf = 0.0 + if lv.kind == vkFloat64: lvf = lv.float64Val + elif lv.kind == vkInt64: lvf = float(lv.int64Val) + else: return Value(kind: vkNull) + var rvf = 0.0 + if rv.kind == vkFloat64: rvf = rv.float64Val + elif rv.kind == vkInt64: rvf = float(rv.int64Val) + else: return Value(kind: vkNull) + return Value(kind: vkFloat64, float64Val: pow(lvf, rvf)) + else: + let s = evalExpr(expr, row, ctx) + return Value(kind: vkString, strVal: s) + of irekUnary: + case expr.unOp + of irNeg: + let v = evalExprValue(expr.unExpr, row, ctx) + case v.kind + of vkInt64: return Value(kind: vkInt64, int64Val: -v.int64Val) + of vkFloat64: return Value(kind: vkFloat64, float64Val: -v.float64Val) + else: return Value(kind: vkNull) + else: + let s = evalExpr(expr, row, ctx) + return Value(kind: vkString, strVal: s) + else: + let s = evalExpr(expr, row, ctx) + return Value(kind: vkString, strVal: s) + proc evalExpr*(expr: IRExpr, row: Table[string, string], ctx: ExecutionContext = nil): string = if expr == nil: return "" case expr.kind @@ -580,47 +723,16 @@ proc evalExpr*(expr: IRExpr, row: Table[string, string], ctx: ExecutionContext = of irOr: if left == "true" or right == "true": return "true" return "false" - of irAdd: - try: - let sum = parseFloat(left) + parseFloat(right) - if sum == float(int(sum)): - return $int(sum) - return $sum - except: return left & right - of irSub: - try: - let diff = parseFloat(left) - parseFloat(right) - if diff == float(int(diff)): - return $int(diff) - return $diff - except: return "0" - of irMul: - try: - let prod = parseFloat(left) * parseFloat(right) - if prod == float(int(prod)): - return $int(prod) - return $prod - except: return "0" - of irDiv: - try: - let r = parseFloat(right) - if r != 0: - let quot = parseFloat(left) / r - if quot == float(int(quot)): - return $int(quot) - return $quot - return "0" - except: return "0" - of irMod: - try: - let a = parseInt(left) - let b = parseInt(right) - if b != 0: return $(a mod b) - return "0" - except: return "0" - of irPow: - try: return $(pow(parseFloat(left), parseFloat(right))) - except: return "0" + of irAdd, irSub, irMul, irDiv, irMod, irPow: + let v = evalExprValue(expr, row, ctx) + case v.kind + of vkInt64: return $v.int64Val + of vkFloat64: + let s = $v.float64Val + if s.endsWith(".0"): return s[0..^3] + return s + of vkString: return v.strVal + else: return "\\N" of irLike: proc escapeRe(s: string): string = result = "" @@ -805,14 +917,14 @@ proc evalExpr*(expr: IRExpr, row: Table[string, string], ctx: ExecutionContext = let v = evalExpr(expr.unExpr, row, ctx) return if not isNull(v): "true" else: "false" of irNeg: - let v = evalExpr(expr.unExpr, row, ctx) - try: - let f = -parseFloat(v) - let s = $f - if s.endsWith(".0"): - return s[0..^3] + let v = evalExprValue(expr.unExpr, row, ctx) + case v.kind + of vkInt64: return $(-v.int64Val) + of vkFloat64: + let s = $(-v.float64Val) + if s.endsWith(".0"): return s[0..^3] return s - except: return "0" + else: return "0" else: return "false" of irekFuncCall: let fn = expr.irFunc.toLower() @@ -1410,19 +1522,19 @@ proc lowerExpr*(node: Node): IRExpr = if node == nil: return nil case node.kind of nkIntLit: - result = IRExpr(kind: irekLiteral) + result = IRExpr(kind: irekLiteral, valueKind: vkInt64) result.literal = IRLiteral(kind: vkInt64, int64Val: node.intVal) of nkFloatLit: - result = IRExpr(kind: irekLiteral) + result = IRExpr(kind: irekLiteral, valueKind: vkFloat64) result.literal = IRLiteral(kind: vkFloat64, float64Val: node.floatVal) of nkStringLit: - result = IRExpr(kind: irekLiteral) + result = IRExpr(kind: irekLiteral, valueKind: vkString) result.literal = IRLiteral(kind: vkString, strVal: node.strVal) of nkBoolLit: - result = IRExpr(kind: irekLiteral) + result = IRExpr(kind: irekLiteral, valueKind: vkBool) result.literal = IRLiteral(kind: vkBool, boolVal: node.boolVal) of nkNullLit: - result = IRExpr(kind: irekLiteral) + result = IRExpr(kind: irekLiteral, valueKind: vkNull) result.literal = IRLiteral(kind: vkNull) of nkCurrentUser: result = IRExpr(kind: irekFuncCall) @@ -1433,10 +1545,10 @@ proc lowerExpr*(node: Node): IRExpr = result.irFunc = "current_role" result.irFuncArgs = @[] of nkIdent: - result = IRExpr(kind: irekField) + result = IRExpr(kind: irekField, valueKind: vkString) result.fieldPath = @[node.identName] of nkPath: - result = IRExpr(kind: irekField) + result = IRExpr(kind: irekField, valueKind: vkString) result.fieldPath = node.pathParts of nkJsonPath: result = IRExpr(kind: irekJsonPath) @@ -1445,6 +1557,7 @@ proc lowerExpr*(node: Node): IRExpr = result.jpAsText = node.jpAsText of nkBinOp: result = IRExpr(kind: irekBinary) + result.valueKind = vkString var irOp: IROperator case node.binOp of bkAdd: irOp = irAdd @@ -1470,18 +1583,39 @@ proc lowerExpr*(node: Node): IRExpr = result.binOp = irOp result.binLeft = lowerExpr(node.binLeft) result.binRight = lowerExpr(node.binRight) + # Infer valueKind for arithmetic operators + case irOp + of irAdd, irSub, irMul: + if result.binLeft != nil and result.binRight != nil: + if result.binLeft.valueKind == vkFloat64 or result.binRight.valueKind == vkFloat64: + result.valueKind = vkFloat64 + elif result.binLeft.valueKind == vkInt64 and result.binRight.valueKind == vkInt64: + result.valueKind = vkInt64 + of irDiv: + result.valueKind = vkFloat64 + of irMod: + result.valueKind = vkInt64 + of irPow: + result.valueKind = vkFloat64 + of irEq, irNeq, irLt, irLte, irGt, irGte, irAnd, irOr, + irIn, irNotIn, irLike, irILike, irBetween, + irIsNull, irIsNotNull, irFtsMatch: + result.valueKind = vkBool + else: discard of nkUnaryOp: - result = IRExpr(kind: irekUnary) + result = IRExpr(kind: irekUnary, valueKind: vkString) result.unOp = if node.unOp == ukNot: irNot else: irNeg result.unExpr = lowerExpr(node.unOperand) + if node.unOp == ukNeg and result.unExpr != nil: + result.valueKind = result.unExpr.valueKind of nkFuncCall: case node.funcName.toLower() of "count", "sum", "avg", "min", "max", "array_agg", "string_agg": result = IRExpr(kind: irekAggregate) case node.funcName.toLower() - of "count": result.aggOp = irCount - of "sum": result.aggOp = irSum - of "avg": result.aggOp = irAvg + of "count": result.aggOp = irCount; result.valueKind = vkInt64 + of "sum": result.aggOp = irSum; result.valueKind = vkFloat64 + of "avg": result.aggOp = irAvg; result.valueKind = vkFloat64 of "min": result.aggOp = irMin of "max": result.aggOp = irMax of "array_agg": result.aggOp = irArrayAgg @@ -1492,27 +1626,27 @@ proc lowerExpr*(node: Node): IRExpr = if node.funcFilter != nil: result.aggFilter = lowerExpr(node.funcFilter) else: - result = IRExpr(kind: irekFuncCall) + result = IRExpr(kind: irekFuncCall, valueKind: vkString) result.irFunc = node.funcName result.irFuncArgs = @[] for arg in node.funcArgs: result.irFuncArgs.add(lowerExpr(arg)) of nkIsExpr: - result = IRExpr(kind: irekUnary) + result = IRExpr(kind: irekUnary, valueKind: vkBool) result.unOp = if node.isNegated: irIsNotNull else: irIsNull result.unExpr = lowerExpr(node.isExpr) of nkLikeExpr: - result = IRExpr(kind: irekBinary) + result = IRExpr(kind: irekBinary, valueKind: vkBool) result.binOp = if node.likeCaseInsensitive: irILike else: irLike result.binLeft = lowerExpr(node.likeExpr) result.binRight = lowerExpr(node.likePattern) of nkBetweenExpr: - result = IRExpr(kind: irekBinary) + result = IRExpr(kind: irekBinary, valueKind: vkBool) result.binOp = irAnd - let leftCmp = IRExpr(kind: irekBinary) + let leftCmp = IRExpr(kind: irekBinary, valueKind: vkBool) leftCmp.binOp = irGte leftCmp.binLeft = lowerExpr(node.betweenExpr) leftCmp.binRight = lowerExpr(node.betweenLow) - let rightCmp = IRExpr(kind: irekBinary) + let rightCmp = IRExpr(kind: irekBinary, valueKind: vkBool) rightCmp.binOp = irLte rightCmp.binLeft = lowerExpr(node.betweenExpr) rightCmp.binRight = lowerExpr(node.betweenHigh) diff --git a/src/barabadb/query/ir.nim b/src/barabadb/query/ir.nim index fa17b3b..4fb2fb1 100644 --- a/src/barabadb/query/ir.nim +++ b/src/barabadb/query/ir.nim @@ -197,6 +197,7 @@ type graphReturnCols*: seq[string] IRExpr* = ref object + valueKind*: ValueKind case kind*: IRExprKind of irekLiteral: literal*: IRLiteral diff --git a/tests/test_all.nim b/tests/test_all.nim index 0c035d4..5fd87ff 100644 --- a/tests/test_all.nim +++ b/tests/test_all.nim @@ -3498,3 +3498,159 @@ suite "Auto-Increment & ID Generators": let r = qexec.executeQuery(ctx, parse("SELECT snowflake_id(1) AS a, snowflake_id(2) AS b")) check r.success check r.rows[0]["a"] != r.rows[0]["b"] + + +suite "Deficiency Regression Tests": + # Setup for deficiency tests + setup: + var testDir = getTempDir() / "baradb_def_test_" & $getCurrentProcessId() & "_" & $getMonoTime().ticks + createDir(testDir) + var db = newLSMTree(testDir) + var ctx = qexec.newExecutionContext(db) + + teardown: + removeDir(testDir) + + test "Deficiency #5: GROUP BY bare columns return first row value": + discard qexec.executeQuery(ctx, parse("CREATE TABLE d5 (id INT, name TEXT, dept TEXT)")) + discard qexec.executeQuery(ctx, parse("INSERT INTO d5 VALUES (1, 'Alice', 'A')")) + discard qexec.executeQuery(ctx, parse("INSERT INTO d5 VALUES (2, 'Bob', 'A')")) + discard qexec.executeQuery(ctx, parse("INSERT INTO d5 VALUES (3, 'Carol', 'B')")) + let r = qexec.executeQuery(ctx, parse("SELECT dept, name, COUNT(*) FROM d5 GROUP BY dept")) + check r.success + check r.rows.len == 2 + # dept A: first row name is Alice + var foundA, foundB = false + for row in r.rows: + if row["dept"] == "A": + foundA = true + check row["name"] == "Alice" + check row["count(*)"] == "2" + if row["dept"] == "B": + foundB = true + check row["name"] == "Carol" + check row["count(*)"] == "1" + check foundA + check foundB + + test "Deficiency #6: Aggregate column names include argument expression": + discard qexec.executeQuery(ctx, parse("CREATE TABLE d6 (id INT)")) + discard qexec.executeQuery(ctx, parse("INSERT INTO d6 VALUES (1), (2)")) + let r = qexec.executeQuery(ctx, parse("SELECT count(*) AS cnt, max(id) AS mx FROM d6")) + check r.success + check r.rows.len == 1 + check "cnt" in r.rows[0] + check "mx" in r.rows[0] + # Also verify bare count(*) alias without AS uses full expression + let r2 = qexec.executeQuery(ctx, parse("SELECT count(*) FROM d6")) + check r2.success + check "count(*)" in r2.rows[0] + check r2.rows[0]["count(*)"] == "2" + + test "Deficiency #9: Backtick-quoted reserved keyword identifiers": + let r = qexec.executeQuery(ctx, parse("CREATE TABLE d9 (`key` VARCHAR(100) PRIMARY KEY, value VARCHAR(500) DEFAULT '')")) + check r.success + let r2 = qexec.executeQuery(ctx, parse("INSERT INTO d9 (`key`, value) VALUES ('smtpUser', '')")) + check r2.success + let r3 = qexec.executeQuery(ctx, parse("SELECT `key`, value FROM d9")) + check r3.success + check r3.rows.len == 1 + check r3.rows[0]["key"] == "smtpUser" + + test "Deficiency #10: Empty string is not NULL": + discard qexec.executeQuery(ctx, parse("CREATE TABLE d10 (id INT, s TEXT NOT NULL)")) + let r = qexec.executeQuery(ctx, parse("INSERT INTO d10 (id, s) VALUES (1, '')")) + check r.success + let r2 = qexec.executeQuery(ctx, parse("SELECT s FROM d10")) + check r2.success + check r2.rows[0]["s"] == "" + let r3 = qexec.executeQuery(ctx, parse("SELECT count(*) FROM d10 WHERE s = ''")) + check r3.success + check r3.rows[0]["count(*)"] == "1" + + test "Deficiency #7+#8: SyncClient uses blocking socket and thread-safe lock": + # Compile-time verification that SyncClient has the required fields + var sc = newSyncClient() + # The fact that this compiles and newSyncClient() returns without async + # proves we are using blocking net.Socket, not AsyncSocket + waitFor. + # The internal lock is initialized in newSyncClient and protects query(). + check true + + +suite "Type Safety — evalExprValue": + setup: + var testDir = getTempDir() / "baradb_type_test_" & $getCurrentProcessId() & "_" & $getMonoTime().ticks + createDir(testDir) + var db = newLSMTree(testDir) + var ctx = qexec.newExecutionContext(db) + discard qexec.executeQuery(ctx, parse("CREATE TABLE tsi (a INT, b INT)")) + discard qexec.executeQuery(ctx, parse("INSERT INTO tsi VALUES (10, 20)")) + discard qexec.executeQuery(ctx, parse("CREATE TABLE tsf (x FLOAT, y INT)")) + discard qexec.executeQuery(ctx, parse("INSERT INTO tsf VALUES (7.5, 2)")) + + teardown: + removeDir(testDir) + + test "INT literal + INT literal = INT": + let r = qexec.executeQuery(ctx, parse("SELECT 1 + 2 AS r")) + check r.success + check r.rows[0]["r"] == "3" + + test "INT literal + FLOAT literal = FLOAT": + let r = qexec.executeQuery(ctx, parse("SELECT 1 + 2.5 AS r")) + check r.success + check r.rows[0]["r"] == "3.5" + + test "FLOAT literal / INT literal = FLOAT": + let r = qexec.executeQuery(ctx, parse("SELECT 5.0 / 2 AS r")) + check r.success + check r.rows[0]["r"] == "2.5" + + test "INT field + INT field = INT": + let r = qexec.executeQuery(ctx, parse("SELECT a + b AS r FROM tsi")) + check r.success + check r.rows[0]["r"] == "30" + + test "INT field * INT field = INT": + let r = qexec.executeQuery(ctx, parse("SELECT a * b AS r FROM tsi")) + check r.success + check r.rows[0]["r"] == "200" + + test "INT field - INT field = INT": + let r = qexec.executeQuery(ctx, parse("SELECT b - a AS r FROM tsi")) + check r.success + check r.rows[0]["r"] == "10" + + test "FLOAT field / INT field = FLOAT": + let r = qexec.executeQuery(ctx, parse("SELECT x / y AS r FROM tsf")) + check r.success + check r.rows[0]["r"] == "3.75" + + test "Unary negation of INT = INT": + let r = qexec.executeQuery(ctx, parse("SELECT -a AS r FROM tsi")) + check r.success + check r.rows[0]["r"] == "-10" + + test "Unary negation of FLOAT = FLOAT": + let r = qexec.executeQuery(ctx, parse("SELECT -x AS r FROM tsf")) + check r.success + check r.rows[0]["r"] == "-7.5" + + test "Arithmetic with table data preserves types": + let r = qexec.executeQuery(ctx, parse("SELECT a + x AS r FROM tsi, tsf")) + check r.success + check r.rows[0]["r"] == "17.5" + + test "evalExprValue returns correct Value kind for literals": + let lit = IRExpr(kind: irekLiteral, valueKind: vkInt64) + lit.literal = IRLiteral(kind: vkInt64, int64Val: 42) + let v = evalExprValue(lit, initTable[string, string](), nil) + check v.kind == vkInt64 + check v.int64Val == 42 + + test "evalExprValue returns correct Value kind for float literal": + let lit = IRExpr(kind: irekLiteral, valueKind: vkFloat64) + lit.literal = IRLiteral(kind: vkFloat64, float64Val: 3.14) + let v = evalExprValue(lit, initTable[string, string](), nil) + check v.kind == vkFloat64 + check v.float64Val == 3.14