fix: destroy connection on transaction error instead of returning to pool#12
Merged
Merged
Conversation
…pool After a transaction error (e.g. query timeout), the connection may be in a dirty state. Previously it was always returned to the pool via bare release(), risking reuse of a broken connection. Now: - DBConnection.release() accepts an optional error/boolean to destroy the connection instead of returning it to the pool - PostgreSQL: delegates to pg PoolClient.release(err) - MySQL: calls connection.destroy() on error - Transaction catch block wraps ROLLBACK in try/catch to preserve the original error when ROLLBACK itself fails - Error path always destroys the connection via release(true) Co-authored-by: Cursor <cursoragent@cursor.com>
foo-ogawa
added a commit
that referenced
this pull request
Jul 15, 2026
…was Rosetta x86)
The prior benchmark run was x86_64 under Rosetta 2 (all adapter binaries were
Mach-O x86_64), which is not representative of the Apple Silicon host. Re-ran
the full cross-lang execution-surface matrix and the ORM comparison bench on the
NATIVE arm64 toolchain and regenerated both reports from the native-ARM numbers.
Toolchain (all arm64, verified): node v24.13.0 (arm64), go1.26.5 darwin/arm64,
rustc host aarch64-apple-darwin. Clean-rebuilt every rust adapter (lm_bench,
lm_codegen, v1rs_bench) + both go binaries (go_bench, lm_codegen) as arm64
(the old target/ carried x86_64 objects); regenerated the go/rust codegen
companions under the arm64 env. Correctness is unchanged (arch does not change
results — the fairness self-check, purity gate, and generate --check drift stay
green); only the perf numbers move.
Cross-lang (CROSS-LANG.md, native arm64, SQLite micro ÷sql):
- Rust codegen ≈ native-SQL PARITY holds (find 0.93×, complexWhere 0.92×,
hasMany 1.09×, tx 1.37×) — find/complexWhere now edge out the raw-SQL baseline.
- Go codegen fair on the materialized baseline (1.11–1.24× reads, tx 2.33×).
- 14/15 cells green. The ts/codegen cell FAILS ("unknown binding: created_at"):
a pre-existing, arch-independent regression from the #12 real-Select-node-IR
merge — bc's boxed `typescript-typed` emitter mis-lowers a WHERE-fragment
column-name marker (`whereGe($.created_at, $.since)`) into a runtime scope
ref, which throws (the go/rust `-typed-native` emitters correctly drop it via
lowerReadGraphForTypedNative). The committed report predated #12 and was stale.
Wired the TS codegen cell to the post-#12 Select-keyed handler contract
(codegen-cell.ts) so the cell surfaces the real bc error instead of
"Select has no handler"; the underlying bc/emitter fix is tracked separately.
ORM comparison (BENCHMARK.md, native arm64, PG:5433): litedbmodel #1 in 13/19
ops; single-key nested 12.18ms (fastest, beats Drizzle's LATERAL JOIN 19.34ms);
composite-key Drizzle 14.24ms vs litedbmodel 14.62ms; vs Prisma 1.4×–13.7×.
Regenerated the SVG + the embedoc table from the native-ARM CSV (drift green)
and refreshed the hand-written analysis/summary numbers to match.
Also untracks benchmark/crosslang/adapters/rust-codegen/target/ — 200 build
artifacts committed before the .gitignore entry existed; they are gitignored
build output (like the other adapters' target/) and were churning the diff on
every rebuild.
Env noted in both docs: measured on native arm64 (Apple Silicon), go arm64 /
node arm64 / rust aarch64-apple-darwin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
foo-ogawa
added a commit
that referenced
this pull request
Jul 15, 2026
…m the typescript-typed input (mirror go/rust -typed-native) #12 replaced the __makeSqlNode/__scope surrogate with the real Select-node IR. That IR's `where` port array carries a fragment for EVERY authored `whereX($.col,…)`, including one whose LHS `$.col` is a WHERE COLUMN-NAME MARKER, never a bound input value (`whereGe($.created_at, $.since)` accesses `$.created_at` only to name the column; only `$.since` is bound). go/rust's `-typed-native` path was already correct: it is fed `lowerReadGraphForTypedNative`, which rebuilds each node's ports from the GENUINE bound heads (`statementsById`), dropping the column-name markers. The TS `typescript-typed` codegen path was still fed the raw portable IR (`bundleToPortableIR`), so it emitted a stray `ref(["created_at"], scope)` read + a `created_at` input-struct field — and the bound module threw `unknown binding: created_at` at execution (the scope has no `created_at`). The ts/codegen cross-lang cell FAILED. Fix (litedbmodel-side, NOT a bc-emitter bug): feed the SAME genuine-bound-head lowering to the TS boxed endpoint too — `isTypedNativeEmitter` gating replaced by `needsHeadLowering` (true for every read emitter). bc#110's native array port means the array-typed heads (complexWhere/inList) lower cleanly for TS as well, so the old "lowering would narrow TS array coverage" concern is stale. Note: the `typescript-typed` codegen is a bench/conformance-only path (`generateCodegenArtifact` is called only from benchmark/, conformance/, test/) — NOT a production read path. Post-#12 production TS reads use the native walker (`executeBundle` → `executeReadGraph` over the real `readGraph.ir`), which this change does not touch. Severity is bench/conformance-only. Verified on native arm64 (Apple Silicon; go/node arm64, rust aarch64-apple-darwin, Docker PG:5433 / MySQL:3307): - cross-lang bench 15/15 cells (ts/codegen no longer fails); codegen<ir for ts on every covered read — find 2.62×<3.50×, complexWhere 2.94×<3.81×, hasMany 1.53×<6.97× (zero inversions); same no-subset matrix. - CROSS-LANG.md regenerated (native arm64). drift-check / selfcheck / purity green. - conformance 5×36/36 + codegen 12/12 (incl. the ts emitted-module exec leg), native vectors green; go/rust/py/php cells unregressed (bench variance only); full `npm run build` (tsc) + eslint + 1122 unit tests green. Test update: the emitted-.ts fingerprint assertion now fingerprints the LOWERED IR (the SAME input generateCodegenArtifact feeds the emitter), mirroring the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4 tasks
Owner
Author
Owner
Author
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DBConnection.release()にオプション引数error?: Error | booleanを追加。エラー発生時にコネクションをプールに返却せず破棄するPoolClient.release(err)に委譲connection.destroy()を呼び出しrelease(true)でコネクションを破棄Background
トランザクション内でクエリタイムアウト（
query_timeout）が発生した場合、コネクションが不確定な状態になる可能性がある。従来はrelease()で常にプールに返却していたため、次のクエリが壊れたコネクションに当たるリスクがあった。Test plan
Made with Cursor