Skip to content

Conversation

@charukiewicz
Copy link
Contributor

This change adds support for a constraint= attribute that allows users to override the automatically generated foreign reference constraint names with their own. The motivation for this change is that we had two instances of very long table/column names that led to automatically generated constraint names exceeding 64 characters, which is not allowed in MySQL. Without a change like this we could not use the printMigrations output.

Please let myself (@charukiewicz) or @belevy know if you have suggestions for a better solution or alternative approach.

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

Code looks great! Version bump sounds reasonable.

Could this break anything? You're only adding a bit of functionality. Old versions of persistent will ignore the constraint=. It will generate a migration to rename a constraint if the constraint= is used, I think, but that's expected.

I don't think it can break anything on further thought. I'll merge this in later today. Thanks!

AnotherVeryLongTableName
veryLongTableNameId VeryLongTableNameId constraint=short_foreign_key
```

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for extending the docs!

pure (DBName x, constraintName)
| Just x <- T.stripPrefix "constraint=" a = do
tableName <- fst <$> (ref c fe as)
pure (tableName, DBName x)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see how this is implemented now! It doesn't require a breaking change addition to a datatype, which is great. It adds a new feature to the QuasiQuoter syntax, but doesn't change the persistent-template library at all.

So if you have a QuasiQuoter block with constraint=fk_name on an old version of persistent, then it'll get ignored. That's okay.

_ -> []
refAdd = case (ref == ref', ref) of
(False, Just (tname, _cname)) -> [(tname, addReference allDefs (refName tblName name) tname name)]
(False, Just (tname, cname)) -> [(tname, addReference allDefs cname tname name)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks great to me. I think I would like to see it included in the other SQL backends, but that can come in a later PR.

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