Skip to content

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

Merged
morgo merged 1 commit intovitessio:masterfrom
dweitzman:preflight_fk
May 27, 2020
Merged

Disable foreign key checks when preparing existing schema for preflight schema change tests#4696
morgo merged 1 commit intovitessio:masterfrom
dweitzman:preflight_fk

Conversation

@dweitzman
Copy link
Copy Markdown
Collaborator

@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
Copy Markdown
Collaborator 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
Copy Markdown
Collaborator Author

Changed, but not yet tested

@dweitzman dweitzman force-pushed the preflight_fk branch 2 times, most recently from 03bc644 to e5d8794 Compare March 2, 2019 08:39
@dweitzman dweitzman force-pushed the preflight_fk branch 2 times, most recently from 0f4b570 to 71dd981 Compare March 13, 2019 22:01
@dweitzman
Copy link
Copy Markdown
Collaborator 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
Copy Markdown
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
Copy Markdown
Collaborator Author

Rebased

@morgo morgo self-requested a review May 27, 2020 00:30
Copy link
Copy Markdown
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
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