Skip to content

feat(database): batch lookups by key with findById and findAllBy - #105

Merged
Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/sql-resolver
Nov 2, 2026
Merged

Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/sql-resolver

Conversation

@artile

@artile artile commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

When many fibers each read one row by its id, effect-orm sends one statement per fiber (the N+1 problem). This PR adds batched lookups over Effect's RequestResolver, so N concurrent lookups send one WHERE key IN (...) statement.

Closes #74.
Part of #52.

Changes

  • New Db.findById(query, { key }) and Db.findAllBy(query, { key }) in /database. Each one takes a builder query and a selected column of its FROM table. The batch statement is the query with key IN (...) added to its WHERE, run with Database.run.
  • Rows decode through the query's own row schema, as run decodes them.
  • findById fails a missing key with Cause.NoSuchElementError (as Effect's SqlResolver.findById does). findAllBy gives an empty array. A key asked for twice is sent once.
  • A query with param.* markers takes its params through .with(params), type-checked as run checks them. A direct call on such a query is a type error.
  • A batch never mixes: Database services, transactions (the Database's own open transaction, and the SqlClient transaction when a client is in context), or params. So a tenant filter in the query (a value or a param) keeps each statement to one tenant, and requireTenantScope checks each batch statement.
  • A query with limit or offset fails with QueryBuilderError and sends nothing, because the clause would apply to the whole batch.
  • TransactionOpen in database.ts is now exported inside the package (not from the entry), so the resolver can key batches by transaction.
  • Docs: new "Batching lookups" section in docs/database.md with a type-checked example, and rows in docs/reference.md. One minor changeset.

Left out

  • T.resolvers.findById on the table itself (the issue's sketch). A table has no dialect or Database, and a resolver from a query can select fewer columns and add filters (soft delete, tenant). It is a small follow-up if you want the table form.
  • Chunking of large IN lists. That is Bound-parameter limits: chunk IN lists and multi-row inserts #57 (bound-parameter limits). The resolver is a plain RequestResolver, so RequestResolver.batchN is not yet exposed; I can add it with Bound-parameter limits: chunk IN lists and multi-row inserts #57.
  • Insert and delete resolvers (SqlModel.makeResolvers has them). Reads are the N+1 case; writes can follow.
  • A ClickHouse live test. The resolver only builds a query and calls run, so it is dialect-neutral, but the tests run on PGlite only.
  • SQLite, MySQL and D1: not on main yet.

Validation

  • bun run build: pass.
  • bun run typecheck: pass (includes the new src/database/resolver.test-d.ts).
  • bunx vitest run --testTimeout=60000 --hookTimeout=60000 with the live ClickHouse 26.8.2.7: 61 files passed, 1040 tests passed, 0 skipped. New tests are in src/database/resolver.test.ts and count statements through observe: N concurrent lookups send 1 statement; two tenants send 2 single-tenant statements under requireTenantScope; a lookup in a transaction and one outside send 2; an unscoped query is refused with TenantScopeViolation.
  • node scripts/check-doc-citations.mjs, node scripts/check-exports-documented.mjs, node scripts/check-doc-examples.mjs: pass.
  • bun scripts/check-package.ts: pass.
  • Upstream CI for a fork PR waits for a maintainer to approve the run.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 371d257e-95bf-464a-9ee5-ed0f78d3440a
Commits

Reviewing files that changed from the base of the PR and between b57faf1 and 8704891.

Files selected for processing: 10
  • .changeset/batched-lookups.md
  • docs/database.md
  • docs/reference.md
  • src/database.ts
  • src/database/database.ts
  • src/database/resolver.sqlite.test.ts
  • src/database/resolver.test-d.ts
  • src/database/resolver.test.ts
  • src/database/resolver.ts
  • tests/clickhouse-http.clickhouse.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Makisuo and others added 2 commits November 2, 2026 16:28
…er limit

D1 binds at most 100 values per statement, so a batch of more keys failed
to compile. runAll now chunks the IN list to leave room for the query's
own params, runs the chunks in turn on the batch's connection, and has
D1/SQLite and live ClickHouse tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Makisuo

Makisuo commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Merge-readiness pass (8704891):

  • Merged current main (SQLite/D1 dialects, Effect 4.0.6, afterCommit, fuzz). It merged with no conflicts.
  • D1 bound-parameter limit (Bound-parameter limits: chunk IN lists and multi-row inserts #57): before this, a batch of more than about 100 keys failed to compile on D1. runAll now reads the dialect's params.maxParameters and compiles the base query to count its own bound params. It then splits the IN list into statements that each fit. These run one after another on the batch's connection or transaction. Each key is in exactly one chunk, so findAllBy keeps the query's ORDER BY for each key. ClickHouse inlines values, so it still sends one statement. Postgres splits only above 65535.
  • Tests: src/database/resolver.sqlite.test.ts covers 125 tenant-scoped lookups on a real Miniflare D1, sent as 2 statements of at most 100 bindings each under requireTenantScope, and the same batch on node:sqlite, sent as 1 statement. tests/clickhouse-http.clickhouse.test.ts has a live ClickHouse findById batch, including a missing key giving NoSuchElementError.
  • Docs (docs/database.md Batching lookups) and the changeset now describe the chunking.

Verified locally: bun run typecheck passes. bun run test against live ClickHouse 26.8 passes: 75 files, 1439 tests. FUZZ_SCALE=0.3 vitest run tests/fuzz passes: 9 files, 269 tests. CI run 41472641427 started without needing approval.

@Makisuo
Makisuo merged commit a56eb7d into MapleTechLabs:main Nov 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request batching with SqlResolver (N+1 safety)

2 participants