fix(ios): six collateral defects found while fixing #2543 and #2544

[datlechin]

The six defects the investigation behind #2548 found and reported rather than fixed. One commit each.

Safe Mode let a write run when it followed a comment or a CTE

QueryEditorView.isWriteQuery classified by hasPrefix over eight uppercased keywords, so recognition required the write verb to be the literal first characters. Anything else fell through the safeModeLevel.writePermission gate and ran unconfirmed. On a Read-Only connection:

-- cleanup
DELETE FROM users

A write-keyword allowlist cannot be made safe, because whatever it has not been taught runs unguarded. SQLWriteClassifier in TableProQuery inverts it: a statement is a read only when its leading keyword is one of a short closed set, and everything else writes, including a keyword it has never seen. It strips leading line and block comments, splits a batch on semicolons that are not inside a literal, an identifier quote or a comment, and treats any write in the batch as a write.

Five holes were found by compiling and running an earlier draft, and each has a test:

Input	Was	Why it matters
EXPLAIN ANALYZE DELETE FROM users	read	PostgreSQL executes the delete
PRAGMA journal_mode = WAL	read	writes on SQLite and DuckDB
SELECT * INTO backup FROM users	read	creates a table on SQL Server
SELECT 'a\' ; DELETE FROM users	one statement	PostgreSQL sets standard_conforming_strings = on, so the backslash is not an escape and this is two statements
WITH x AS (…) SELECT * INTO backup FROM x	read	same as row three, through the CTE path

Redis needed its own arm. It speaks commands rather than SQL, so the SQL path called every GET a write, and Read-Only would have refused every read on a supported engine whose Query tab is always shown. The read-command set is the one QueryClassifier.swift:436-451 already curates for the Mac.

The classifier lives in TableProQuery, which only TableProMobile and TableProCore's own tests link, so this compiles nothing the macOS app builds. macOS keeps its own QueryClassifier; sharing one would mean generalising ~1,300 lines over two different DatabaseTypes and was out of scope here.

Deep links could never work on iOS

TableProMobileApp.onOpenURL parses tablepro://connect/<uuid>, the Open Connection intent opens that URL, both widget views and the Live Activity build it, and docs/ios/index.mdx says deep links work as on the Mac. CFBundleURLTypes was absent from the iOS Info.plist, so the app claimed no scheme and the OS had nothing to route to. Measured: PlistBuddy reported the key does not exist on the built app, and simctl openurl failed with LSApplicationWorkspaceErrorDomain error 115 until the plist was patched by hand.

The entry mirrors the Mac's first one exactly. The Mac's second entry, the postgresql/mysql schemes, is deliberately not copied: nothing on iOS parses an external database URL, and an unused claim is how this drifted in the first place.

PostgreSQL and Redshift read the wrong schema

fetchTables, fetchColumns, fetchIndexes and fetchForeignKeys each resolved a nil schema to the literal "public" and never read currentSchema, which switchSchema keeps in step with search_path. Measured against PostgreSQL 17: with an app.orders of three columns and a public.orders of two, the driver returned two columns, so the app showed and edited metadata belonging to a different table rather than simply finding nothing.

effectiveSchema alone would have been inert. connect() never probed, so currentSchema stayed the seeded "public" and the fallback returned the literal it replaced on every fresh connection; the fix would only have bitten after the user switched schema by hand. adoptServerSchema runs SELECT current_schema() at connect, the way MSSQLDriver and OracleDriver already do. Measured: returns app for a role carrying ALTER ROLE … SET search_path TO app, and public otherwise.

SQL Server read and wrote the login's default schema

T-SQL has no session-level schema to set, so MSSQLDriver.switchSchema correctly only records the choice, and metadata already followed it. The data statements did not: every SELECT, COUNT, UPDATE and DELETE named a bare table, which resolves against the login's default schema rather than the one in the toolbar.

Two paths were worse than the report described, and neither was in it:

• TableListView ran TRUNCATE TABLE and DROP TABLE unqualified. With a sales.orders and a dbo.orders, the destructive statement could hit the wrong one while the UI named the other.
• FKPreviewView built an unqualified SELECT reachable from the row context menu.

schema is now a required parameter on all ten SQLBuilder data builders, so no call site can silently stay unqualified, and it flows from coordinator.activeSchema through the view model. The foreign-key preview uses ForeignKeyInfo.referencedSchema rather than the browsing schema, because a key may point into another schema; no iOS driver populates that field yet, so it stays unqualified exactly as before and becomes correct the moment one does.

The browser keeps one schema snapshot. An earlier version recomputed it per child, so a schema switch with a table open left the view model on the old schema for the rows and deletes while the row editor and insert form used the new one, and an edit could reach a different table than the rows on screen. Every statement now reads viewModel.schema, and a switch re-attaches and reloads.

This reaches five engines, not one: supportsSchemas covers PostgreSQL, Redshift, SQL Server, DuckDB and Oracle.

Computed SQL Server columns were offered as editable

MSSQLColumnRow carried no computed flag, so ColumnInfo.isGenerated stayed false and a computed column rendered as an ordinary field. Measured on SQL Server 2022: COLUMNPROPERTY(…, 'IsComputed') returns 1 for it, writing to it gives Msg 271 … cannot be modified because it is either a computed column or is the result of a UNION operator, and omitting it computes correctly.

The macOS plugin had the same bug independently, and PluginColumnInfo.isGenerated predates #2548 and was simply never populated. Because the plugin owns its own DML generation, populating the field was not enough on its own: computed columns now join the identity cache that generateMssqlInsert already filters, and fetchAllColumns populates both caches, having previously populated neither.

Exports corrupted data on a round trip

Three separate defects in ClipboardExporter:

• CSV wrote row[i] ?? "NULL" unquoted, so a real NULL and a cell holding the text NULL were identical. NULL is now an unquoted empty field and the text is quoted.
• JSON decided numeric-versus-string by parsing the cell's own text, so Int64("01234") succeeded and the export emitted 01234 unquoted, which no JSON parser accepts. It now tests the JSON number grammar over ASCII digits, which also keeps +5, 0x10 and ١٢٣ quoted. Gating on the column type was rejected: columns is the result set, MySQL reports NEWDECIMAL, and SQLite reports "" for every expression, so SELECT COUNT(*) would have regressed to a quoted string.
• SQL INSERT hard-coded ANSI double-quoted identifiers, which MySQL and MariaDB reject as a syntax error whatever the escaping, and applied ANSI literal escaping everywhere. Identifiers now go through SQLBuilder.quoteIdentifier and literals through the driver, matching how SQLBuilder already works. This also closes an unescaped embedded quote in an identifier.

parseCSV is deliberately unchanged. Pairing CSV NULL → "" with parseCSV "" → .null would turn every empty string into NULL on re-import, which is the same indistinguishability being fixed.

Verification

• iOS: 267 tests pass, 0 fail
• TableProQuery: 21 classifier tests pass
• macOS app: builds clean
• MSSQLDriver plugin target: builds clean, which PR CI never does because MSSQL is registry-only
• SwiftLint: 0 violations across TableProMobile, Packages/TableProCore/Sources and Plugins/MSSQLDriverPlugin; docs checks pass

Behaviour was measured against a real PostgreSQL 17, a real SQL Server 2022, sqlite3 and the duckdb CLI rather than read off documentation. Both patched MSSQL queries were run against the live server to confirm IS_COMPUTED lands at the index the parser reads.

Two reviewers read the diff. A code-review pass produced six findings, five fixed here. A Codex pass produced five more, all addressed, including the schema-snapshot desync above and a CTE that called replace() classifying as a write, which would have made Read-Only block an ordinary read. A self-directed security pass then found one more, fixed here: narrowing the CTE scan to remove that false positive had reopened SELECT INTO through the CTE path.

Two things to know before this ships

MSSQL is registry-only. The plugin half of the computed-column fix reaches users through a plugin-mssql-v* tag rather than the next app release. No plugin has been published.

isGenerated now also blocks grid edits on macOS for computed columns, via DataGridView+Editing.swift. That is correct, since SQL Server refuses the write, but it is a visible change to existing behaviour.

Not covered

TableProMobile has no UI test target, so none of this could get XCUITest coverage. Oracle and DuckDB still report neither identity nor generated columns, and SQL Server reports no generated flag on iOS beyond the computed one added here; their metadata queries live in packages the macOS app also links. A column left on DEFAULT is omitted on every engine regardless, so the user-facing behaviour is correct everywhere; what those engines lose is the extra refusal when a value is typed in.

[chatgpt-codex-connector]

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