Skip to content

fix(ios): insert-row defaults and the missing table list search field - #2548

Merged
datlechin merged 1 commit into
mainfrom
fix/ios-insert-defaults-and-table-search
Aug 27, 2026
Merged

fix(ios): insert-row defaults and the missing table list search field#2548
datlechin merged 1 commit into
mainfrom
fix/ios-insert-defaults-and-table-search

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #2543
Fixes #2544

Two iOS defects that turned out to share a shape: a three-valued choice modelled as a two-valued one.

#2543 Insert Row cannot leave a column on its database default

Root cause

InsertRowView held a per-column state that is really three-valued (use the database default, SQL NULL, a typed literal) as two parallel arrays, values: [String] and isNullFlags: [Bool]. Two booleans cannot express three states, so "untouched" and "empty string" collapsed into one and buildInsertSQL appended every column to the INSERT. The only omission path was a heuristic on primary keys. ColumnInfo.isNullable was never read, so the NULL badge was offered on NOT NULL columns where it can only fail.

Measured against MySQL 8.4 with the default sql_mode, on the reporter's table shape:

Statement Result
INSERT ... VALUES ('', ...) ERROR 1366 Incorrect integer value: ''
INSERT (created_at, ...) VALUES (NULL, ...) ERROR 1048 (23000) Column 'created_at' cannot be null
INSERT (amount) VALUES ('5'), omitting the rest succeeds, created_at = now, name = its default
INSERT (amount, gen) VALUES ('5','10') ERROR 3105, a generated column rejects any value

The second row is the issue's error text verbatim.

The fix

The three-valued model already existed next door. PayloadValue (.null / .text, with an absent key meaning "leave it out") and RowInsertPlanner are what the App Intents path has always used, which is why a Shortcut could already leave a column on its default while the UI could not. InsertRowView now holds one [String: PayloadValue] keyed by column name, and both paths share the planner. PayloadRow and RowInsertPlanner moved from Intents/ to Helpers/ now that two features depend on them.

  • A per-field menu switches between DEFAULT, NULL and a typed value. NULL appears on nullable columns only.
  • A generated column is never written, whatever the payload says. Every engine rejects one.
  • An auto-increment column is omitted while it is on DEFAULT, and an explicitly typed key is still written. This matches macOS SQLStatementGenerator, which excludes only generated columns, and keeps the shipped includesProvidedPrimaryKey contract for the Add Rows Shortcut.
  • A table whose every column is auto-increment or has a default still inserts. There is no portable statement for it, so the dialect decides: INSERT INTO t () VALUES () on MySQL and MariaDB, INSERT INTO t DEFAULT VALUES on PostgreSQL, Redshift, SQLite, SQL Server and DuckDB, and nothing on Oracle, which accepts neither. All four forms were measured, not read off a doc page.
  • The Shortcuts path keeps its old behaviour. allowAllDefaults and dropsEmptyPrimaryKey both default to what a payload has always meant, where absence is the only way to say "leave this out". The form turns both off, because it distinguishes DEFAULT from an explicitly empty value: picking Empty String on a text primary key now writes '' rather than silently dropping the column while the badge reads VALUE.

ColumnInfo gained isAutoIncrement and isGenerated, populated from metadata the drivers already fetch: MySQL's Extra column (index 6 of SHOW FULL COLUMNS, previously read past), SQL Server's IsIdentity (already parsed into MSSQLColumnRow and dropped on the floor), and PostgreSQL's is_identity / is_generated. SQLite reports a lone INTEGER PRIMARY KEY as the rowid alias, and its PRAGMA table_info already hides generated columns. Oracle and DuckDB keep the defaults rather than change a schema query the macOS app also links; they lose nothing, because a column left on DEFAULT is omitted regardless.

ColumnInfo lives in TableProModels, which only the iOS app and TableProCore's own targets import. The macOS app has its own separate ColumnInfo, and the plugin boundary type is PluginColumnInfo, which already carries both fields. This is not a PluginKit ABI change and needs no version bump.

MySQL DEFAULT_GENERATED contains the substring GENERATED but is a default, not a generated column. There is a test for that specifically.

#2544 The table list search field never appears

Root cause

Two structural facts, both measured on an iOS 27.0 simulator with a probe app built for the iOS 18 deployment target:

  1. A .searchable applied inside a Tab reaches no navigation bar, because the TabView is the NavigationStack's root and the tab content has no navigation container of its own. A .navigationTitle inside the tab is dropped identically. Moving .searchable onto the TabView renders nothing either.
  2. A TabView whose tabs each own a NavigationStack renders no navigation bar at all when that TabView is a NavigationSplitView detail. Verified with one binary and one variable: the identical connection screen renders its title and search field as a root, and neither when wrapped in NavigationSplitView { sidebar } detail: { it }. Broken on iPad too, where the split view does not collapse.

So both suggestions in the issue fail. Tab(role: .search) renders the iOS 26 search affordance but its own .searchable still does not surface, and it is the wrong control anyway: a search-role tab owns search for the whole tab view rather than filtering one list. Inverting the nesting alone is defeated by (2).

Nine structures were measured in total. .searchable(placement: .navigationBarDrawer(displayMode: .always)) does force a field into the current nesting, but displaces the tab bar; .searchToolbarBehavior(.minimize) and .searchPresentationToolbarBehavior(.avoidHidingContent) restore the tab bar and suppress the field, and both are iOS 26 only.

The fix

The connection screen becomes a root: ConnectionListView presents it in a .fullScreenCover instead of a NavigationSplitView detail, and ConnectedView puts the TabView outermost with one NavigationStack per tab. .tabViewStyle(.sidebarAdaptable) (iOS 18) gives iPad a sidebar for the four sections instead of a phone tab bar. TableListView keeps its .searchable exactly as written; it then renders. coordinator.tablesPath and the DataBrowserView destination moved into the Tables tab's stack, and the shared toolbar is applied per tab.

NavigationSplitView supplied the back affordance for free, so ConnectedView gained an explicit Connections button. That is the visible cost of the change, along with the iPad connection list no longer sitting beside the connection.

The two search fields also had one @SceneStorage key each, shared by every connection and every table, so a filter typed in one connection silently hid rows in another with no field on screen to reveal it. The table list filters client-side and applies immediately, so its text is now keyed per connection and still restored. The data browser searches on the server and applies on submit, so restoring its text would name a filter the rows on screen were never fetched under; it is plain @State and no longer persisted.

Also fixed in the same files

  • Insert Row seeded its state arrays from columnDetails.count in init, and the + button was not gated on the column list having loaded. Opening the sheet early left the arrays empty while the form rendered every section, so every keystroke was discarded by the bounds guard and every column inserted as ''. Keying by column name removes the failure mode; the button is gated as well.
  • isPrimaryKey && typeName.contains("INT") pre-nulled every integer primary key, and the insert then dropped it. A composite PRIMARY KEY (order_id, line_no) over two INT NOT NULL columns rendered both as the word NULL, labelled both auto-increment, and omitted both. The heuristic is gone, replaced by real metadata.
  • The row editor offered NULL on NOT NULL columns too, the same defect on the sibling write path. It needed no driver work: ColumnInfo.isNullable was already there.
  • Insert Row was offered on Redis connections, where the generated SQL is sent as a Redis command and can only fail. Both entry points, the toolbar button and the empty-table placeholder, now ask the same question.

Verification

  • xcodebuild build, iPhone 17 Pro Max, iOS 26/27 simulator: BUILD SUCCEEDED.
  • xcodebuild test -only-testing:TableProMobileTests: 250 passed, 0 failed.
  • swiftlint: 0 violations across TableProMobile and Packages/TableProCore/Sources/TableProModels.
  • Docs house style and source-claim checks: pass.
  • SQL semantics measured against a real MySQL 8.4 container, a real PostgreSQL 17 container, sqlite3, and the duckdb CLI, not read off documentation. INSERT INTO t () VALUES () is a syntax error on PostgreSQL, SQLite and DuckDB; DEFAULT VALUES is a syntax error on MySQL. Both forms are covered by tests.
  • The app was run in the simulator against that MySQL instance with a seeded connection.

New tests: ColumnMetadataRulesTests (including DEFAULT_GENERATED, the composite-key rank, and WITHOUT ROWID), SQLBuilderDefaultValuesTests, nine cases added to RowInsertPlannerTests covering omission, generated columns, an explicit auto-increment value, an explicitly empty primary key, and each dialect's all-defaults form, plus RowDetailViewModelTests.isNullableFollowsMetadata.

Review

Two reviewers read this diff. A security-review pass found nothing: the new SQL surfaces reuse the escaping the surrounding driver code already applies, and the value flow through escapeStringLiteral is unchanged. A code-review pass at high effort filed six findings; five were fixed here, and they are the reason the diff looks the way it does:

  • Both Insert Row entry points now share one canInsertRow. Only the toolbar button had been gated, so the empty-table placeholder still offered the sheet on Redis and before the column list arrived.
  • dropsEmptyPrimaryKey, above.
  • A "Nothing to Insert" alert that was unreachable on every engine has gone; a form footer explains the case where Save is genuinely unavailable.
  • The PostgreSQL capability probe caches its answer per driver instead of failing once per table, and re-throws CancellationError rather than swallowing it and issuing a second query.
  • The data browser search, above.

The sixth is real and deliberately not fixed here: MSSQLColumnRow carries no computed-column flag, so isGenerated stays false on SQL Server and a computed column is still offered as an editable field. Adding it means changing MSSQLSchemaQueries in a package the macOS app also links, which is the blast radius this PR set out to avoid. It joins Oracle and DuckDB in the gap list below.

An earlier Codex review was started and died before producing a verdict, so it is not counted among the reviewers. Its partial reasoning did surface three leads that were verified by measurement and fixed: the SQLite pk rank, WITHOUT ROWID, and the Redshift query break.

What could not be verified here

No before/after screenshots of the connected screens. The Xcode on this machine ships no Simulator.app, simctl has no touch-injection command, and simctl openurl raises a SpringBoard confirmation that cannot be dismissed without a pointer. The connection list was captured; the Tables tab and the Insert Row sheet were not. The structure they depend on was measured directly instead, with a probe app built at the iOS 18 deployment target and screenshotted on both an iPhone and an iPad running iOS 27, in the exact shape this PR ships.

No UI automation. TableProMobile has no UI test target at all (TableProMobile/project.yml declares only TableProMobileTests), so neither fix could get XCUITest coverage without adding one. Both flows also need a live database, which does not run deterministically on CI.

Oracle, DuckDB and SQL Server do not report generated columns, and Oracle and DuckDB do not report identity either. Their metadata queries live in packages the macOS app also links. A column left on DEFAULT is omitted on every engine regardless, so the reported bug is fixed everywhere; what these three lose is the extra protection of refusing to write such a column when a value is typed into it.

SQLite composite primary keys were mis-read app-wide before this branch: PRAGMA table_info reports pk as a 1-based rank, and the driver tested it for == "1", so only the first column of a composite key was treated as a key at all. Fixed here because the auto-increment rule depends on the count.

@mintlify

mintlify Bot commented Aug 27, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Aug 27, 2026, 3:59 AM

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

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@datlechin
datlechin merged commit 96d7cdd into main Aug 27, 2026
9 checks passed
@datlechin
datlechin deleted the fix/ios-insert-defaults-and-table-search branch August 27, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS 26: the table list search field never appears iOS: Insert Row cannot leave a column on its database default

1 participant