Conversation
|
Updated 3:57 PM PT - Aug 15th, 2026
✅ @robobun, your commit 9f51b7d9ab438c4ff7926e197c502f6db23d8c15 passed in 🧪 To try this PR locally: bunx bun-pr 30848That installs a local version of the PR into your bun-30848 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a ChangesMySQL foundRows feature
Suggested reviewers:
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this changes the default affectedRows semantics for all existing Bun MySQL users (now matched-rows instead of changed-rows) — that's a product decision worth a human sign-off, and there's also a CI build failure on x64-baseline.
Extended reasoning...
Overview
This PR adds a foundRows connection option to Bun.SQL's MySQL adapter that controls the CLIENT_FOUND_ROWS capability flag during handshake, threaded from the public TypeScript API (sql.d.ts, shared.ts, mysql.ts, private.d.ts) through the Rust FFI layer (JSMySQLConnection.rs) into the protocol handshake (MySQLConnection.rs). It ships with a self-contained mock-server test (sql-mysql-found-rows.test.ts). It also bundles two unrelated changes: a JSON.stringify wrap for the OVERLAY_CSS define in bake-codegen.ts (cold-build fix), and ~10 Rust files of pure cargo fmt whitespace from the autofix.ci commit.
Security risks
None identified. The new option is a boolean that toggles a single capability bit; URL query parsing is case-insensitive string comparison with no injection surface; the null-byte / protocol-injection guards on connection strings are unchanged.
Level of scrutiny
Medium-high. The mechanical plumbing is straightforward and the capability-bit change in handle_handshake is minimal and correctly placed before intersect(). However, the key judgment call is that foundRows defaults to true, which silently flips affectedRows semantics for every existing Bun MySQL user from changed-rows to matched-rows. The PR argues this matches mysql2/mariadb defaults (true), but it's still a behavioral break for anyone already on Bun's MySQL adapter — that's a product/compat decision a maintainer should explicitly endorse rather than have auto-approved.
Other factors
- robobun reports a build failure on
x64-baseline - build-rustfor the latest commit, which should be resolved or explained before merge. - The PR mixes three concerns (feature + unrelated build fix + autofix formatting); fine if that's the project norm, but worth a human glance.
- Test coverage for the new option is solid (6 cases including URL vs. options precedence and OK_Packet round-trip).
fe16787 to
69a7af4
Compare
6a6c689 to
345720f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bun-types/sql.d.ts`:
- Around line 383-387: Update the JSDoc for the foundRows option (the comment
block around foundRows in packages/bun-types/sql.d.ts) to remove "MySQL only"
and explicitly state it applies to both MySQL and MariaDB protocols/adapters;
mention that when enabled the driver reports rows matched by the WHERE clause in
affectedRows and that this matches the default behavior of the mysql2 and
mariadb drivers so adapter: "mariadb" is supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce0252cd-d4b6-4f23-a5b5-282ac2d665c5
📒 Files selected for processing (18)
packages/bun-types/sql.d.tssrc/codegen/bake-codegen.tssrc/crash_handler/lib.rssrc/errno/lib.rssrc/js/internal/sql/mysql.tssrc/js/internal/sql/shared.tssrc/js/private.d.tssrc/perf/tracy.rssrc/runtime/cli/Arguments.rssrc/runtime/cli/run_command.rssrc/runtime/cli/upgrade_command.rssrc/runtime/jsc_hooks.rssrc/runtime/webview/ChromeProcess.rssrc/spawn/process.rssrc/spawn_sys/spawn_process.rssrc/sql_jsc/mysql/JSMySQLConnection.rssrc/sql_jsc/mysql/MySQLConnection.rstest/js/sql/sql-mysql-found-rows.test.ts
There was a problem hiding this comment.
Thanks for addressing all the nits — the code looks good to me. I'm not auto-approving only because this flips the default affectedRows semantics for existing MySQL users (changed-rows → matched-rows), which is a product call worth a maintainer's explicit sign-off; the musl build failures in CI are also worth a glance before merge.
Extended reasoning...
Overview
This PR adds a foundRows connection option to Bun.SQL's MySQL/MariaDB adapter, threading it from parseOptions() (TS) → createMySQLConnection (TS binding) → JSMySQLConnection::create_instance (Rust FFI) → MySQLConnection.found_rows field → handle_handshake's CLIENT_FOUND_ROWS capability bit. It defaults to true, matching the mysql2/mariadb driver defaults. The PR also bundles an unrelated cold-build fix in bake-codegen.ts (wrap OVERLAY_CSS in JSON.stringify) and ~10 Rust files of purely cosmetic #[cfg(...)] attribute reformatting from autofix.ci. A new mock-MySQL-server test file (7 cases) and a docs section in runtime/sql.mdx round it out.
All three of my earlier inline nits (duplicate "max" in the type union, String() coercion for duplicate URL query keys, and the missing sql.mdx docs section) have been addressed and resolved.
Security risks
None identified. The option is a boolean that toggles a single well-defined MySQL capability flag during the handshake; no user-controlled data flows into protocol bytes beyond that. URL query parsing coerces through String() before .toLowerCase(), avoiding the array-method TypeError on duplicate keys.
Level of scrutiny
Moderate-to-high. The implementation is small, well-isolated, and well-tested, but two things push this above the auto-approve bar:
- Default behavior change. Setting
foundRows: trueby default flipsaffectedRowssemantics for every existing Bun MySQL user from changed-rows to matched-rows. Code that relied onaffectedRows === 0to mean "no column values changed" will silently behave differently. The PR description and docs are clear about this and the rationale (ecosystem alignment) is sound, but choosing to ship a default flip vs. an opt-in is a product decision a maintainer should explicitly sign off on. - Wire-protocol path. The Rust change in
MySQLConnection::handle_handshakerestructures capability negotiation slightly (compute → set bit → intersect). It's correct on inspection, but handshake code is load-bearing.
Other factors
- CI (Build #54959) shows build failures on three musl targets (x64-musl, x64-musl-baseline, aarch64-musl). These may be infrastructure-related and unrelated to this change, but should be confirmed green before merge.
- The bundled
bake-codegen.tsfix is independent of the MySQL change (acknowledged in the PR description). - Test coverage for the new option is thorough (default, explicit true/false, URL form, options-over-URL precedence, duplicate-key URL, and
affectedRowsround-trip via mock server).
2655518 to
5e65cda
Compare
There was a problem hiding this comment.
All prior nits are addressed and I didn't find any bugs, but this changes the default affectedRows semantics for existing MySQL users and threads a new arg through the Rust handshake path — worth a human sign-off on the default flip.
Extended reasoning...
Overview
Adds a foundRows connection option (default true) for the MySQL/MariaDB adapter that toggles CLIENT_FOUND_ROWS in the HandshakeResponse41 capability flags. Threaded through parseOptions (shared.ts) → createMySQLConnection (mysql.ts, positional arg 16) → JSMySQLConnection::create_instance (Rust FFI) → MySQLConnection::init → handle_handshake. Also adds public types, docs in runtime/sql.mdx, a 300-line mock-server test, and an unrelated build fix in bake-codegen.ts.
Security risks
None identified. The option only flips a single capability bit during handshake; no auth, crypto, or input-validation surface changes. The URL query-string parser coerces via String() before .toLowerCase() (fixed after my earlier nit), so no new throw paths on adversarial URLs.
Level of scrutiny
Medium-high. While the implementation is clean, well-tested, and all prior review feedback (mine and CodeRabbit's) has been addressed, this is not a purely additive change — it flips the default affectedRows semantics for every existing Bun MySQL user from changed-rows to matched-rows. That's the right call for mysql2/mariadb compatibility, but it's a behavioral default change to a public API that a maintainer should explicitly approve. It also touches the Rust protocol handshake (MySQLConnection.rs:642-655) and adds a positional FFI argument (callframe.argument(16)), which sits adjacent to the recently-rebased allowPublicKeyRetrieval plumbing.
Other factors
- All four prior inline comments (3 from me, 1 from CodeRabbit) are resolved with follow-up commits.
- Bundles an unrelated
bake-codegen.tsbuild fix (JSON.stringifywrap forOVERLAY_CSS) — reasonable but worth a glance. - Test coverage is solid (mock server inspects the actual capability bit on the wire, plus precedence and duplicate-key cases).
5e65cda to
e97f7e9
Compare
There was a problem hiding this comment.
No bugs found and all prior nits are addressed — deferring because this changes the default affectedRows semantics for all existing MySQL users and threads a new param through the JS→Rust FFI handshake path, which is worth a human sign-off.
Extended reasoning...
Overview
This PR adds a foundRows connection option to Bun.SQL's MySQL adapter, mapping to the CLIENT_FOUND_ROWS capability flag. It touches 9 files across the stack: public types (sql.d.ts), prose docs (sql.mdx), JS option parsing (shared.ts, mysql.ts, private.d.ts), Rust FFI/protocol (JSMySQLConnection.rs, MySQLConnection.rs), a 300-line mock-MySQL-server test, and an unrelated build fix in bake-codegen.ts.
Security risks
None identified. The change adds a boolean that toggles a single capability bit during the MySQL handshake; no auth, crypto, or input-validation surface is affected. The URL-query-param parsing is defensive (coerces through String() after my earlier nit was addressed).
Level of scrutiny
Moderate-to-high. Two factors push this above auto-approve threshold:
- Default behavior change:
foundRowsdefaults totrue, which flipsaffectedRowssemantics for every existing Bun MySQL user from changed-rows to matched-rows. The rationale (matching mysql2/mariadb defaults) is sound and well-documented, but it's still a product decision that could silently affect existing code that branches onaffectedRows === 0. - Native FFI threading: A new positional argument is added at index 16 of
createMySQLConnectionand plumbed throughJSMySQLConnection::create_instance→MySQLConnection::init→handle_handshake. The rebase notes describe resolving a conflict withallowPublicKeyRetrievalat index 15; positional FFI arg ordering is exactly the kind of thing a human should eyeball.
Other factors
- All three of my earlier inline nits (duplicate
"max"in the type union,String()coercion for duplicate URL keys, missingsql.mdxdocs) and CodeRabbit's MariaDB-wording nit have been addressed and resolved. - Test coverage is solid: a Docker-free mock MySQL server asserts the capability bit in all six option/URL/precedence permutations plus a regression test for the duplicate-key case.
- An unrelated
bake-codegen.tsbuild fix is bundled in; it's small and well-commented but adds to the review surface. - No human reviewer has weighed in yet; CodeRabbit suggested alii.
mysql2 and mariadb both enable CLIENT_FOUND_ROWS by default, so an UPDATE that matches a row but does not change any column value returns affectedRows: 1. Bun.SQL's MySQL adapter had the flag defined in Capabilities but never set it and had no way to opt in, so the same query returned affectedRows: 0 — indistinguishable from an UPDATE that matched nothing. Code migrated from mysql2 / mariadb hit this. Add a foundRows connection option (default true, matching mysql2 and mariadb) that OR-s CLIENT_FOUND_ROWS into the client-side capability set built in handshake negotiation. Accepted as options.foundRows and as the URL query param ?foundRows=false; options object wins over URL. For non-MySQL adapters the option is parsed and ignored. Fixes #30843
Bun.build parses define: values as JSON. The other defines
(side, IS_ERROR_RUNTIME, IS_BUN_DEVELOPMENT) already go through
JSON.stringify / String, but OVERLAY_CSS was passing the raw minified
CSS as-is. That CSS starts with '*{...}'; bun versions that lack the
auto-quote fallback from #30679 reject it with 'Operators are not
allowed in JSON' and abort bake-codegen. The CI Dockerfile bootstraps
from bun 1.3.13, which predates the fix, so cold builds fail. Wrap
the value in JSON.stringify to match the other defines.
node-http-backpressure.test.ts allocates a 2GB buffer and was SIGKILL'd on one aarch64 shard. Unrelated to the MySQL foundRows diff; every other test-bun shard on every other platform passed.
…asing Bun's URLSearchParams.toJSON() returns an Array when a key appears more than once in the query string, so `?foundRows=true&foundRows=false` gave us an array and `.toLowerCase()` threw `TypeError`. Coerce via String() first to match the sslmode branch's handling — duplicate keys now fall through to the default instead of crashing the connection path. Covered by a new test case in sql-mysql-found-rows.test.ts. Spotted by @claude on review.
…ction JSDoc had "MySQL only" which could imply `adapter: "mariadb"` is unsupported. Both adapters route through MySQLAdapter → createMySQLConnection → the same MySQL wire protocol, so the option works for either. Also add a short block to docs/runtime/sql.mdx explaining the new default (matched-rows) and how to opt out via `foundRows: false` or `?foundRows=false` on the URL.
e97f7e9 to
717469d
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/sql_jsc/mysql/JSMySQLConnection.rs:503-512— 🟡 Nit: the PR adds multi-line narrative comments at 8 sites that the repo'scomment-copbot is already flagging (unresolved inline comments dated 2026-08-15 against HEAD). REVIEW.md's enforced-style section: "Only comment what the code cannot say. One line. Never restate what the code does. Never narrate the change." The 'matches mysql2/mariadb defaults' rationale is repeated 4+ times across the diff — it's already stated once in the JSDoc (sql.d.ts:413-428) and once in docs/runtime/sql.mdx, so each in-code site can drop to one line or nothing.Extended reasoning...
What this is
The PR adds paragraph-length comments at 8 locations that either restate what the code does, narrate the change, or repeat the same rationale verbatim across multiple sites. The repo's REVIEW.md explicitly enforces a comment style under "Code style & idioms reviewers enforce":
Only comment what the code cannot say. One line. Never restate what the code does. Never narrate the change. Prefer links to GitHub issues.
CLAUDE.md #13 reinforces: "If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code." And #14: "After every code comment you write, ask yourself, 'Is this information the next Claude would spend multiple tool calls trying to understand?'. If the answer isn't clearly yes, the code comment is noise — delete it."
The repo automates this check: the
comment-copgithub-actions bot has left 8 unresolved inline comments on this PR (dated 2026-08-15, i.e. against current HEAD 717469d), each quoting the CLAUDE.md #13 rule verbatim, at exactly the locations below.Affected sites
Location Lines What it comments src/codegen/bake-codegen.ts59–66 7-line comment justifying JSON.stringifyon the OVERLAY_CSS definesrc/js/internal/sql/shared.ts851–852 2-line doc comment (extended to name foundRows)src/js/internal/sql/shared.ts1802–1805 3-line comment on let foundRows: boolean = truesrc/js/internal/sql/shared.ts1839–1843 5-line comment inside the foundrowsURL branchsrc/js/internal/sql/shared.ts2019–2021 3-line comment before the options-override block src/sql_jsc/mysql/JSMySQLConnection.rs503–511 9-line comment for a 1-line let found_rows = callframe.argument(16).to_boolean()src/sql_jsc/mysql/MySQLConnection.rs94–96 3-line doc on the found_rowsfieldsrc/sql_jsc/mysql/MySQLConnection.rs678–682 5-line comment before requested.CLIENT_FOUND_ROWS = self.found_rowsWhy these violate the rule
Repetition: The phrase "matches the mysql2 / mariadb defaults" (or a close variant) appears at shared.ts:1802, JSMySQLConnection.rs:505, MySQLConnection.rs:96, and MySQLConnection.rs:680 — four in-code sites — plus the JSDoc at
packages/bun-types/sql.d.ts:413-428anddocs/runtime/sql.mdx. The JSDoc and docs page are the canonical, discoverable places for this rationale; repeating it at every plumbing hop is exactly the "narrate the change" pattern the rule forbids.Restating the code: The 5-line comment at MySQLConnection.rs:678–682 explains that
requested.CLIENT_FOUND_ROWS = self.found_rowssets CLIENT_FOUND_ROWS from the foundRows option — the code says that. The 9-line comment at JSMySQLConnection.rs:503–511 spends three lines explaining thatcallframe.argument(16)returns UNDEFINED when omitted andto_booleancoerces UNDEFINED tofalse— that's howto_booleanworks everywhere in the file (seeargument(15)two lines up, which has no such comment).Paragraph-length workaround justification: The bake-codegen.ts comment (7 lines) is precisely the shape CLAUDE.md #13 targets: a paragraph explaining why a workaround is OK. The information that matters ("pre-#30679 bootstrap bun rejects unquoted CSS as a define value") fits in one line with an issue link.
Step-by-step: JSMySQLConnection.rs:503-511
- The line being commented is
let found_rows = callframe.argument(16).to_boolean();— a single-token assignment identical in shape tolet allow_public_key_retrieval = callframe.argument(15).to_boolean();two lines above (which has no comment). - The 9-line comment explains: (a) what
foundRows: truedoes semantically, (b) that it matches mysql2/mariadb, (c) how it's applied at handshake, (d) whatargument(16)returns for a missing arg, (e) whatto_booleandoes with UNDEFINED, (f) that mysql.ts always passes an explicit boolean. - Points (a)–(c) are already in the JSDoc, docs page, and MySQLConnection.rs field/handshake comments. Points (d)–(e) describe standard
CallFrame/JSValuesemantics used throughout the file. Point (f) is change narration. - Per REVIEW.md, none of this is "what the code cannot say" — the code plus the JSDoc is self-explanatory.
Impact
None at runtime — this is purely comment hygiene. Filed as nit: it doesn't block on correctness, but (1) REVIEW.md explicitly lists this in its enforced-style section, and (2) the repo's own
comment-copbot has 8 unresolved flags on the PR, so a human reviewer will almost certainly request it.Fix
Trim each site to at most one line (or delete entirely — the JSDoc + docs page carry the rationale). Suggested replacements:
- bake-codegen.ts:59-66 →
// Quote so pre-#30679 bootstrap bun (1.3.13, CI Dockerfile) accepts the raw CSS as a define value. - shared.ts:1802-1805 → delete (the JSDoc covers it), or
// mysql2/mariadb default; ignored by non-MySQL adapters. - shared.ts:1839-1843 →
// "false"/"0" disables; coerce first — toJSON() returns an array for duplicate keys. - shared.ts:2019-2021 →
// options object wins over URL query string(or delete — this is the file's convention for every option) - JSMySQLConnection.rs:503-511 → delete (matches the uncommented
argument(15)line above) - MySQLConnection.rs:94-96 →
/// Request CLIENT_FOUND_ROWS during handshake (matched-rows semantics). - MySQLConnection.rs:678-682 → delete (the field doc + capability-negotiation comment 10 lines up cover it)
🔬 also observed by github-actions
- The line being commented is
- extend wire-frames.ts with MYSQL_CLIENT_FOUND_ROWS, an affectedRows param on mysqlOkPacket, and a HandshakeResponse41 client_flag parser; import them instead of hand-rolling the byte layout - read options.foundRows into a local once (bun/no-duplicate-conditional-property-access) - drop restating comments
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs — all earlier nits have been addressed. Because it changes the default affectedRows semantics for existing MySQL users (changed-rows → matched-rows), adds new public API surface (foundRows option + URL param), and bundles an unrelated bake-codegen.ts build fix, a human sign-off on the default/API decision would still be worthwhile.
What was reviewed:
- Option threading through
parseOptions→createPooledConnectionHandle→JSMySQLConnection(arg 16) →MySQLConnection.found_rows→handle_handshake'srequested.CLIENT_FOUND_ROWSbefore the capability intersect. mysqlOkPacket's newaffectedRowsparam — default (0) is byte-identical to the prior hardcoded payload, so existing wire-frames callers are unaffected.- URL
?foundRows=parsing coerces via template literal (handles the duplicate-key array case) and options-object precedence over URL — both covered by tests. - Postgres path ignores the extra trailing
!!foundRowsarg tonativeCreateConnection, consistent with howallowPublicKeyRetrievalalready works.
Extended reasoning...
Overview
This PR adds a foundRows boolean connection option to Bun.SQL for MySQL/MariaDB that toggles the CLIENT_FOUND_ROWS capability flag during the protocol handshake. When enabled (the new default), affectedRows reports rows matched by the WHERE clause rather than rows changed — matching the mysql2 and mariadb npm driver defaults. It touches 10 files: option parsing in shared.ts, the MySQL native-call type in mysql.ts, the Rust JSC binding (JSMySQLConnection.rs reads argument(16)), the connection struct and handshake in MySQLConnection.rs, public types in sql.d.ts, internal types in private.d.ts, prose docs in sql.mdx, a new 7-case test file using the shared wire-frames.ts mock helpers, and small additions to wire-frames.ts itself. It also carries an unrelated one-line build fix in bake-codegen.ts (wrapping OVERLAY_CSS in JSON.stringify for bootstrap-bun compatibility).
Security risks
None identified. The option is a boolean that controls one bit in the client's capability-negotiation word; it's coerced with !! before crossing to native, and to_boolean() on the Rust side. The URL query-param path coerces via template-literal string interpolation before comparison (so no method calls on attacker-controlled non-string values). No new parsing of untrusted network bytes; the only wire-protocol change is which capability bits the client requests, which the server intersects anyway.
Level of scrutiny
Medium-high. The implementation itself is mechanically simple — one bool threaded through five layers, one assignment before a capability intersect — and is well-covered by a mock-server test that inspects the actual HandshakeResponse41 bytes. However, two aspects push this above the auto-approve bar:
- Default behavior change. Existing Bun MySQL users will see
affectedRowsflip from changed-rows to matched-rows semantics on upgrade. This aligns with the ecosystem (mysql2/mariadb) and is well-documented in this PR, but it's a product decision that a maintainer should consciously accept, not one an automated review should ratify. - New public API surface. The
foundRowsoption name, itstruedefault, and the?foundRows=URL query-param spelling are all user-facing API commitments. Per the repo's own.claude/docs/landing-prs.mdAPI-design guidance, new API surface warrants maintainer agreement.
Other factors
- All five prior review rounds (duplicate
"max"in the type union,.toLowerCase()on duplicate-key arrays, MariaDB doc wording, missingsql.mdxdocs, wire-frames helper reuse,node:netprefix, comment-cop paragraph trims) have been addressed and every thread is resolved. - Test coverage is solid: 7 cases including default, explicit true/false, URL param, duplicate URL keys, options-over-URL precedence, and an end-to-end
affectedRowsround-trip. The PR description shows fail-before withUSE_SYSTEM_BUN=1(4 fail) and pass-after (7 pass). - The
wire-frames.tschange tomysqlOkPacketkeeps the default payload byte-identical (mysqlLenencInt(0)→[0x00]), so 30+ existing callers are unaffected. - The bundled
bake-codegen.tsbuild fix is orthogonal to the MySQL change; a maintainer may prefer it split out, but it's a one-line low-risk codegen-script tweak.
This branch has not been deployed
1 participant
Fixes #30843.
Repro
Cause
Capabilities::get_default_capabilitiesinsrc/sql/mysql/Capabilities.rsnever set
CLIENT_FOUND_ROWS, and there was no user-facing option to optin. The MySQL server's
OK_Packet.affected_rowstherefore carriedchanged-rows counts, not matched-rows counts.
mysql2enablesCLIENT_FOUND_ROWSby default (opt-out viaflags: ["-FOUND_ROWS"]) andmariadbdoes the same (opt-out viafoundRows: false). Code migrated from either driver silently lost theability to tell "matched but unchanged" from "didn't match" apart.
Fix
Add a
foundRowsconnection option (accepted on the options object andas a
?foundRows=...URL query param). Default istrue, matching themysql2 / mariadb defaults. When enabled, the client OR-s
CLIENT_FOUND_ROWSinto the capability set it requests during theHandshakeResponse41, so subsequent
affected_rowscounts come back withmatched-rows semantics.
The option is threaded through:
parseOptions→createMySQLConnectioncall →JSMySQLConnection::create_instance→MySQLConnection::init→MySQLConnection.found_rowsfield →handle_handshake'srequested.CLIENT_FOUND_ROWS.parseOptionsalso writesfoundRowsinto theDefinedOptionsstructure for Postgres/SQLite adapters — they ignore it, so the option
is MySQL-only without any adapter switch in the parser.
Verification
New test:
test/js/sql/sql-mysql-found-rows.test.tsuses a minimal mockMySQL server (no Docker required) to inspect the client's
HandshakeResponse41
capability_flagsbit 1 (CLIENT_FOUND_ROWS):foundRows)foundRows: truefoundRows: false?foundRows=falsefoundRowskeysfoundRows: true+ URL?foundRows=falseaffectedRows: 1reaches JSFail-before:
Pass-after:
Existing MySQL tests (
sql-mysql-auth-short-nonce,28004,adapter-env-var-precedence,adapter-override,sql-mysql-cached-error,sql-mysql-bind-oob,sql-mysql-bind-blob-borrow,sql-mysql-raw-length-prefix,sql-mysql-columns-realloc-oom,sql-mysql-clean-reentry) still pass.Extra commit: build fix
build: JSON-stringify OVERLAY_CSS define value in bake-codegen— wrapsthe raw minified CSS in
JSON.stringifyso the pre-auto-quote bootstrapbun (1.3.13, used in the CI Dockerfile) can parse
define:values thatstart with
*{...}. Independent of the MySQL change; needed for coldbuilds.
Rebase notes
Rebased twice as main moved:
Onto
0b20408b65(Hardening: input validation and bounds tightening across 26 subsystems #31129), which addedallowPublicKeyRetrievalto thesame MySQL connection plumbing. Resolved by keeping both parameters
side-by-side: the native
createConnectionarg list ends with...!prepare, !!allowPublicKeyRetrieval, !!foundRows, andJSMySQLConnection.rsreadsargument(15)/argument(16).Onto
ac312f0952, which refactoredPooledMySQLConnectioninto a sharedBasePooledConnectionbase class: the per-driver option destructuring andnative
createConnectioncall moved frommysql.tsintocreatePooledConnectionHandleinshared.ts. Resolved by taking main'sstructure and moving the
foundRowsthreading there (destructurefoundRows = true, pass!!foundRowsafter!!allowPublicKeyRetrieval),following the same trailing-arg pattern
allowPublicKeyRetrievalalreadyuses (extra trailing args are ignored by the Postgres driver).
Onto
8f8695f4cd. Two conflicts:shared.ts: main expanded the URL query parsing (renamed the loop var andadded
ssl-mode/ssl_mode/ssl/tlsbranches). Kept main's structure andmoved the
foundrowsbranch into it, using the same template-literal stringcoercion style as the neighboring branches (still covers the
duplicate-key-array case).
MySQLConnection.rs: main added MariaDB capability negotiation(
self.mariadb_capabilities = ...) in the same handshake block. Kept both:requested.CLIENT_FOUND_ROWS = self.found_rowsbefore the intersect, thenmain's new mariadb_capabilities line.
All 7
sql-mysql-found-rows.test.tscases pass after each rebase.[review] gate passed · iteration 10 · 10 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 10
evidence per changed file
root cause · written by the author bot
Bun's MySQL client never requested the CLIENT_FOUND_ROWS capability during the protocol handshake, so UPDATE statements reported rows actually changed rather than rows matched, diverging from the default behavior of the mysql2 and mariadb drivers. The fix adds a foundRows connection option, settable via the options object or URL query string and defaulting to true, which is threaded from option parsing through the native binding into the connection's capability negotiation. When enabled, the CLIENT_FOUND_ROWS flag is set in the requested capabilities before intersecting with the server's, s…