Skip to content

perf(compare): read a whole schema in a few queries and make a saved comparison set both endpoints - #2597

Merged
datlechin merged 3 commits into
mainfrom
feat/compare-sync-fast-and-fewer-steps
Sep 1, 2026
Merged

perf(compare): read a whole schema in a few queries and make a saved comparison set both endpoints#2597
datlechin merged 3 commits into
mainfrom
feat/compare-sync-fast-and-fewer-steps

Conversation

@datlechin

@datlechin datlechin commented Sep 1, 2026

Copy link
Copy Markdown
Member

A licensed user reported Compare & Sync as "slow and not very easy (needs many steps)". Both halves are real and both have a single cause.

Slow: the read was one statement per table, per kind, per side

CompareMetadataService.tableReads asked for columns, indexes, foreign keys and table metadata one table at a time. On MySQL fetchColumns is itself two statements (SHOW FULL COLUMNS plus a generation-expression lookup), so a table cost five round trips. Source was read to completion, then target. A 200-table pair cost 2 + 10 × 200 = 2002 round trips before one difference appeared.

Two things made it worse than it looks.

The existing concurrency was a no-op. CompareMetadataService fanned out four tasks against the driver withMetadataDriver yields, but that is one driver, and a driver dispatches its statements onto its own serial queue (MariaDBPluginConnection.swift:153). The four tasks queued behind each other. Its gate was supportsConnectionPooling, which answers whether a second connection is safe, not whether one connection can run two statements at once. Removed.

PluginKit already had three of the four whole-schema reads and the comparison called none of them. fetchAllColumns, fetchAllForeignKeys and fetchAllTriggers are single queries on every flagship driver. There was no whole-schema read for indexes or table metadata at all.

You cannot just switch to the bulk calls, which is why this is not a one-line change:

Driver fetchAllColumns against fetchColumns
PostgreSQL identical, both go through the same projection builder
MySQL dropped generationExpression and generationKind
SQLite used pragma_table_info, which omits generated columns entirely (its own fetchColumns uses table_xinfo for exactly that reason)

A naive switch would have silently stopped reporting generated-column differences on MySQL and made generated columns vanish from a SQLite comparison. Both are fixed first, and both drivers now share one shaping function with their per-table read so the two cannot drift again.

What the read does now

  • fetchAllIndexes(schema:) and fetchAllTableMetadata(schema:) added to PluginDatabaseDriver with N+1 default implementations, plus providesBulkColumnFetch / providesBulkIndexFetch / providesBulkTableMetadataFetch / providesBulkTriggerFetch alongside the existing providesBulkForeignKeyFetch. Implemented for MySQL, PostgreSQL and SQLite. This bumps PluginKit to 20, see below.
  • CompareMetadataService.read builds a read plan: whole-schema where the driver declares it, per-table where it does not, and per-table always for a caller that named specific tables (Copy Objects), which would otherwise read the rest of the database to throw it away.
  • Source and target read concurrently. SessionDriverGate is a FIFO queue per connection, not a lock a task deadlocks itself on, and these are concurrent rather than nested, so the worst case (both sides on one connection's shared driver) is that they serialise, which is what they did before.
  • A data comparison stops reading indexes and table metadata. It pairs tables, reads their shared columns and walks their rows; it needs foreign keys for statement ordering and looks at neither of the other two.

A 200-table structure comparison now costs four statements a side instead of a thousand, and the two sides overlap.

Many steps: the feature that saves the setup could not be reached without redoing the setup

CompareSyncSession.apply(_:) restored the mode, the kinds and the options and never set source or target. Meanwhile savedProfiles filtered on the current source, target and mode, so a saved comparison only appeared once its own two endpoints had been picked by hand. The stored scopes were used for nothing but the storage key.

  • Loading a saved comparison now sets both endpoints. One whose connection has since been deleted says so instead of loading half a pair.
  • The saved list is no longer filtered by the pair on screen, and a Comparisons pull-down sits on the toolbar's leading edge beside Source and Target. Save Comparison… is there and under Database > Compare, per the HIG's rule that every toolbar item is also a menu command.
  • The window reopens on the source, target, mode and options it last held. Opening it from a connection still makes that connection the source, and the remembered target comes back only when it was the target of that same source, because the target is the side that gets written to.
  • Data mode lists the tables both sides share as soon as there is a pair, without reading a row. It used to spend its first Compare discovering that list and then pay the whole metadata read again on the second, so a data sync cost two Compares.
  • Apply… builds the script when there is not one. Generate Script stays for reading or copying the SQL.
  • Structure results gained the Select > All / None control data mode already had. It covers what the pane is showing, so the search field and Show Identical Objects narrow what it reaches.

Found while working on this: the Apply sheet's hazard allowance was unreachable

applyDisabledReason returned a reason while any statement carried an unacknowledged hazard, and presentApplySheet guarded on canApply. So the sheet could only open once every hazard was already allowed, and the allowance UI inside it (added specifically because "the only checkbox that could allow one lived in the script pane behind this sheet") never rendered. Its onAppear, which opens the sheet on the Warnings tab when something is held back, could not fire either.

The gate moved to where the control is: applyDisabledReason asks only whether there is something to apply, and the new runRefusalReason is what the sheet's Apply button and CompareRunner.apply() both answer to, so an unallowed hazard still stops the run. The sheet's own copy said held-back statements "stay out of this run", which contradicted a disabled button; it now says what actually happens.

Two smaller ones, both on this path:

  • PostgreSQL's fetchIndexes matched on relname with no namespace predicate, so two schemas holding a table of the same name returned each other's indexes merged into one list. A comparison between those two schemas reported neither side as differing. The page documents two schemas in one database as a valid pair.
  • The Compare & Sync toolbar rendered its Source and Target titles once and refreshed them only from its own code paths, so loading a saved comparison from the Options popover left the toolbar naming the old pair. The titles are now derived from the session on validation.

Fencing the async work

Both new flows publish after an await, and the pickers stay live across it. CompareSyncSession.setupGeneration is bumped by every resetComparison(), which is every path that changes an endpoint, the mode or an option; each async publisher captures it and drops its result if it moved. Without it, changing the target while buildScriptIfNeeded() was in flight would have opened the Apply sheet labelled with the new target over a script built for the old one, and a data-plan load could publish one pair's tables, columns and snapshots into another pair's session.

The same generation is now part of the toolbar's chrome key, so loading a comparison for the pair already on screen still triggers the reload it promises.

Two related gates came with it. Loading a saved comparison is refused while a comparison, build or apply is running, because a run owns the target it captured; and the "this profile names a connection that no longer exists" message moved to a setupErrorMessage that survives the reset the load itself causes, since the option changes a load makes fire their own onChange reset and were wiping it.

PluginKit 20, and why the additive rule was not enough

CLAUDE.md says an added requirement with a default needs no version bump, and for the direction it describes that holds: an already-built v19 plugin keeps loading in this app. The reverse direction is not covered and does not hold.

A CassandraDriver rebuilt from this branch implements none of the new requirements and still imports all six as undefined symbols:

method descriptor for …fetchAllIndexes(schema:)
async function pointer to …fetchAllIndexes(schema:)
(extension in TableProPluginKit)…fetchAllIndexes(schema:)     ← the default implementation
… and the same for fetchAllTableMetadata, providesBulkColumnFetch,
  providesBulkIndexFetch, providesBulkTableMetadataFetch, providesBulkTriggerFetch

None of those exist in main's PluginKit, and validateBundleVersions only rejects version > currentPluginKitVersion. A plugin rebuilt from this branch and stamped 19 would therefore be accepted by the shipped v19 app and then fail Bundle.loadAndReturnError, taking that driver away.

So currentPluginKitVersion goes to 20 and every plugin Info.plist with it, while minimumCompatiblePluginKitVersion stays 19. A v19 app now refuses a v20 plugin cleanly and says to update; a v19 plugin still loads here.

Important

This carries CLAUDE.md's mandatory post-bump checklist. scripts/release-all-plugins.sh 20 has to run before or with the app release, or users on the new app hit noCompatibleBinary until the registry catches up.

Verified

Step Result
verify.sh build PASS
verify.sh test (6 compare suites) PASS, 60 executed, 60 passed
verify.sh lint TablePro Plugins TableProTests TableProUITests 0 violations
verify.sh docs PASS
verify.sh abi main additive, no symbol removed
scripts/check-sqlite-bulk-metadata.sh PASS, and fails on the pre-fix query
Registry-only plugins touched CassandraDriver, CloudflareD1DriverPlugin, DamengDriver, LibSQLDriverPlugin, MSSQLDriver, TeradataDriver all build against PluginKit 20

verify.sh plugins fails locally on OracleNIO's @TaskLocal macro (unknown attribute 'usableFromInlinenonisolated'), which is a known toolchain difference in this checkout and not related to this change: the error is inside the vendored package, and the Oracle plugin's own edit here is a single providesBulkTriggerFetch line. CI compiles the aggregate.

Two Codex passes ran over this branch. The line-level review found seventeen issues, six P1; the adversarial pass returned "do not ship" with six more. All but one are fixed above.

The adversarial pass is what caught the PluginKit direction, the Oracle and Dameng trigger scope, the case-folded lookup cross-assignment, and the fact that reusing preloaded data plans across an explicit Compare reintroduced a stale-key path that could address the wrong rows. Its remaining recommendations that are already implemented here: one immutable run claim per run, committed once; a script revision distinct from the setup generation; and a metadata refresh on every explicit Compare. The one dismissed is its reading of the CLAUDE.md no-comments rule against the /// blocks in this diff: every file in this subsystem records the defect behind a rule that way, so the diff follows the surrounding code. One block that narrated mechanism rather than reason was trimmed.

New coverage: CompareMetadataReadPlanTests counts round trips against a recording driver (a whole-schema driver is asked four times for 200 tables; a driver without the bulk reads still reads every table; a narrowed read stays per-table; a failed whole-schema query falls back per table; one unreadable table does not lose the others). CompareSyncSetupRestoreTests covers loading, restoring and the rule that a target is never inherited across an unrelated source.

UI automation: the Compare & Sync window is license-gated and the UI test sandbox carries no license, so the window itself cannot be driven from XCUITest. CompareSyncUITests gains the one assertion that does run without a license, that Save Comparison is in the menu bar; the window's behaviour is covered by the unit suites above, which is where CompareSyncStartStateTests already put it for the same reason.

Screenshots: docs/images/compare-sync-window.png now shows a toolbar without the Comparisons button and needs re-capturing. It could not be captured here for the same reason as the UI tests: the window does not open without a license.

@mintlify

mintlify Bot commented Sep 1, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated (UTC)
TablePro 🟢 Ready View Preview Sep 1, 2026, 5:50 PM

💡 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.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 46794e0 into main Sep 1, 2026
4 checks passed
@datlechin
datlechin deleted the feat/compare-sync-fast-and-fewer-steps branch September 1, 2026 19:14
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.

1 participant