feat(compare): compare and sync structure or data between two connections (#721) - #1968
feat(compare): compare and sync structure or data between two connections (#721)#1968datlechin wants to merge 3 commits into
Conversation
datlechin
added
the
abi-additive
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20bfb6e978
ℹ️ 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".
| let required: PluginCapabilities = mode == .structure ? .schemaCompare : .dataCompare | ||
| guard !driver.capabilities.contains(required) else { return nil } |
There was a problem hiding this comment.
This eligibility check rejects every real database driver because the commit defines the two capability bits but never adds either bit to any PluginDatabaseDriver.capabilities implementation. I checked all driver capability declarations under Plugins; consequently both structure and data comparisons always stop with the unsupported message before doing any work.
Useful? React with 👍 / 👎.
| let result = try await self.executorRun( | ||
| statements: self.statements, | ||
| target: target, |
There was a problem hiding this comment.
When the user changes the editable script preview, the binding only updates editedScript, but Apply still passes the original self.statements here. Thus edits—including removing or correcting a destructive statement—are ignored while the confirmation implies the displayed script will run; either rebuild executable statements from the edited text or make the preview read-only.
Useful? React with 👍 / 👎.
| guard entries.count < limit else { | ||
| truncated = true | ||
| return | ||
| } | ||
| entries.append(entry) |
There was a problem hiding this comment.
For tables exceeding maxRetainedEntries (5,000 by default), this cap discards every later entry even though generateDataScript() builds SQL exclusively from summary.entries. A table whose first 5,000 rows match but whose 5,001st row differs reports the exact difference count yet generates no statement; other large tables are only partially synchronized. Retention used for display must not truncate the data used to build the sync script.
Useful? React with 👍 / 👎.
| if let leftNumber = Double(leftKey), let rightNumber = Double(rightKey) { | ||
| return leftNumber < rightNumber ? .orderedAscending : .orderedDescending |
There was a problem hiding this comment.
When key strings are large exact integers, converting them to Double collapses distinct values—for example, 9007199254740992 and 9007199254740993. The merge join can then choose the wrong stream to advance, misclassify a row present on both sides as an insert, and generate duplicate-key or otherwise incorrect synchronization SQL. Numeric ordering needs an exact decimal/integer comparison consistent with the database ordering.
Useful? React with 👍 / 👎.
| if shouldRollback { | ||
| try? await driver.rollbackTransaction() |
There was a problem hiding this comment.
If rollbackTransaction() fails after cancellation or a statement error, try? silently discards that failure and the returned result still sets rolledBack to true. The apply view then explicitly tells the user that the target is unchanged even though the transaction may remain open or partially applied; propagate the rollback error or record rollback failure separately instead of claiming success.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
1 participant
Closes #721.
Adds Compare & Sync: pick a source and a target connection, compare structure or data, and generate the SQL script that brings the target in line with the source. Only three Mac clients ship this at all (Navicat, Querious for MySQL only, Valentina Pro), and it is the most-cited reason people keep a Navicat licence alongside their preferred Mac client. Sequel Pro's request has been open since 2008; its maintainer said the blocker was the UI, not the SQL generation.
Requires a Starter licence (
ProFeature.compareSync).Where it lives
A separate auxiliary window rather than a new tab kind. The HIG calls this shape an auxiliary window and rules sheets out for "complex or prolonged user flows". The codebase agrees:
MainContentCoordinator.connectionis a storedlet,EditorTabPayloadcarries oneconnectionId, andTabPersistenceCoordinatorwrites one file per connection, so a second connection cannot be threaded through the tab model without touching every one of those.DatabaseManager.activeSessionsis already keyed by connection, so the data layer needed no changes at all.The payoff:
MainEditorContentView.tabContent,WindowTitleResolver, andresolveDetailMinimumThicknessgain zero cases.Opened from File > Compare & Sync Databases... or a connection's context menu on the welcome screen.
Structure compare
Compares parsed metadata, never DDL text. Every text-diffing tool in this space has shipped either a permanent-diff loop or a false-positive storm; Redgate once reported 1004 objects as different over a trailing semicolon, and MySQL Workbench closed its version as Not a Bug.
SchemaStatementGeneratoris reused completely unchanged. The newSchemaSyncScriptBuilderadds only the table-level create/drop it deliberately does not model, plus cross-table ordering.Data compare
Key-ordered streaming merge join. Neither side is materialised, so there is no equivalent of Redgate's documented 2-4x disk budget, and no server-side hash function has to agree between two engines.
updated_atfrom matching and it is still written on insert or update. Editing keys or excluded columns clears that table's result and offers Compare Again.Safety
Reuses the app's existing machinery rather than inventing a parallel path.
SafeModeLevel.readOnlyis structurally ineligible as the target, disabled in the picker with the reason, not warned about at apply time.ExecutionGate.authorizingcall per run, following thedispatchStatementsprecedent..confirmationPreClearedsuppresses the generic dialog because the compare window already showed a typed one, while biometric auth forsafeModetargets still fires. The statement loop stays inside that call so the task-local receipt holds for every statement.supportsTransactionalDDL, not assumed. On MySQL and Oracle the window says so rather than implying a rollback you will not get. Continue-on-error disables the transaction option, mirroringRowImportRunner's existing rule.PluginKit: additive, no version bump
Two new
PluginCapabilitiesbits and one extractedForeignKeyTopologicalSort(shared withSQLExportPlugininstead of duplicated).scripts/check-pluginkit-abi.shconfirms the diff is additions only, with no symbol removed or changed, so nocurrentPluginKitVersionbump and norelease-all-plugins.shrun. Please add theabi-additivelabel.Default
capabilities: []means no existing plugin becomes eligible until it opts in.Structure differences also show source and target definitions side by side, split or unified, rendered from parsed metadata through one function so a formatting difference cannot appear as a difference. Comparison setups can be saved by name against a source, target, and mode.
Tests
89 new tests, all passing, all against fake drivers so none are gated behind a C bridge. They pin the failure modes the field actually shipped: auto-increment drift is not a difference, a name-only index difference emits no DDL, primary key membership is reported once rather than twice, float tolerance is symmetric, equivalent instants at different offsets compare equal, comparing without a key throws rather than guessing, and a refused statement never reaches the driver.
Reviewer notes, and what is not verified
SQLExportPlugin.topologicallySortnow delegates to the extracted function and has no test coverage, before or after. Worth exercising a real SQL export of a database with foreign keys. One behaviour did change deliberately: the original usedDictionary(uniqueKeysWithValues:), which crashes on two tables sharing a name across schemas; it now keeps the first.