-
Notifications
You must be signed in to change notification settings - Fork 301
Description
A footgun I (and many others) often stumble across is when you create a unique key with a nullable column:
NullableUnique
fieldA Text
fieldB Text Maybe
UniqueNullable fieldA fieldB !force
If you use this unique key with a Just value in fieldB, everything will work as expected. However, if you have values with a Nothing, certain behaviors start to degrade, especially when you start to bring in other database engine behaviors like NULLS NOT DISTINCT in postgresql.
For instance, insertBy on a record with a Nothing unique key may, often unexpectedly, continue to insert new records that seem to match. If you have enabled NULLS NOT DISTINCT on your uniqueness constraint (or the equivalent syntax in other engines), you will always run into SQL exceptions violating the constraint at this point.
This behavior makes some sense today. When null values are present and treated as distinct values, a getBy could return multiple matching rows. Given the type signature of the function, getBy :: (PersistRecordBackend record SqlBackend, Typeable record, MonadSqlQuery m) => Unique record -> m (Maybe (Entity record)), you don't have the option to return multiple matching records for that unique key, and it returns Nothing instead.
When you don't provide !force on this unique key, persistent does try to warn you already:
persistent-test > Exception when trying to run compile-time code:
persistent-test > Error: By default Persistent disallows NULLables in an uniqueness constraint. The semantics of how NULL interacts with those constraints is non-trivial: most SQL implementations will not consider two NULL values to be equal for the purposes of an uniqueness constraint, allowing insertion of more than one row with a NULL value for the column in question. If you understand this feature of SQL and still intend to add a uniqueness constraint here, *** Use a "!force" attribute on the end of the line that defines your uniqueness constraint in order to disable this check. ***
But I think many users will simply add the !force and move on without realizing exactly how this odd behavior will materialize later.
This is often surprising, and I think it can be improved in a few ways.
-
Update the default error message (when missing !force) to specifically say that because the behavior is undefined otherwise, getBy on any unique key with a Nothing field will return no values.
-
Add a new syntax to persistentmodels for NullsNotDistinct. This should generate a unique constraint with the correct syntax in migrations.
-
Whether that syntax is present or not, decide how to handle Nothing values in Unique keys (e.g. allow users to select old or new behavior)
-
Update the !force message to point to this next syntax where supported (my use case focuses on postgresql, but I'm happy to extend it to other database engines where easy).
I'm planning to open a PR for this in the next few days, but wanted to open an issue first to invite discussion and see if I'm missing anything else here.