batch-write-item product behaviour causing problems inserting items with lists

[wotbrew]

If we have top level lists in our items to insert e.g

{:put [{:id 0 :foo [1 2]}, {:id 1}]}

it seems the request for item with :id 0 will be duplicated as

[{:id 0, :foo [1]}, {:id 0, :foo [2]}, ...] 

I believe you take a product when list values are involved to support deletes as documented in the doc string for batch-write-item:

{:users {:put    [{:user-id 1 :username \"sally\"}
                        {:user-id 2 :username \"jane\"}]
             :delete [{:user-id [3 4 5]}]}})

Obviously this causes problems when the intention is to put a list into dynamo.

Hopefully that is clear, comment if it isn't!

Regards

Dan

[ptaoussanis]

Hi Dan, thanks for the report.

You're right about the cause. When I wrote attr-multi-vs way-back-when, there wasn't a native list type. Added support for the list type later and hadn't thought about the interplay here.

I don't actually use DDB/Faraday myself, so would appreciate input from folks who do. One idea off the top of my head: we could support a ^:literal metadata flag on vectors given to batch-write-item to skip the usual product behaviour?

Or the default could go the other way (i.e. we could offer an ^:each metadata flag) and default to literal vectors otherwise.

Thoughts/PRs welcome.

[joelittlejohn]

Strawman proposal (feel free to shoot this down :smile:):

Rather than use metadata flags to dictate the behaviour here, would it be better at this point to simply remove the list shortcut and force updates to be provided 'longhand'. Obviously the cartesian product behaviour would be lost, but it's possible to achieve this outside Faraday of course.

This would be a major breaking change and worth a major version change IMO (breakver notwithstanding).

[ptaoussanis]

Strawman proposal (feel free to shoot this down :smile:):

No, no - this is the kind of feedback I'm looking for (thanks Joe). Again, I haven't used any of this code so have no opinion myself.

it be better at this point to simply remove the list shortcut and force updates to be provided 'longhand'

I'd be open to that. Any objections?

This would be a major breaking change and worth a major version change IMO (breakver notwithstanding).

A v2 would seem appropriate to me.

[wotbrew]

I would much rather 'opt-in' to the product behaviour, and agree it should be ideally an explicit action and doesn't really need to be part of faraday.

[tcoupland]

Agree with the idea of pushing this out of faraday. A nice to have that's no longer so nice :)

[ptaoussanis]

Okay, on track to remove this in a future breaking v2 release.

Have introduced a non-breaking opt-out flag in the meantime (untested): https://github.com/ptaoussanis/faraday/commit/d2daff3b742faf5bc4884e79390fb5c1c01f07ec - just supply {attr-multi-vs? false} to batch-write-item or batch-get-item.

[ptaoussanis]

Changing the attr-multi-vs? default to false in the latest v1.9.0 SNAPSHOT.