-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
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. |
Changed, but not yet tested |
03bc644
to
e5d8794
Compare
0f4b570
to
71dd981
Compare
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. |
@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>
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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.