Add Cloudflare D1 database driver#980
Conversation
Agent-Logs-Url: https://github.com/ether/ueberDB/sessions/8a65052e-06ec-479d-b70b-ad1ec805e2d4 Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ether/ueberDB/sessions/8a65052e-06ec-479d-b70b-ad1ec805e2d4 Co-authored-by: JohnMcLear <220864+JohnMcLear@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Review Summary by QodoAdd Cloudflare D1 database driver with comprehensive test support
WalkthroughsDescription• Adds Cloudflare D1 database driver for ueberDB Workers support • Implements all required database operations using D1's prepared-statement API • Includes lightweight Node.js SQLite-backed mock for local testing • Passes full shared test suite across all cache/buffer combinations Diagramflowchart LR
A["ueberDB Core"] -->|"lazy load"| B["cloudflare_d1_db.ts"]
B -->|"uses"| C["D1Database API"]
D["test/lib/mock_d1.ts"] -->|"implements"| C
E["test/lib/databases.ts"] -->|"provides"| D
F["test/cloudflare_d1/test.cloudflared1.spec.ts"] -->|"runs"| E
File Changes1. databases/cloudflare_d1_db.ts
|
Code Review by Qodo
1. Bulk bypasses key limit
|
| async doBulk(bulk: BulkObject[]): Promise<void> { | ||
| if (bulk.length === 0) return; | ||
| const statements: D1PreparedStatement[] = []; | ||
| for (const op of bulk) { | ||
| if (op.type === 'set') { | ||
| statements.push( | ||
| this._d1db! | ||
| .prepare('INSERT OR REPLACE INTO store (key, value) VALUES (?, ?)') | ||
| .bind(op.key, op.value), | ||
| ); | ||
| } else if (op.type === 'remove') { | ||
| statements.push( | ||
| this._d1db!.prepare('DELETE FROM store WHERE key = ?').bind(op.key), | ||
| ); | ||
| } | ||
| } | ||
| await this._d1db!.batch(statements); | ||
| } |
There was a problem hiding this comment.
1. Bulk bypasses key limit 🐞 Bug ≡ Correctness
CloudflareD1DB.doBulk() writes keys without enforcing the 100-character limit, so oversized keys can be persisted when write buffering triggers bulk flushes. This creates inconsistent behavior vs immediate writes (set() throws) and vs other drivers that constrain keys at the schema level.
Agent Prompt
## Issue description
`CloudflareD1DB.set()` rejects keys longer than 100 chars, but `CloudflareD1DB.doBulk()` does not. Because the cache/buffer layer prefers `doBulk()` for multi-op flushes, long keys can be persisted successfully (and without throwing) whenever buffering is enabled.
## Issue Context
- In this driver, the `store` table schema does not enforce a 100-char limit (`key TEXT ...`).
- The cache/buffer layer uses `wrappedDB.doBulk(ops)` for multi-op flushes, bypassing `wrappedDB.set()`.
## Fix Focus Areas
- databases/cloudflare_d1_db.ts[72-135]
- lib/CacheAndBufferLayer.ts[489-555]
## What to change
1. Enforce the same key length validation in `doBulk()` for `set` operations (and consider validating `remove` too for consistency).
- If any key is >100 chars, throw an Error so the cache layer falls back to individual writes (which will then consistently fail via `set()`), rather than silently persisting invalid keys.
2. (Optional but recommended) Align the D1 schema with other drivers by adding a DB-level constraint, e.g. `CHECK(length(key) <= 100)` on the key column in the CREATE TABLE statement. (Note: `VARCHAR(100)` is not sufficient on SQLite for enforcement.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Adds a new
cloudflare_d1database driver so ueberDB can be used inside Cloudflare Workers backed by a D1 database binding.Driver (
databases/cloudflare_d1_db.ts)D1Databaseruntime binding viasettings.d1Database— no npm peer dependency needed, types are defined inline to mirror@cloudflare/workers-typesinit,get,set,findKeys,remove,doBulk,close) using D1's prepared-statement API andINSERT OR REPLACE/LIKE-pattern SQLdoBulkusesD1Database.batch()for atomic multi-operation writesInfrastructure
DatabaseTypeunion andinitDB()switch inindex.tsupdatedSettingstype gainsd1Database?: unknownfieldTests (
test/cloudflare_d1/)test/lib/mock_d1.ts: lightweightD1Databasemock backed by Node.js's built-innode:sqlite— no network, no miniflare requiredtest/lib/databases.ts:cloudflare_d1entry with a getter that produces a fresh in-memory instance per testtest_dbsuite (80 tests across all read-cache × write-buffer combinations)Usage in a Worker