Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable foreign key checks when preparing existing schema for preflight schema change tests #4696

Merged
merged 1 commit into from
May 27, 2020

Conversation

dweitzman
Copy link
Member

@dweitzman dweitzman commented Mar 2, 2019

The preflight check isn't clever enough to create the existing schema in a foreign-key-friendly order, so we temporarily disable foreign key checks while creating the existing tables.

@dweitzman dweitzman requested a review from sougou as a code owner March 2, 2019 02:56
@dweitzman dweitzman changed the title Disable foreign key checks during preflight schema change tests Disable foreign key checks when preparing existing schema for preflight schema change tests Mar 2, 2019
@dweitzman
Copy link
Member Author

It's probably best not to merge this in it's current state: ideally it would respect the existing foreign_key_checks value rather than assuming it was on, in case someone wants to apply a schema change with foreign key checks already off globally.

@dweitzman
Copy link
Member Author

Changed, but not yet tested

@dweitzman
Copy link
Member Author

I removed unnecessarily cleverness from earlier versions of this PR, so it should be equivalent to the version we use in the Pinterest codebase.

Since the foreign_key_checks variable is session-scoped the new DDL runs in a new session it still be subject to foreign key checks if they're enabled.

@morgo
Copy link
Contributor

morgo commented Feb 6, 2020

@dweitzman this PR still makes sense to me. Can you rebase on master, and lets try and merge it?

The preflight schema check copies the entire existing schema into a temporary
database and then tests the user-provided DDL.

During that copy we need to disable foreign key checks because we're not yet
clever enough to copy the tables in an order that's compatible with foreign
key checks being on.

Foreign key checks are still applicable to the user-provided DDL

Signed-off-by: David Weitzman <dweitzman@pinterest.com>
@dweitzman
Copy link
Member Author

Rebased

@morgo morgo self-requested a review May 27, 2020 00:30
Copy link
Contributor

@morgo morgo left a comment

Choose a reason for hiding this comment

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

LGTM

@morgo morgo merged commit 33b03b7 into vitessio:master May 27, 2020
@deepthi deepthi added this to the v7.0 milestone Jul 27, 2020
systay pushed a commit that referenced this pull request Jul 22, 2024
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