🐛 Added migration to update empty strings to null #10428
Conversation
|
I've tested this locally but figured it could use some more eyes. @gargol What do you think of this? |
core/server/data/migrations/versions/2.13/1-remove-empty-strings.js
Outdated
Show resolved
Hide resolved
|
Changes make sense. Will also give it a spin on my local instance 👍 |
| }); | ||
| }; | ||
|
|
||
| module.exports.up = ({connection}) => { |
There was a problem hiding this comment.
DML migrations should use a transaction.
https://github.com/TryGhost/knex-migrator#transactions
| * */ | ||
| const tablesToUpdate = Object.keys(schema.tables).reduce((tablesToUpdate, tableName) => { | ||
| const table = schema.tables[tableName]; | ||
| const columns = Object.keys(table).filter(columnName => table[columnName].nullable); |
There was a problem hiding this comment.
To make this a bit more explicit, maybe the type fo string or text should be checked here as well?
There was a problem hiding this comment.
I was thinking this too - though we only update if the existing value is '' so we shouldn't pick up any fields like that. Maybe it's better to be explicit tho 👍
|
@allouis running locally using at: how are you testing it on your setup? |
|
^ This broke since I pushed the transaction stuff - fixing now |
|
@gargol fixed! I'm testing with |
| }; | ||
|
|
||
| module.exports.down = ({connection}) => { | ||
| const replaceNullWithEmptyString = createReplace(connection, null, ''); |
There was a problem hiding this comment.
@kirrg001 I found something interesting, which is that setting the {transaction: true} config on the exports, passed a transacting object to up, but still a connection object to down
There was a problem hiding this comment.
|
@allouis migrations work fine here 👍 merge at will 🚀 |
refs TryGhost#10388, original PR TryGhost#10428 - re-introduces the migration which normalizes all empty strings in the database to `NULL` - this was previously reverted due to being too large of a migration for a minor but a major is a good time to do it
closes #10388
This migration finds all tables with nullable columns, it then loops through the tables and their nullable columns, updating each column to a null when its current value is an empty string.