fix: stop rewriting user SQL in query execution layer (#956)#960
fix: stop rewriting user SQL in query execution layer (#956)#960
Conversation
# Conflicts: # CHANGELOG.md # TablePro/Core/MCP/MCPConnectionBridge.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4624cf17a
ℹ️ 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".
| return nil | ||
| } | ||
| return (false, 0) | ||
| return dataGridSettings.validatedQueryResultRowCap |
There was a problem hiding this comment.
Treat zero row cap as unlimited before executing user query
resolveRowCap now returns validatedQueryResultRowCap directly, but that setting explicitly uses 0 to mean “unlimited”. Passing 0 into executeUserQuery is interpreted as a real cap, so any non-empty result gets truncated to zero rows (prefix(0)) and marked truncated. This breaks query execution whenever users have row cap set to 0 with truncation enabled (including migrated/manual settings), returning empty grids instead of full results.
Useful? React with 👍 / 👎.
Summary
LIMITclause and substituted the app's row cap, soSELECT ... LIMIT 10returned 10,000 rows.fetchFirstPage/fetchNextPage/fetchRows/fetchRowCountfamily from the driver protocol and replaces it withexecuteUserQuery(query:rowCap:parameters:). User SQL passes to the database unchanged; the row cap is enforced at the result level instead of by query rewrite.Showing N rows (truncated) [Fetch All]banner. Table-tab pagination is unchanged.What changed
PluginDatabaseDriver,DatabaseDriver,PluginDriverAdapter): six paged methods removed,executeUserQueryadded with a Strategy B default (run query, slice client-side if rows exceed cap, setisTruncated). PluginKit ABI bumped from 8 to 9.stripLimitOffsetregex removed from SQLite, MySQL, PostgreSQL, Redshift, ClickHouse, DuckDB, LibSQL, CloudflareD1. SQLite override uses Strategy A (collect fromstreamRows, break at cap). All other SQL plugins inherit Strategy B from the protocol default. MSSQL/Oracle no longer injectORDER BY 1/ORDER BY (SELECT NULL)into user SQL. All plugins bumped to ABI 9 inInfo.plist.MCPConnectionBridge.executeQuerynow callsexecuteUserQueryand readsisTruncatedfrom the driver instead of computing it from a+1probe. The timeout race andcancelQuerywiring are unchanged.MainContentCoordinator+QueryHelpersand+LoadMoreupdated;loadMoreRows()deleted;fetchAllRows()routes throughexecuteUserQuery(rowCap: nil).pagination.hasMoreRowssemantics narrow to "result was truncated".enforceQueryResultLimitrenamed totruncateQueryResults,queryResultLimittoqueryResultRowCap. No fallback decode. Custom values revert to default on first launch.docs/features/data-grid.mdxProgressive Loading section rewritten as Result Truncation. Stale "sorting is local to the loaded page" line corrected.TableProTests/Core/Database/...for SQLite covering userLIMIT,OFFSET, CTE, UNION, subquery, truncation flag, and Fetch All.Test plan
Mandatory regression on SQLite (use the issue reporter's database or any 1000+ row table):
SELECT * FROM t LIMIT 10returns exactly 10 rows, no truncation bannerSELECT * FROM t LIMIT 5 OFFSET 50returns 5 rows starting at row 51WITH cte AS (SELECT * FROM t LIMIT 3) SELECT * FROM ctereturns 3 rowsSELECT * FROM ton a 50k-row table shows "Showing 10,000 rows (truncated)" + Fetch AllCross-plugin smoke (whichever you have access to): same
LIMIT 10query against MySQL, PostgreSQL, Redshift, MSSQL, Oracle, ClickHouse, DuckDB, LibSQL, CloudflareD1, BigQuery.Settings:
queryResultLimitvalue is gone (revert to 10,000 default; per CHANGELOG)Non-regression:
LIMITrespects the LIMITCmd+.) interrupts a long-running queryMCP path:
SELECT * FROM huge_table LIMIT 5returns 5 rows,is_truncated: falsemaxRows(e.g. 10) returns 10 rows,is_truncated: trueSELECT 1;;;) executes cleanlyExternal plugins:
Manual screenshot follow-up: re-capture
images/progressive-loading.pngand the dark variant after build (UI changed from button row to banner row).