amazonka-dynamodb: Use a sum type for AttributeValue

[endgame]

Note:

• I haven't actually used this in anger yet. Putting it up now for discussion and possibly to test work code against once we get across to amazonka-2.0 RC1 (next week, most likely).
• Branch is currently based off amazonka-route53: correct S3 Website Hosted Zone IDs #723 so I can write a decent changelog entry. Merge that first and then this will look sensible.
• I had to temporarily revert amazonka-gen: Synthesise NFData and Hashable implementations #721 while developing this because the generated instances failed to compile with "Ambiguous Type" errors.
• I have tested building using --constraints 'aeson < 2.0' as well as --constraints 'aeson >= 2.0'.
• Need to scripts/generate dynamodb before testing, obviously.

Open questions:

• Do we actually want this? I feel that it's OK for amazonka to do this because it hews closely to the data formats, but a more advanced/opinionated mapping library would be out of scope for this project.
• Should we use the pattern synonym/newtype trick here in case AWS add more data types to dynamo? Feels a bit unlikely.
• I believe @brendanhay has a long-term plan to remove the direct lens dependency - I'm prepared to write the prisms manually if necessary.

[endgame]

CC @lrworth @Unisay @axman6

[ghost]

This looks great, my only comments relate to the in memory representation. I can't see much point in not using the most space efficient representations we can for a package like this. I'm sure most heavy users of the package would appreciate not having a large increase in memory size compared to the underlying JSON. I would be very surprised if laziness in the values is particularly useful here. Happy to discuss, but this is how I would design it for better performance/memory efficiency.

[endgame]

Oh, and we'll need to repin stackage after merge.

[brendanhay]

See https://github.com/brendanhay/amazonka/tree/bh/dynamodb-mapper/amazonka-dynamodb-extras/src/Amazonka/DynamoDB for a blast from the past. It was mostly just random musings around what a layer above amazonka-dynamodb might look like.

[endgame]

I had a look at your mapper; we have something a bit similar in an internal codebase. The main problem I have with the typeclass-driven approaches is that I often don't want to read/write an entire item - many of our operations are doing partial SETs, or projecting out a couple of fields, or whatever.

For this reason, I prefer building up encoder/decoders by combining values. If you keep an Encoder a and Decoder a next to each other, you can make a profunctor P a a and use the tools in product-profunctors to build up a codec for larger structures. Then you can break off the encoder or decoder as-needed. But I need to get around to finishing the big refactor of that package...

---

Anyway. I'll build up an internal test branch for work (with this, regen amazonka-dynamodb, etc) to make sure this type does what we need here. That should happen early-mid next week, after which I can merge this.

[brendanhay]

The branch is close to 6 years old - I likely now wouldn't use type classes either, as it currently stands. I just figured it might be of interest in regards to other functionality.

[endgame]

Oh BTW @brendanhay do you want hand-written prisms instead of TH-generated ones?

[endgame]

Discovered that DynamoDB Streams has the same type. Not sure if there's a better way to do this.

[endgame]

Oh, and we'll need to repin stackage after merge.

And, annoyingly, stackage is on hashable-1.3.0.0 for GHC 8.10.7, which doesn't have instance (Hashable k, Hashable v) => Hashable (Map k v). Should we hashUsing (Map.toList)?

[axman6]

🎉

[endgame]

Status update: generic-lens has a good story for prisms, and this branch makes using AttributeValue so much more pleasant.

[endgame]

Gonna merge this now, but I've filed commercialhaskell/stackage#6386 to see how likely it we'll see Stackage LTS with GHC 8.10.x and hashable >=1.3.4.0. If it's not gonna happen, I'll PR a compatibility fix.

[endgame]

Since amazonka-2.0 (whenever we get it out) won't make it into existing stackage LTS, stack users will have to extra-deps their way to victory. I'm comfortable with forcing them to add hashable to their extra-deps:.