search

yesodweb / persistent

Persistence interface for Haskell allowing multiple storage methods.
MIT License
467 stars 296 forks source link

RFC: Should MigrationOnly indicate nullability? #918

Open parsonsmatt opened 5 years ago

parsonsmatt commented 5 years ago

Currently, using MigrationOnly on a column will omit the field from the Haskell datatype but the resulting SQL is still intact.

This means if you do an insert on a record with a non-nullable (and no default) MigrationOnly column that it will fail at runtime with a "missing value" error.

You can fix this by doing field FieldType Maybe MigrationOnly. Should MigrationOnly imply Maybe?

hdgarrood commented 5 years ago

Sounds sensible to me; it's hard to think of a case where the current behaviour is desirable.

Vlix commented 5 years ago

I guess if the Haskell program using persistent is only an observer of the table and just doesn't want to observe ALL columns, it can use MigrationOnly to shrink the amount of fields in the resulting data type, but this isn't really why you'd do migrations with persistent IMHO.

I'd say it makes a lot of sense that anything marked as MigrationOnly should imply Maybe. So I'd second this change.

Vlix commented 4 years ago

Has this been changed already, or is this still a to-do?

parsonsmatt commented 4 years ago

This has not been done yet, as far as I know.

hdgarrood commented 3 years ago

On second thoughts, I think it is possible/reasonable to genuinely want a NOT NULL field which Persistent isn't aware of, and the insertion issue can be addressed with a default, right? If I write myField Int MigrationOnly default='0' then I think I will still be able to do everything I need to do from Persistent as if that field didn't exist. So perhaps we should leave things as they are and instead emit a warning if all of the following are true? (Can we do that?)

parsonsmatt commented 3 years ago

@hdgarrood Yeah that sounds like the best solution to me. A column being NULL or Maybe really is just an implied default=NULL.