bitemyapp / esqueleto

New home of Esqueleto, please file issues so we can get things caught up!
BSD 3-Clause "New" or "Revised" License
376 stars 108 forks source link

`val` may not round trip for an obvious `PersistField` instance #314

Open parsonsmatt opened 2 years ago

parsonsmatt commented 2 years ago

We have this instance at work:

instance PersistField UUID where
  toPersistValue = PersistLiteralEscaped . uuidToBytes
  fromPersistValue (PersistLiteralEscaped bytes)
    | Just uuid <- fromAsciiBytes bytes = Right uuid
  fromPersistValue other =
    Left $ "Failed to make UUID from: " <> tshow other

This fails to round-trip if you do a select $ pure $ val uuid - it fails, because we were given a PersistText instead of a PersistLiteralEscaped.

The logic, as far as I can tell, is something like:

  1. We splice in val uuid, which turns into ( "?", toPersistValue uuid) to ("?", PersistLiteralEscaped uuid)
  2. After parameter substitution, postgres sees a '1234-2345-3456-4567' looking thing, which looks a lot like a text value. So that's the type it infers.
  3. postgresql-simple asks Postgres what the type of the column is, and says text.
  4. So persistent wraps the text in a PersistText constructor
  5. And then esqueleto tries to parse a UUID from it, which fails, since the PersistText constructor isn't specified above.

One potential fix is to provide a type annotation to val. Something like,

val x = 
    ( mconcat ["(? :: ", showType (sqlType (Proxy :: Proxy a)), ")"]
    , toPersistValue x
    ) 

Another fix is on persistent side, where we encourage folks to use helpers instead of the PersistValue constructors directly. Eg, withBytes :: PersistValue -> (ByteString -> Either Text a) -> Either Text a, similar to the withObject etc functions in aeson. These would, behind the scenes, translate a PersistText txt -> f (encodeUtf8 txt).