Skip to content

ORM: every patch on an aggregateIndex/rankIndex table reads the pre-image twice #444

Closed

Description

@MikeyZhang75

Summary

On any table declaring an aggregateIndex or rankIndex, every db.patch reads the target document twice. The auto-injected write barrier occupies the update.before hook slot, which makes the pre-image re-read unconditionally necessary from the lifecycle's point of view — even though the barrier provably never writes the row.

Measured on current lifecycle.ts:

Table db.get calls for a 5-row orm.update()
declares aggregateIndex('by_org') 10
change trigger only (no before hook) 5
unhooked 0

The 2× is attributable solely to the barrier occupying the before slot.

Root cause

packages/kitcn/src/orm/lifecycle.ts:395-398. The re-read exists because a user before hook may write the row through innerDb, so the post-hook document cannot be trusted. The injected barrier cannot do that — it only probes aggregate_state.

needsDocuments && tableHooks.update?.before is therefore true on every aggregate-indexed table, whether or not the user wrote a hook.

Not fixed by the recent work

packages/kitcn/src/orm/lifecycle.ts is byte-identical between the audit commit 471ec753 and origin/main — git diff is empty and git log 471ec753..origin/main -- lifecycle.ts returns nothing.

Suggested fix

Capture existing.update?.before before wrapping, and gate the re-read on that rather than on the post-wrap hook. Tables with an aggregateIndex but no user-supplied update.before then fall back to oldDoc.

Alternatively tag the barrier-only wrapper with a symbol meaning "read-only, no re-read needed" and check for it.

~10 lines in lifecycle.ts.

Existing tests

None pin this. packages/kitcn/src/orm/aggregate-index/write-barrier.read-amplification.vitest.ts (added by #420) counts only aggregate_state by_table_status ranges — its own header says so. The pre-image db.get count is unmeasured.

Pin the fix with a read-bound vitest in that file's style: an N-row update on an aggregateIndex table must issue N pre-image reads, not 2N.

Environment

  • kitcn 0.32.1
  • convex 1.44.0
  • verified and measured against origin/main at 14bab503
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions