persistent-mysql: truncate foreign key constraint names to MySQL's 64 char limit - #1633
Open
Moferanoluwa wants to merge 1 commit into
Open
Conversation
… char limit persistent-mysql currently suggests foreign key constraint names using the shared default naming scheme (tableName_columnName_fkey) with no length limit applied. Unlike Postgres, which silently truncates over-length identifiers (which persistent-postgresql already works around via refName in Postgresql.hs, added in yesodweb#996), MySQL raises an error instead: "Identifier name ... is too long". Add an equivalent refName override for MySQL using the same truncation algorithm persistent-postgresql already uses, just with MySQL's 64 character limit instead of Postgres' 63, and wire it in via setBackendSpecificForeignKeyName. persistent-test/src/LongIdentifierTest.hs and the commented-out / xdescribe'd hookup in persistent-mysql/test/main.hs were already in place, explicitly waiting on this (main.hs even links directly to issue yesodweb#1000) - enabled both. Bumped persistent-mysql's version per CONTRIBUTING.md (patch: new bug fix). Will follow up with the Changelog.md entry once I have a PR number to link, per the documented changelog process. Fixes yesodweb#1000
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1000.
persistent-mysqlcurrently generates foreign key constraint names using the shared default scheme (tableName_columnName_fkey) with no length limit. Unlike Postgres, which silently truncates over-length identifiers (worked around inpersistent-postgresqlviarefName, added in #996), MySQL just raises an error for identifiers over 64 characters.This adds the MySQL equivalent of that same
refNameoverride, reusing the exact same truncation algorithmpersistent-postgresqlalready uses (approximating how the database itself truncates), just with MySQL's 64-character limit instead of Postgres' 63, wired in via the existingsetBackendSpecificForeignKeyNameextension point.Evidence this is the intended fix
persistent-test/src/LongIdentifierTest.hsand its hookup inpersistent-mysql/test/main.hswere already sitting there, explicitly disabled and waiting on exactly this:I enabled that test (both the
xdescribewrapper and the commented-out migration registration a few lines up) rather than leaving it disabled.Changes
persistent-mysql/Database/Persist/MySQL.hs— addrefName, mirroringpersistent-postgresql's function of the same name/signature almost exactly (maximumIdentifierLength = 64instead of63), and wire it intomysqlMkColumnsviasetBackendSpecificForeignKeyName.persistent-mysql/test/main.hs— enableLongIdentifierTest, which was previously disabled specifically because this was missing.persistent-test/src/LongIdentifierTest.hs— update the comment that said MySQL didn't run this test yet.persistent-mysql/persistent-mysql.cabal— bump patch version (2.13.1.6 → 2.13.1.7) per CONTRIBUTING.md's versioning guide (bug fix = patch bump).Per CONTRIBUTING.md, I'll follow up with the
Changelog.mdentry once I have a PR number to link.Testing/Review Recommendations
I don't have a Haskell toolchain (no ghc/stack/cabal) available in this environment, so I was not able to compile or run this locally — please treat that as a real gap, not just a formality, and lean on CI here. What gives me confidence despite that:
refName's type signature and every symbol it uses (EntityNameDB,FieldNameDB,ConstraintNameDB,setBackendSpecificForeignKeyName,emptyBackendSpecificOverrides) come fromDatabase.Persist.Sql, whichMySQL.hsalready imports unqualified — same import the Postgres file uses for the same symbols.shortenNamesrecursion by hand rather than just assuming it carries over.LongIdentifierTestis exactly the test the maintainers left in place for this fix (it references issue MySQL backend generates foreign keys over length limit #1000 by number in its own skip message), so CI running it is a real, existing check of correctness, not something I added ad hoc.AI Usage
refNameimplementation and the pre-wired-but-disabledLongIdentifierTest, porting the former to MySQL, and enabling the latter.