ReviewOS

also looking at this

craft-native/craft

feat(android): dbExecute and dbQuery in Zig — ratchet 84 → 82

#160
Merged glennmichael123 wants to merge feat/android-sqlite into main
7 files +541 -3
9 days ago

The local database pair, and the first natives to borrow a handle the Kotlin owns rather than build their own.

The connection stays Kotlin's

The shim caches one SQLiteDatabase in a field, which is what makes a BEGIN in one call and a COMMIT in the next reach the same connection. Zig could open its own — Context.openOrCreateDatabase hands out a fresh connection every call — and then a page that mixed a served call with a declined one would have two connections to one file, taking each other's locks for no reason a reader could see.

So the database arrives as an argument, the way SharedPreferences does for the secure store. The delegation sits after the open for the same reason:

if (database == null) {
    database = activity.openOrCreateDatabase("craft.db", Context.MODE_PRIVATE, null)
}

database?.let { if (CraftNative.dbExecute(it, sql, paramsJson)) return }

Which parameter shapes this serves

JSONArray.getString coerces, so the shim binds "1" for 1, "true" for true, and the four characters "null" for an explicit null — the same JSONObject.NULL.toString() quirk the calendar module documents. Worth stating plainly: craft.db.execute("… WHERE x = ?", [null]) binds the text null, not SQL NULL. That is the shim's behaviour, and it is reproduced rather than fixed.

A float is not served, because Double.toString picks the shortest form that round-trips; nor is a nested object or array, whose string form is org.json's own printer; nor is an integer past i64. Those make bindArgs return null before any JNI call has touched the database, and the shim binds them with the coercions it has.

A NULL column disappears from its row

row.put(col, cursor.getString(index)) with a null value removes the mapping rather than storing a JSON null, so a row with a NULL column comes back without that key. A page genuinely cannot tell a NULL column from one the query never selected.

The test covers the leading, middle, and every-column cases — a naive comma shows up in exactly the first of those, and only there.

rowsAffected is a literal

execSQL returns void, so the 1 the shim resolves with is not a count: a DELETE matching nothing reports the same 1 as an INSERT, and CREATE TABLE reports it too. Ported as a named exec_result constant with the reasoning attached rather than corrected — changing it is a behaviour change for any page that checked === 1. Filed as #159.

Testing

zig build test:android passes with the ratchet at 82. Both mutations fired before this was committed: making a NULL column emit null, and accepting a float parameter, each fail a named test. Both libcraft.so targets still build with no NDK.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.