ReviewOS

craft-native/craft

dbExecute always reports rowsAffected: 1, whatever happened

#159
Closed glennmichael123 opened this 9 days ago ·
9 days ago

CraftBridge.dbExecute resolves with a literal:

database?.execSQL(sql, args)

activity.runOnUiThread {
    webView.evaluateJavascript(
        "window._craftDbExecResolve && window._craftDbExecResolve({rowsAffected: 1})",
        null
    )
}

The 1 is not a count. SQLiteDatabase.execSQL returns void, so nothing in this method ever learns how many rows were touched:

  • DELETE FROM t WHERE 0 reports rowsAffected: 1;
  • UPDATE t SET x = 1 over ten thousand rows reports rowsAffected: 1;
  • CREATE TABLE, which affects no rows at all, reports rowsAffected: 1.

A page using the value to decide whether a delete matched anything gets the wrong answer every time, and the shape of the reply ({rowsAffected: n}) is what invites that use.

The fix

SQLiteStatement.executeUpdateDelete() returns the count for UPDATE/DELETE, and executeInsert() returns the row id for INSERT:

val statement = database!!.compileStatement(sql)
args.forEachIndexed { i, arg -> statement.bindString(i + 1, arg) }
val affected = statement.executeUpdateDelete()

That only works for statements that return a count, so a DDL statement still has no honest number and the field would have to be 0, or absent, or the reply shape would have to change.

Which is the real question, and it is a compatibility one rather than a technical one: pages already read rowsAffected and get 1. Changing it to a true count is a behaviour change for anything that checked === 1. Worth deciding deliberately rather than in passing, which is why this is an issue and not a patch.

Zig ports it as-is

bridge_android_db.zig sends the same constant, from a named exec_result with the reasoning attached, so the two bridges cannot drift apart while the question is open.

Related: #127, #148, #154, #157 — the family of replies that are sent but do not mean what they appear to.

9 days ago

Implemented and pushed in a457fda. Android database execution now uses SQLiteStatement.executeUpdateDelete() and returns its actual affected-row count, with native/Zig and generator regressions. Exact-commit CI, SBOM/vulnerability scan, Binary Size, and Benchmark are green.

Sign in to comment on this issue.