fix(db): remove hard varchar length limits and stop migrations importing the umzug instance#222
Open
markcrivera wants to merge 3 commits into
Open
fix(db): remove hard varchar length limits and stop migrations importing the umzug instance#222markcrivera wants to merge 3 commits into
markcrivera wants to merge 3 commits into
Conversation
Rewrite every DataTypes.STRING / STRING(n) / STRING(DATETIME_LENGTH) in
migrations and models to DataTypes.TEXT, convert SQL VARCHAR(n) in the
SQLite table rebuild blocks, and fix a parenless VARCHAR${DATETIME_LENGTH}
typo that produced the type name VARCHAR29. Remove the unused
DATETIME_LENGTH constant from db/umzug.js and server/utils/constants.ts,
drop it from 39 migration import lists, and update the sample migration
template.
Refs #12
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
… to TEXT Postgres-only: SQLite never enforced varchar lengths and its table rebuild path risks constraint loss. On fresh databases built from the rewritten migration chain the query matches nothing and the migration is a no-op. Closes #12 Signed-off-by: Mark Rivera <mcrivera@gmail.com>
…ng the instance
Remove the runtime import of ../umzug.js from all 55 migrations and take
the Sequelize instance from the { context } argument the runner has always
passed like the seeders use. Stops migration files from loading a second
copy of umzug.js from disk with its own connection pool, dotenv call, and
hooks. the dev CLI is unaffected. Should clean up dotenv dev issues.
Signed-off-by: Mark Rivera <mcrivera@gmail.com>
|
Collaborator
Author
Can't do anything about the duplicated code on database migrations. @Amndeep7, @aaronlippold can we get relief on this here? |
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
2 participants
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.
Summary
Removes every hard text-length limit from the database schema (#12), in the models, throughout the historical migration chain, and on existing deployments via a new idempotent normalizing migration. Converts all migrations to consume the Sequelize instance from umzug's context argument instead of importing it from db/umzug.js.
Changes
refactor(db): replace all varchar column types with TEXT
fix(db): add idempotent migration converting existing varchar columns to TEXT
refactor(db): migrations consume the umzug context instead of importing the instance
Testing
Related
Closes #12