Fix redundant change stream SET FOR on FOR scope transitions#82
Merged
daichirata merged 1 commit intomasterfrom Feb 15, 2026
Merged
Fix redundant change stream SET FOR on FOR scope transitions#82daichirata merged 1 commit intomasterfrom
daichirata merged 1 commit intomasterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where redundant ALTER CHANGE STREAM ... SET FOR ... statements could be emitted during table drop processing when a change stream transitions from table scope (FOR t1, t2) to non-table scope (FOR ALL or FOR NONE). The generator was incorrectly emitting an intermediate SET FOR <remaining tables> followed by the final scope change, resulting in incorrect final DDL state.
Changes:
- Fixed change stream alter emission logic to only emit intermediate
SET FOR <remaining tables>when the final target scope isFOR TABLES - Updated Japanese comments to English for better maintainability
- Added regression tests for multi-table to
FOR ALLand multi-table toFOR NONEtransitions with table drops
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/hammer/diff.go | Added condition to check target change stream type before emitting intermediate ALTER, preventing redundant statements when transitioning to non-table scopes; updated comments from Japanese to English |
| internal/hammer/diff_test.go | Added two regression tests covering the bug scenarios: multi-table to FOR ALL with table drop, and multi-table to FOR NONE with table drop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
Fix an issue where redundant
ALTER CHANGE STREAM ... SET FOR ...could be emitted during table drop processing, resulting in final DDL that does not match the target schema.Background
When a change stream transitions from table scope to non-table scope (
FOR ALLor noFOR), and one of watched tables is dropped:FOR t1, t2FOR ALL(orFOR NONE) +t1droppedthe generator could output:
ALTER ... SET FOR ALL(orDROP FOR ALL)ALTER ... SET FOR t2(unexpected)This made the final state incorrect (
FOR t2instead of target scope).Changes
internal/hammer/diff.go:SET FOR <remaining tables>only when the final target scope isFOR TABLES.FOR ALL/FOR NONE.internal/hammer/diff_test.go:FOR t1,t2 -> FOR ALLwitht1dropFOR t1,t2 -> FOR NONEwitht1drop