yesodweb / yesodweb/persistent

[RFC] Deprecate non-normal fields

Open
#1,254 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

rfc
Dominant language
Haskell
Stars
486
Forks
306
PR merge metrics
No merged PRs in 30d

Description

persistent currently allows a bunch of non-normalized fields. Lists, maps, entire other entities.

For the most part, these are serialized as Strings, using the {To,From}JSON PersistValue instance. This isn't great from a database design perspective or a data serialization perspective. While the functionality can be useful, new features like JSONB columns subsume it, and type-directed serialization (like a JSONB newtype wrapper) would better serve people's uses.

This proposal has two components:

  • Deprecate embedded entities
  • Deprecate all non-normalized fields (by default).

Backwards Compatibility

This should probably come about in two phases:

  1. The TH code reports a warning when we're doing an embedded field. A newtype is provided to maintain the current behavior - something like AsPersistValueString that has the relevant PersistField instances, and the warning points users to the newtype. Complexity in the encoding is hidden in the newtype.
  2. Finally the embedding code can be deleted in the next major version. The PersistField (Entity rec) instances are no longer generated.

Embedded Entities

This is heavily used in MongoDB, where it makes sense - everything is just JSON anyway, so Mongo almost entirely relies on the EmbedEntityDef rather than the EntityDef.

Unfortunately, the code to handle all of this is extremely complex. Entities can refer to each other, themselves, and also have mutual recursion. The code parses Text -> [EntityDef], and then lifts the [EntityDef] into Q Exp using a special type that can defer some information to the next phase of compilation. Then mkPersist is capable of the same trick - deferring some of the code generation using Q Exp. Finally, the embedEntityDefMap relies on laziness to set the fields - a cryptic comment states "Let Haskell tie the knot."

Dropping this feature would be fantastic from a code maintenance standpoint.

I believe we could easily retain the current behavior using a newtype AsPersistValueString rec which performs the relevant encoding.

Denormalized fields

A [Int] in a database column is currently a PersistList, which is serialized using PersistText . encodeUtf8 . Aeson.encode . map toPersistValue. This is terrible on a number of fronts. Likewise, a Map Int Char is stored as a PersistMap, which (in SQL backends) is stored in the same way.

As above, we could warn users and point them to AsPersistValueString to wrap it. Eg:

 User
     name  Text
-    emails [Text]
+    emails (AsPersistValueString [Text])
-    contacts (Map String PhoneNumber)
+    contacts (AsPersistValueString (Map String PhoneNumber))

This retains the current behavior. Users can be directed to other newtypes with better characteristics, and possibly migration strategies too (though this is backend-dependent).

Side benefits

This will make work towards #862 easier, allowing us to define types that are represented as multiple columns.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the TH code, mkPersist, embedEntityDefMap, and the PersistField instances described in the issue. Determine how embedded entities and non-normalized fields are currently generated, then define the warning and compatibility-wrapper behavior before considering removal in a later major version.

Written by the indexing model from the issue text.

Assessment

Tech stack
haskell
Domain
databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.