fix(me): fields whose type is a list of enums with @default([]) are now migrated without crashes on Postgres

[jkomyno]

Closes prisma/prisma#15705.

[tomhoule]

I think the fix can be better expressed as a single line (a .filter() after line 1003). I'd rather we never produce ALTER TABLE ... ALTER COLUMN ... without the last part of the clause. It's most likely a no-op.

[jkomyno]

Do you mean something like:

    default_str_option.filter(|default_str| {
        let set_default = format!(
            "ALTER TABLE {table_name} ALTER COLUMN {column_name} SET DEFAULT '{default}'",
            table_name = Quoted::postgres_ident(&table_name),
            column_name = Quoted::postgres_ident(&column_name),
            default = escape_string_literal(default_str),
        );
        stmts.push(set_default)
    });

Is it idiomatic in Rust to have mutable operations inside functional methods like .filter() and .map()?

[jkomyno]

We can also express the same logic with:

    if let Some(default_str) = default_str_option {
        let set_default = format!(
            "ALTER TABLE {table_name} ALTER COLUMN {column_name} SET DEFAULT '{default}'",
            table_name = Quoted::postgres_ident(&table_name),
            column_name = Quoted::postgres_ident(&column_name),
            default = escape_string_literal(default_str),
        );
        stmts.push(set_default)
    }

[tomhoule]

No usually we avoid mutation inside iterator methods.

I meant at line 1003, insert one more call .filter(|(_, next)| next.default.is_some()), and that would be the only change needed in this file. If you want to also remove the unwrap in the loop body, you can .filter_map(|(previous, next)| next.default.map(|d| ((previous, (next, d)))).

[tomhoule]

The if let would also work, but it's a larger diff and it adds more noise.

[tomhoule]

Could we test the following scenario:

• Single model with a field with mood Mood @default(HUNGRY)
• Migration to drop one variant from the enum and the default on the field at the same time.

I think it's a more minimal reproduction of the conditions for the crash.

That test can live in tests/migrations/enums.rs

[jkomyno]

Isn't this scenario already tested by alter_enum_name_remove_and_change_default? I could move it to tests/migrations/enums.rs.

[tomhoule]

yep we could move it there. It's a different test however, alter_enum_name_remove_and_change_default still has a default, just a different one. We should test what happens when the whole @default() is removed in the second schema.

[jkomyno]

I have moved the tests to tests/migrations/enums.rs and left a few TODOs.

[jkomyno]

Closing this PR in favor of #3379.