keredson / peewee-db-evolve

Automatic migrations for the Peewee ORM.
GNU Lesser General Public License v3.0
126 stars 26 forks source link

Evolve setting existing values to new default value. #38

Closed dsmurrell closed 5 years ago

dsmurrell commented 5 years ago

I have a table 'Surveys' which has fields 'title' and 'description'.

The surveys all had existing titles and descriptions and were also null=True.

When removing the null=True and replacing it with default="", evolve replaces all the existing values with "".

So for:

title = CharField(45, null=True)
description = CharField(500, null=True)

changing to:

title = CharField(45, default="")
description = CharField(500, default="")

evolve does:

UPDATE "surveys" SET "description" = %s; ['']
ALTER TABLE "surveys" ALTER COLUMN "description" SET NOT NULL;
UPDATE "surveys" SET "title" = %s; ['']
ALTER TABLE "surveys" ALTER COLUMN "title" SET NOT NULL;

Is this expected behaviour?

How can I get it to not replace the existing values?

dsmurrell commented 5 years ago

Ok, I seem to have found a workaround. That is to drop the null=True first, then evolve, then add the default="" for which the evolve doesn't seem to do anything.

This does mean that we need to migrate to prod twice being careful not to nuke data :)

keredson commented 5 years ago

well that is horrifying. sorry about that. there should definitely be a WHERE "title" IS NULL on the UPDATE statement.

keredson commented 5 years ago

found the issue. affects peewee3 only.

in peewee's schema migration library (which we use in places under peewee3) the migration function apply_default() applies the default for all values, not just where they're currently null. that seems wrong, but maybe i'm not understanding the intent.

should have a patch out soon to work around.

thanks for the bug report!

keredson commented 5 years ago

peewee-db-evolve-3.7.3 released

dsmurrell commented 5 years ago

That was fast, thank you!

And thanks for the extremely useful peewee-db-evolve!

willgdjones commented 5 years ago

Seconded @keredson !