-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
match mysql fk name generation #7711
Conversation
@coffeegoddd DOLT
|
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.
Looks good!
It's a bummer to lose the ability for two branches to both add an FK constraint or secondary index to the same table and merge them back cleanly – that is a cool trick. I think being MySQL compatible ultimately wins out here though, especially since we are giving customers a new statement to easily rename a constraint, and since customers can explicitly specify constraint names to get around the merge conflict issue.
We may eventually want to support both modes, but that's adding complexity for users to deal with, so I'd hold off on that until we know it's definitely a customer problem we need to work on.
Tagging @zachmu on this one, since he may have an opinion on the tradeoff between fully matching MySQL's behavior and keeping the deterministic fk / secondary index naming. Since the customer isn't blocked on this currently, let's give Zach a day to share an opinion, just in case he sees another problem this could cause that we aren't seeing.
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, see comments on GMS change
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.
But write tests of merge logic (can skip for now)
If two sides of the merge declare the same fk name with a different definition, that should be a schema conflict.
@coffeegoddd DOLT
|
@coffeegoddd DOLT
|
@coffeegoddd DOLT
|
This PR removes logic where we would use a consistent hash to create foreign key names, when one isn't explicitly provided.
Instead, we use logic in GMS to generate a name with the template
"<tbl_name>_ibfk_<num>"
, which matches MySQL behavior.As a result, we now allow multiple foreign keys over the same sets of columns (as long as the names are different).
companion pr:
dolthub/go-mysql-server#2438
fixes:
#7650