aws icon indicating copy to clipboard operation
aws copied to clipboard

GetItem should use projection expressions

Open MichaelXavier opened this issue 9 years ago • 5 comments

According to the documentation, AttributesToGet in GetItem (in aws these are communicated with the giAttrs field) are deprecated. I can't find any solid info as to when or if support for this will be removed. They have been replaced by a string field called a projection expression. It looks like those are more powerful, but at their most basic, they can work the same: a list of toplevel attributes.

The route I advocate taking here is 2 steps:

  1. Keep giAttrs :: Maybe [Text], but behind the scenes to convert that into an unexported ProjectionExpression type. This could be released as a patch release as no APIs would change.
  2. Add the full features of a projection expression to the type, export it and add it as a field to GetItem. The next choice is whether to break compatibility by dropping giAttrs or keep it, add a Monoid instance to ProjectionExpression and mappend them together in the json instance. This would also entail being smart about attribute naming, as attributes with invalid characters in their name have to be put in as variables and referenced with the ExpressionAttributeNames field, which aws should be able to do automatically, which would be great.

Thoughts? I'd be willing to do the work here, but item 2 may entail some more thoughtful consideration on your end as to how you want that API to look, whereas 1 can be done at any time with little to no input.

MichaelXavier avatar Jul 08 '15 17:07 MichaelXavier

Hmm, good question. I guess it's best to introduce a new type GetItemProjecting, with gipProjectionExpression instead of giAttrs, and keep GetItem as it is.

@ozataman - what do you think?

I don't expect they will actually remove AttributesToGet anytime soon, so no need to change that.

And I'd be happy to receive a pull request for this.

aristidb avatar Jul 08 '15 18:07 aristidb

The only thing to consider about keeping giAttrs is that they cannot be sent over the API together. On the backend, aws will need to combine them into one projection expression and the documentation should reflect that.

MichaelXavier avatar Jul 08 '15 18:07 MichaelXavier

If you make a separate type, that will enforce them not being sent together.

Michael Xavier [email protected] schrieb am Mi., 8. Juli 2015 um 20:25 Uhr:

The only thing to consider about keeping giAttrs is that they cannot be sent over the API together. On the backend, aws will need to combine them into one projection expression and the documentation should reflect that.

— Reply to this email directly or view it on GitHub https://github.com/aristidb/aws/issues/169#issuecomment-119689094.

aristidb avatar Jul 08 '15 19:07 aristidb

I'm a bit confused. I thought you were advocating keeping giAttrs and adding a projection field, in which case they must be combined as it would be possible to send both. Are you saying giAttrs should be removed?

MichaelXavier avatar Jul 08 '15 20:07 MichaelXavier

I'm saying: Add a TYPE. Current type: GetItem. New type: GetItemProjecting. The new type will not have the old field, only the new one

Michael Xavier [email protected] schrieb am Mi., 8. Juli 2015 22:01:

I'm a big confused. I thought you were advocating keeping giAttrs and adding a projection field, in which case they must be combined as it would be possible to send both. Are you saying giAttrs should be removed?

— Reply to this email directly or view it on GitHub https://github.com/aristidb/aws/issues/169#issuecomment-119715418.

aristidb avatar Jul 09 '15 16:07 aristidb