Skip to content

Log or raise on unexpected duplicated entry warnings during INSERT IGNORE#100

Merged
tiwilliam merged 8 commits into
masterfrom
raise-non-pk-dups
Jul 19, 2021
Merged

Log or raise on unexpected duplicated entry warnings during INSERT IGNORE#100
tiwilliam merged 8 commits into
masterfrom
raise-non-pk-dups

Conversation

@tiwilliam

@tiwilliam tiwilliam commented Jun 16, 2021

Copy link
Copy Markdown

Closes https://github.com/Shopify/shopify/issues/302218

Today when migrating a schema that ends up violating a UNIQUE constraint, the records to the new shadow table is dropped silently. This may cause huge silent data loss when, for example, a column is dropped that is part of a UNIQUE index, without modifying or dropping the index. Same problem can occur if we change collation, causing a unique index to ignore case and drop rows silently.

This PR attempts to make these drops halt the migration and alert the operator that this is happening, making it possible to resolve the issue before migrating again. This is done by inspecting any warnings in-case we can see that the number of inserted rows were lower than we expected, signalling ignored rows. These warnings are in most cases fine, since PK duplicates may occur by design, but other constraints should not.

@tiwilliam tiwilliam changed the title Abort migration on duplicate non PRIMARY KEY Abort migration on unexpected warnings during INSERT Jun 16, 2021
@tiwilliam tiwilliam requested review from shuhaowu and xliang6 June 22, 2021 11:45
@tiwilliam tiwilliam force-pushed the raise-non-pk-dups branch from a09a2a4 to 371bbef Compare June 23, 2021 17:13
Comment thread lib/lhm/chunker.rb

@xliang6 xliang6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I would change the PR title to Log or raise on unexpected duplicated entry warnings during INSERT IGNORE to be more clear.

@tiwilliam tiwilliam changed the title Abort migration on unexpected warnings during INSERT Log or raise on unexpected duplicated entry warnings during INSERT IGNORE Jul 19, 2021
@tiwilliam tiwilliam merged commit 285b22a into master Jul 19, 2021
@tiwilliam tiwilliam deleted the raise-non-pk-dups branch July 19, 2021 13:00
@shopify-shipit shopify-shipit Bot temporarily deployed to production September 23, 2021 20:57 Inactive
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.

3 participants