fix(plugin-mysql): report the real error when a query fails while reading rows (#1884)

[datlechin]

Fixes #1884.

A MySQL SELECT ending in ORDER BY came back as an empty grid: no rows, no error, ~0.1s. The same query without the ORDER BY returned rows, and the reporter's other MySQL clients ran it fine.

Root cause

Two layers, both ours.

1. The silent failure (this is the bug). mysql_fetch_row() returns NULL for two different things: a clean end of rows, and an error part way through reading. The C API requires checking mysql_errno() to tell them apart. executeQuerySync only checked errno inside its if truncated branch, so when the loop ended naturally it returned whatever rows it had, [] if the failure hit before row one, as a successful query. Column metadata arrives before row data, so the result still had real column headers. That combination is exactly the report: correct-looking grid, zero rows, no error. The prepared-statement path had the same defect, treating MYSQL_NO_DATA (real end of rows) and a genuine error the same way.

ORDER BY exposes it because a filesort makes the server read, sort, and materialize the whole result before it sends row one, so the client's first fetch blocks through that entire window. An unsorted scan streams immediately and has almost no equivalent window. Parse-time errors were already caught by mysql_real_query's return code, which is why only this class slipped through silently.

2. The trigger. Since 0.55.0 the row cap was applied by appending LIMIT cap+1 to the SQL. That is not neutral: adding a LIMIT to an ORDER BY query moves MySQL 8.0 onto an ordered index scan (prefer_ordering_index) instead of a filesort. It is the one thing that differed between TablePro and the clients that worked, since they send the query as written.

What changed

• The MySQL driver checks mysql_errno() on every exit path and throws the error the server reported, so a real failure shows up as an error instead of an empty table.
• The row cap no longer rewrites the SQL. The query runs exactly as typed and the cap applies to the rows read back, which is how SQL Server and Oracle already behaved here, and what DataGrip and DBeaver do. SQLLimitInjector and the injectRowLimit driver hook are gone.
• On truncation the driver issues KILL QUERY through the existing cancel path instead of draining every remaining row over the wire.
• Fixed a separate bug: setting the row cap to unlimited (0) resolved to 0 rather than nil, so the over-fetch became LIMIT 1 and unlimited silently returned a single row.

Note for review

The injector did two jobs, and only one was wrong. It also detected an existing top-level LIMIT/FETCH/TOP and skipped the cap for that run. Deleting it outright would have capped an explicit LIMIT 50000 down to 10,000 and regressed #956 ("your own LIMIT wins"). The lexer is kept as a read only SQLLimitDetector; the string rewriting is what was unsound, not the scanning.

ABI

scripts/check-pluginkit-abi.sh main reports one change: injectRowLimit removed from PluginDatabaseDriver and its default extension. It defaulted to nil, which CLAUDE.md lists as additive, so no currentPluginKitVersion bump and no plugin re-release. Needs the abi-additive label.

Testing

• swiftlint lint --strict: 0 violations.
• New SQLLimitDetectorTests covering both reported repros, plus the ORDER BY coverage the old injector tests never had.
• New resolveRowCap tests, including the unlimited-returns-one-row regression.
• TableProPluginKit builds via SwiftPM after the protocol removal.

Not covered by the suite: the driver fix needs a live MySQL to exercise, since it is C-bridge code. Worth confirming on a real server that KILL QUERY on a truncated result surfaces ER_QUERY_INTERRUPTED (1317) and not a connection-lost code, otherwise every truncated query would report an error.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
TablePro	🟢 Ready	View Preview	Jul 16, 2026, 4:26 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4e7e52580

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Enforce rowCap before executing default plugin queries

When a capped SELECT reaches a plugin that relies on PluginDatabaseDriver’s default executeUserQuery (for example PostgreSQL/ClickHouse/DuckDB-style .limit plugins), this now sends the original SQL and only passes rowCap; the default implementation calls execute(query:) first and trims raw.rows after the full result has already been materialized. Before this change, plan.executedSQL contained the injected LIMIT cap+1 for uncapped .limit dialect queries, so the row-cap setting bounded server/client work. Large uncapped result sets can now still be fully fetched and held in memory despite the row cap, so the affected drivers or default path need to stream/stop at rowCap before dropping the server-side limit.

Useful? React with 👍 / 👎.