From d932657e45b8145aafcb89135b72a3afe6f0cbbd Mon Sep 17 00:00:00 2001 From: Shannon Holland Date: Mon, 10 Aug 2026 16:53:38 -0700 Subject: [PATCH] fix(query): NULL-safe columnValue for cached-type default branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Database.query(_:) captures column types once from the first row and reuses them across the whole result set. A nullable TEXT column whose first row has a value and later row is NULL sends the NULL cell into columnValue's `default:` branch (as SQLITE_TEXT), which called `String(cString: UnsafePointer(sqlite3_column_text(stmt, index)))` — force-unwrapping a nil return, trapping with "Unexpectedly found nil while implicitly unwrapping an Optional value". Guard the pointer and return nil, matching the SQLITE_NULL case's semantics (row dictionary omits the key). Two regression tests added, one for the crashing shape and one documenting the pre-existing "NULL-first-row-loses-later-value" behavior so this fix doesn't regress it. Reported downstream at totalslacker/WebBrain#361 (Dashboard timeline crash) and totalslacker/SemanticHistory#173 (pin-bump for the fix). --- Sources/SQLiteVec/Database.swift | 14 +++++- Tests/SQLiteVecTests/DatabaseTests.swift | 62 ++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/Sources/SQLiteVec/Database.swift b/Sources/SQLiteVec/Database.swift index dec6e36..5a6b002 100644 --- a/Sources/SQLiteVec/Database.swift +++ b/Sources/SQLiteVec/Database.swift @@ -349,7 +349,19 @@ public actor Database { case SQLITE_NULL: return nil default: - return String(cString: UnsafePointer(sqlite3_column_text(stmt, index))) + // NULL-safe: `sqlite3_column_text` returns nil when the cell is + // genuinely NULL. This branch is reached (instead of the + // `SQLITE_NULL` case above) whenever `type` was captured from a + // *different* row's column value than the one being read — because + // `query(_:)` caches `columnInfo.types` from the first row and + // reuses them for the rest. On a nullable TEXT column where row 1 + // has a value and a later row is NULL, we land here with + // `type == SQLITE_TEXT` but a NULL cell. Force-unwrapping via + // `UnsafePointer(nil!)` would then trap. Return nil to match the + // `SQLITE_NULL` case's semantics — the caller sees the key absent + // from the row dictionary, which is what NULL means. + guard let ptr = sqlite3_column_text(stmt, index) else { return nil } + return String(cString: ptr) } } } diff --git a/Tests/SQLiteVecTests/DatabaseTests.swift b/Tests/SQLiteVecTests/DatabaseTests.swift index 6693ca1..64c95c3 100644 --- a/Tests/SQLiteVecTests/DatabaseTests.swift +++ b/Tests/SQLiteVecTests/DatabaseTests.swift @@ -231,4 +231,66 @@ final class DatabaseTests: XCTestCase { XCTAssertEqual(result[2]["distance"] as! Double, 0.2000000, accuracy: accuracy) XCTAssertEqual(result[2]["rowid"] as? Int, 2) } + + /// A nullable TEXT column where the *first row* has a value and a *later + /// row* is NULL used to crash `query(_:)` with an implicit-nil-unwrap trap. + /// Column types are captured once from the first row and reused for the + /// rest, so the NULL cell was dispatched into `columnValue`'s `default:` + /// branch (as SQLITE_TEXT) rather than `case SQLITE_NULL:`. The default + /// branch force-unwrapped `sqlite3_column_text`, which returns nil for a + /// genuinely-NULL cell — trap. + /// + /// The fix returns nil for that case, matching the `SQLITE_NULL` case's + /// semantics: the row dictionary simply omits the key. + func testNullableTextColumnAcrossRowsDoesNotCrash() async throws { + let db = try Database(.inMemory) + try await db.execute( + """ + CREATE TABLE items ( + id INTEGER PRIMARY KEY, + label TEXT + ) + """ + ) + try await db.execute("INSERT INTO items(id, label) VALUES (?, ?)", params: [1, "first"]) + try await db.execute("INSERT INTO items(id, label) VALUES (?, ?)", params: [2, NSNull()]) + + let result = try await db.query("SELECT id, label FROM items ORDER BY id") + + XCTAssertEqual(result.count, 2) + XCTAssertEqual(result[0]["id"] as? Int, 1) + XCTAssertEqual(result[0]["label"] as? String, "first") + XCTAssertEqual(result[1]["id"] as? Int, 2) + // NULL cell: the key is absent from the row dictionary — same semantics + // as `case SQLITE_NULL:` in `columnValue`. Not empty-string, not a + // present NSNull. + XCTAssertNil(result[1]["label"]) + } + + /// The reverse shape: first row is NULL, second row has a value. + /// This shape does not crash today (type captured as SQLITE_NULL sends the + /// value row through `case SQLITE_NULL:` returning nil, losing the value — + /// a distinct bug, but not the one this fix targets). Locked in so the + /// null-safety fix does not regress the existing behaviour. + func testNullFirstRowLosesValueInSecondRow() async throws { + let db = try Database(.inMemory) + try await db.execute( + """ + CREATE TABLE items ( + id INTEGER PRIMARY KEY, + label TEXT + ) + """ + ) + try await db.execute("INSERT INTO items(id, label) VALUES (?, ?)", params: [1, NSNull()]) + try await db.execute("INSERT INTO items(id, label) VALUES (?, ?)", params: [2, "second"]) + + let result = try await db.query("SELECT id, label FROM items ORDER BY id") + + XCTAssertEqual(result.count, 2) + XCTAssertNil(result[0]["label"]) + // Documents the pre-existing type-caching behavior: the second row's + // value is dropped because the first row's SQLITE_NULL type is reused. + XCTAssertNil(result[1]["label"]) + } }