perf(plugin-mysql): bound a capped read at the server with SQL_SELECT_LIMIT (#2427)

[datlechin]

Fixes #2427.

What is actually wrong

TablePro bounds a large MySQL or MariaDB read on the client, then tries to stop the server through a side channel: a brand-new connection issuing KILL QUERY. When that side channel is refused, misrouted or simply slow, the whole result set still crosses the network before the first row reaches the grid. The parameterized path never even tries.

#2430 added the executeBoundedQuery hook and its body claimed MySQL "already did ... Unchanged". It did not. grep executeBoundedQuery Plugins/MySQLDriverPlugin/ finds nothing, so the PluginKit default returns nil and every MySQL query falls back to executeUserQuery. The claim was about the older client-side cap in executeUserQuery, which is the mechanism that is broken.

The side channel is unavoidable given that design, and I measured why:

mysql_kill() on the live connection, mid-result	errno 2014 Commands out of sync
mariadb_cancel(MYSQL *)	rc = -1, and it kills the connection (errno 2026, then 2006 Server has gone away)

So a second connection genuinely is the only way to stop a result set that is already streaming. Which is the argument for never starting one.

MySQL and MariaDB expose a server-side row cap, SQL_SELECT_LIMIT, and TablePro has never used it. It is what MySQL Connector/J implements Statement.setMaxRows() with (ConnectionImpl.java:2388-2394), which is how DBeaver bounds a MySQL read on its default path.

Measured, end to end, through the real plugin bundle

A Swift harness loads a built MySQLDriver.tableplugin and calls the driver directly. Baseline is the plugin built from 5f162aa79, the merge base. Local MariaDB 12.3.2, 2,000,000-row InnoDB table, SELECT * FROM big, cap 1,000, loopback.

A server that allows the second connection:

	before	after
executeBoundedQuery	nil, the hook is dead	2.2 ms, 1,000 rows, truncated, columnMeta intact
executeUserQuery	36.7 ms	1.8 ms
SELECT ... LIMIT 1000 (the ceiling)	2.0 ms	2.1 ms

A server that refuses it (a user at MAX_USER_CONNECTIONS 1, standing in for AWS RDS denying plain KILL, a proxy where mysql_thread_id names the wrong backend, or a connection limit):

	before	after
executeUserQuery	679.0 ms	1.6 ms

That 679 ms is 138 MB of rows nobody asked for. On a 100 Mbit link it is about 11 seconds, which is the reporter's "around 10 seconds".

Deleting the client-side drain loop would not have helped: mysql_free_result on an unbuffered result reads the tail itself, 0.813 s against 0.818 s. The cost belongs to the server still sending.

The parameterized path was worse, because it applied the cap and then issued no KILL and no drain at all, leaving mysql_stmt_close to pay for everything: 875 ms, against 0.9 ms with the limit set.

The change

MariaDBPluginConnection gains one concept: a statement that arrives with a row cap asks the server to stop at cap + 1 before it runs. The invariant that makes it safe is that a rowCap already means the caller discards rows past the cap, so a server-side limit is a no-op on what the caller sees; it only stops the rows being produced.

• MySQLSelectLimitStatement.swift holds the pure statement builders and the two decisions worth testing: what to reconcile to, and what a fetch count means.
• reconcileSelectLimit runs before every statement on the primary handle, from the connection's serial queue, and moves its cached value only when the server confirms the change. Failing to install a cap costs only the client-side fallback and is logged. Failing to move one that is already installed throws, because running the statement under a stricter limit than it asked for and reporting the short result as complete is silent data loss.
• The cap is not installed in front of a statement that can only ever return one row, and only while this connection holds no limit of its own. A SET resets what ROW_COUNT() reports, so UPDATE t SET ...; SELECT ROW_COUNT() would have answered 0 rather than 3, measured on both engines. The test strips comments, string literals and quoted identifiers first, so a keyword spelled inside one is never read as syntax, and FROM DUAL still counts as no table.
• The session's own SQL_SELECT_LIMIT is captured lazily, the first time this connection is about to take the variable over. At connect would be too early: the connection's startup commands run after connect() returns, so a SET SESSION SQL_SELECT_LIMIT among them would have been lost rather than restored.
• Reconciling before every statement, rather than leaving the variable set, is what keeps a capped read from truncating the next information_schema query on the same connection. Measured: it does apply to those. Back-to-back capped reads at one cap still issue one SET and none after, which is Connector/J's own optimisation.
• The client reads one row past the cap so the result reaches its natural end. killQueryOnServer and the drain survive but now fire only when the server ignored the limit: a CALL, or a statement carrying its own larger LIMIT, both measured uncapped by the variable.
• MySQLPluginDriver.executeBoundedQuery delegates to the capped read, the one-line idiom KafkaPluginDriver.swift:200 already ships for a driver bounded at its source. Not boundedQueryFromStream, which cannot carry columnMeta.

Why SQL_SELECT_LIMIT is safe here

Measured on both MariaDB 12.3.2 and MySQL 8.4.11, and committed as scripts/check-mysql-select-limit.sh so a server upgrade re-checks it rather than trusting this paragraph:

• Bounds the outermost result: top-level SELECT, UNION [ALL], VALUES, TABLE t, an information_schema SELECT.
• Leaves everything else alone: scalar subqueries, derived tables, CTEs, INSERT ... SELECT, CREATE TABLE ... SELECT, REPLACE ... SELECT. All copied every one of their rows with the limit set.
• An explicit LIMIT always wins, above and below the session value.
• SET SQL_SELECT_LIMIT = DEFAULT restores. It is not transactional, so ROLLBACK does not restore it and USE <db> does not clear it; the reconcile is the only thing that touches it.

Your SQL still reaches the server exactly as you wrote it. Nothing is rewritten.

Honest limit

This bounds the rows the server returns. It cannot make a blocking plan node finish sooner. On the same table, ORDER BY on an unindexed column went 925 ms to 462 ms, and GROUP BY stayed at about 11.3 s for every variant including an explicit LIMIT, because mysql_real_query blocks before row 1. We do not have @Nisgrak's SQL. If it is a GROUP BY, this removes the transfer but not the wait, and that 11.3 s is uncomfortably close to the reported number, so the query would be worth seeing.

Also fixed

killQueryOnServer set no MYSQL_OPT_PROTOCOL, while the primary connect sets MYSQL_PROTOCOL_TCP. Measured with the shipped library: with the host spelled localhost the primary reaches localhost via TCP/IP and the kill connection reaches Localhost via UNIX socket, ignoring the port argument entirely. Against a second local instance on another port it still went down the default socket, so KILL QUERY <threadId> landed on a different server, where that thread id belongs to somebody else's session. It now repeats the primary's transport and its cleartext-auth option, and logs a refused connect or a rejected KILL instead of discarding it silently.

Rejected

• Rewriting the SQL to append a LIMIT. TablePlus does this and it is TablePlus Connect to Oracle database, program bengk #1683 (a trailing -- swallows it) and macOS Tests Gate is not a required check, so PRs merge red #2422 (a leading comment skips it).
• SET STATEMENT SQL_SELECT_LIMIT=n FOR <stmt>. Works and self-restores, but MariaDB-only, and still text rewriting.
• A read-only server cursor (STMT_ATTR_CURSOR_TYPE = CURSOR_TYPE_READ_ONLY with STMT_ATTR_PREFETCH_ROWS), the other classic server-side bound. Measured: mysql_stmt_execute alone costs 805 ms because the server materialises the whole result into an internal temporary table before returning. It makes the server do more work, not less.
• CLIENT_MULTI_STATEMENTS to fold the SET into one round trip. The connection passes client_flag = 0 today, and widening the injection surface to save a round trip is a bad trade.

Verification

Step	Result
verify.sh build	PASS
verify.sh test MySQLSelectLimitTests BoundedQueryTests PluginStreamAbortTests MySQLQueryTimeoutTests QueryExecutorTests	PASS, 75 cases
verify.sh lint Plugins/MySQLDriverPlugin TableProTests/Plugins	0 violations
MySQLDriver scheme	BUILD SUCCEEDED
shellcheck --severity=warning scripts/check-mysql-select-limit.sh	clean
scripts/check-mysql-select-limit.sh	all checks agree, on MariaDB 12.3.2 and MySQL 8.4.11
End-to-end harness, before and after	the tables above
Codex review	5 findings, all acted on
Codex adversarial-review	6 findings, all acted on

New tests: MySQLSelectLimitTests over the reconcile decision and the fetch-count outcome, including the case a reviewer caught in the draft, where leaving the client cap at cap rather than cap + 1 would have kept the KILL firing on every capped SELECT. Two cases added to BoundedQueryTests for a driver bounded at its source, so the hook returning nil (the opt-in present but doing nothing) fails.

No UI automation: the flow needs a live MariaDB with a multi-million-row table and no deterministic fixture exists for it. No screenshots: nothing visual changed, the row cap affordance already shipped.

swiftformat could not run: the local binary rejects --ifdefindent from .swiftformat. SwiftLint is clean.

No PluginKit change, so no ABI bump and no plugin re-release. MySQL is a bundled plugin and ships with the app.

What the review changed

Codex reviewed the diff and found five things. All five were real and all five are fixed; the measurements are in this PR because they came out of chasing them.

1. A hidden SET clobbers ROW_COUNT(). Measured 3 to 0 on MariaDB 12.3.2 and MySQL 8.4.11. Fixed by not installing the cap on a statement that reads no row source. SHOW WARNINGS turned out to survive the SET, so that half of the finding did not hold.
2. A failed reconcile still ran the user's SQL. A stale, stricter limit would end an uncapped SELECT at oldCap + 1 rows and report it complete. It now throws, and the cancellation gate is rechecked after the hidden SET.
3. The reset discarded a session limit the connection arrived with. Now captured at connect and restored.
4. The live-server check could not detect a broken reset. It counted SELECT COUNT(*) FROM (SELECT ...), whose one-row outer result is not bounded by the session limit, so it passed either way. It now counts rows the client actually received.
5. The check script put the password in argv. It goes through the environment now.

A sixth came out of reviewing my own diff for the security pass: the kill connection had gained MYSQL_ENABLE_CLEARTEXT_PLUGIN without mirroring the primary's TLS, so cleartext auth could have gone over a plaintext channel.

Then adversarial-review attacked the result and returned "do not ship" with six more. All six were real:

1. A stale cap could be reported as a complete result. Run a query at cap 5, raise the Row Cap setting, then run a multi-row query with no FROM: the draft skipped reconciliation entirely, the server ended at the old limit, and the client called the short result complete. The skip now applies only while the connection holds no limit of its own, so a misread statement can cost a redundant SET but never a silent truncation.
2. SELECT ROW_COUNT() FROM DUAL, and FROM inside a comment or a literal, defeated the first version of that test. It is now a proper scan that strips comments, strings and quoted identifiers, and it recognises set operations and VALUES rather than just looking for FROM.
3. The kill connection forced TLS from the configured mode. .preferred is the default and falls back to plaintext, so on a server without TLS the primary would connect and every KILL after it would repeat the attempt that had already failed, breaking Stop. It now reproduces what the primary actually negotiated, read from mysql_get_ssl_cipher.
4. The baseline was captured too early and could not see a limit of 0. Fixed by capturing lazily and giving the probe its own LIMIT 1, since an unbounded probe returns no rows at all when the session limit is 0 and reads as a failed probe.
5. The check script would have dropped a pre-existing database of the same name. It now creates a uniquely named one, fails rather than adopting a collision, and arms its cleanup only after the create succeeded. Verified: a planted tablepro_select_limit_check survives a run untouched.
6. The check script took the password as an argument and always disabled TLS. The password now comes from MYSQL_PWD only, and TLS is negotiated normally unless --insecure is passed, which is refused for anything but a loopback host.

The three behaviours those findings named are now measured against the baseline plugin, and match it: ROW_COUNT() after an UPDATE of 3 rows returns 3, ROW_COUNT() FROM DUAL returns 3, and a 7-row constant query run after a cap of 5 returns 7 rows with truncated=false.

Found while investigating, not shipped here

Each was verified by an adversarial pass that tried to refute it, and each has a reproduction. None is a regression from this change.

1. User Stop blocks the main thread for up to 5 seconds. DatabaseManager is @MainActor and cancelRunningQuery(reach: .userStop) is deliberately synchronous; for MySQL that runs a blocking mysql_real_connect. Measured against an unroutable host: 5,001 ms, the MYSQL_OPT_CONNECT_TIMEOUT. PostgreSQL's PQcancel costs 70-160 ms by comparison. MariaDBPluginConnection.swift:384.
2. mysqlStatementIsReadOnly treats every SELECT as replayable. After a dropped connection executeWithReconnect re-runs it, so SELECT NEXTVAL(seq) burns a second sequence value and SELECT GET_LOCK(...) re-acquires. MySQLStatementClassification.swift:6.
3. The shipped Libs/libmariadb_arm64.a is Connector/C 3.4.4 while the vendored header declares MARIADB_PACKAGE_VERSION 3.4.8.
4. [Unreleased] in CHANGELOG.md carries two ### Fixed headings. CLAUDE.md says a section appears at most once per version. Left alone here because restructuring it would conflict with every in-flight branch.

https://claude.ai/code/session_01KsqHrFwJxUW6eWozYjT8JZ

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.