ReviewOS

stacks/bun-query-builder

fix(cache): isolate cached results by SQL, bindings and connection

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

Reproduction on current main

Reproduced against e8d66ca with Bun 1.4.1 and disposable SQLite, MySQL 8.4.5 and Postgres 16 databases. The same connection-isolation failure also affects the built-in SQLite wrapper.

The optional query result cache can return a different query's result, cross independent database connections, or retain rows from a rolled-back transaction:

  1. On a native Bun SQL connection, cache select('name').where('id', '=', 1), then select('id').where('id', '=', 1). The second result still contains only name.
  2. Create two independent connections with session-local tables of the same name but different rows. Cache the same SELECT on both. The second returns the first connection's row.
  3. Cache a read inside a transaction, then roll back. The same cached read outside can return the rolled-back value.
  4. Cache a read before a transaction. The transaction can receive the stale cached row instead of its own write.
  5. A valid bigint binding fails before execution because the cache key calls JSON.stringify.

All examples use synthetic rows and temporary tables. This is an opt-in .cache() defect, not evidence that uncached session queries return cached users.

Cause

client.ts keys a module-global result cache with String(finalQuery) and JSON-encoded bindings. Native Bun queries stringify as [object Promise]; the key omits connection identity, and transaction builders use the same cache.

Acceptance

  • Key cache entries by actual final SQL, connection identity and typed bindings.
  • Bypass result caching inside transactions/savepoints so reads remain transactionally correct and uncommitted rows never enter the shared cache.
  • Preserve ordinary same-connection cache hits, global capacity limits and explicit invalidation.
  • Add required real MySQL/Postgres CI probes plus native and wrapper SQLite coverage.
  • Keep uncached execution paths unchanged. Automatic invalidation of all ordinary writes is outside this fix; existing TTL semantics remain documented.

A root-cause fix and executable regression fixture are in progress. No migration or production database changes are involved.

9 days ago

Fixed in 5f271d5. CI passed: https://github.com/stacksjs/bun-query-builder/actions/runs/34691625029 . Local validation: 2185 tests pass, four existing skips, typecheck/build/lint pass. Required MySQL/Postgres fixtures plus native and wrapper SQLite coverage. Also verified through the actual Stacks query-builder wrapper with the candidate dependency. No npm release performed; installed 0.2.69 does not yet include this fix.

Sign in to comment on this issue.