move-tables: ensure target tables are dropped in noop mode - #1754
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures move-tables noop runs clean up temporary target tables used for schema validation.
Changes:
- Drops validation tables during noop cleanup.
- Adds an integration test covering target-table removal.
Show a summary per file
| File | Description |
|---|---|
go/logic/migrator.go |
Adds noop target-table cleanup. |
go/logic/migrator_test.go |
Tests cleanup of multiple target tables. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
ericyan
force-pushed
the
move-table-noop
branch
from
August 18, 2026 13:20
66ae9bd to
26e4439
Compare
jakubpliszka
approved these changes
Aug 18, 2026
ericyan
added
the
feature-move-tables
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR ensure the
--noopin move-table mode is genuinely side-effect free with respect to the target tables itcreates for validation.
In move-tables mode, a
--noop(dry-run) run creates the target tables purely to validate their schema — no data is moved and no cutover happens. However,moveTablesFinalCleanup()only performed cleanup inside the--ok-to-drop-tablebranch, which is scoped to source-side artifacts (the__delrollback handle and the checkpoint table). As a result, the schema-validation target tables were left behind on the target whenever--ok-to-drop-tablewas not set, leaking tables from what is supposed to be a side-effect-free dry run.script/cibuildreturns with no formatting errors, build errors or unit test errors.