Issue: migration: Default value is set to non-unique value with add_column even if field needs uniqueness
Author: ccosby
State: closed
Created: 2018-04-12 14:42:14

https://github.com/coleifer/peewee/blob/449450d738a8c8f2893e5309d03d03bc249e9d7e/playhouse/migrate.py#L273

This is the migrate() equivalent of https://github.com/coleifer/peewee/issues/429

When using playhouse.migrate.SchemaMigrator.add_column() with field.default set to a callable and field.unique=True, more work has to be done to create a unique value for every row. This will be slow for big tables, but is necessary for UUIDs and things of that ilk.

If you think there's enough need for this, I'll spend some time on it. If not, just close this issue with a note stating so.



Interesting... so what you're running into is -- you'd like to add a new, non-nullable, unique field with a dynamic default. The migrator's `apply_default()` helper is implemented such that, if the field's default is callable, it is only called once:

```python
    def apply_default(self, table, column_name, field):
        default = field.default
        if callable(default):
            default = default()

        return (self.make_context()
                .literal('UPDATE ')
                .sql(Entity(table))
                .literal(' SET ')
                .sql(Expression(
                    Entity(column_name),
                    OP.EQ,
                    field.db_value(default),
                    flat=True)))
```

I'm leery of changing this to run a separate UPDATE for every row that may already exist in the table.

I'd propose instead that you do something like this:

1. use migrator to add the field (non-unique and nullable)
2. in a transaction/loop update the existing rows
3. use migrator to make field not-null and add a unique index on it.

WDYT?

Your suggestion is in fact what I did. LMK if you ever want to address it in peewee official.

```python
migrator = MySQLMigrator(db)

class StrUUIDField(peewee.CharField):
    def db_value(self, value):
        if isinstance(value, uuid.UUID):
            return value.hex
        try:
            return uuid.UUID(value).hex
        except (TypeError, ValueError):
            return value

    def python_value(self, value):
        if isinstance(value, uuid.UUID):
            return value.hex
        return None if value is None else uuid.UUID(value).hex
        
def add_unique_column(model, field_name='uuid', field_default=uuid.uuid4):
    """Add uuid field to a given model type.
    This is here because calling add_column calls apply_default, resulting in every row
    getting the same initial value. Not good when you want a unique value.
    """

    # First add the column with null as the default value. No indexes.
    field_definition = StrUUIDField(null=True, index=False, unique=False)
    migrate(
        migrator.add_column(model._meta.table_name, field_name, field_definition),
    )

    # Update every row with a unique uuid
    for inst in model.select(model.id):
        setattr(inst, field_name, field_default() if callable(field_default) else field_default)
        inst.save()

    # Now set the column to NOT NULL and create the unique index
    migrate(
        migrator.add_not_null(model._meta.table_name, field_name),
        migrator.add_index(model._meta.table_name, (field_name,), unique=True)
    )
```

I think I'll pass but will keep this in mind should it come up again. Thanks for sharing your implementation.

For what it's worth, I too ran into this and was confused by the behavior.

If the migrator's `apply_default()` determines that the default is callable (i.e., dynamic), isn't caching the value breaking the expectation of the user?

I understand the reason for passing on this implementation, but it would be helpful to throw a warning or error explaining this limitation (or even pointing them to this issue!)