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

Add --drop_constraints to MoveTables #9904

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Add --drop_constraints to MoveTables #9904

merged 1 commit into from
Mar 18, 2022

Conversation

Phanatic
Copy link

Description

This PR adds a drop_constraints switch to MoveTables which will allow us to automatically strip constraints on tables in the target keyspace when importing a database into Vitess.

Setting this to true causes us to use the createDDLAsCopyDropConstraint create DDL mode, which will strip all constraints from the table before creating it in Vitess.
https://github.com/vitessio/vitess/blob/6fce6c66899dbd593e5d4e80ce7d2ff95046e13f/go/vt/wrangler/materializer.go#L1034-L1041

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Signed-off-by: Phani Raj <phani@planetscale.com>
@rohit-nayak-ps rohit-nayak-ps added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VReplication release notes labels Mar 17, 2022
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm

@rohit-nayak-ps rohit-nayak-ps merged commit 6e5572b into vitessio:main Mar 18, 2022
@mattlord
Copy link
Contributor

mattlord commented Mar 18, 2022

Sorry, I was a bit late here... if it's not too late we should change this to be more specific. There are currently two types of constraints:

  1. Foreign Key Constraints
  2. Check Constraints (custom type / input validation on write)

I don't think we'd ever want to strip the second and this has some potential for confusion.

@rohit-nayak-ps
Copy link
Contributor

Sorry, I was a bit late here... if it's not too late we should change this to be more specific. There are currently two types of constraints:

This underlying feature was added in #6422, only to Materialize, where the primary aim was to drop foreign key constraints, but it does just drop all constraints. This PR is just exposing that same feature in MoveTables.

I don't think we'd ever want to strip the second and this has some potential for confusion.

Since Check Constraints was properly implemented in a recent version of MySQL (at the time of original PR), it may not have been considered.

I agree we will probably never need to drop check constraints in a workflow, since it is essentially a row level validation scoped to the same table.

@Phanatic, does this sound good: dropping constraints will be scoped only to foreign key constraints? If you like I can work on a follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants