Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanded support for map type in DynamoDB #339

Merged
merged 2 commits into from
Feb 10, 2015
Merged

Expanded support for map type in DynamoDB #339

merged 2 commits into from
Feb 10, 2015

Conversation

tgross
Copy link
Contributor

@tgross tgross commented Feb 10, 2015

For #308

This PR provides a UpdateExpressionUpdateAttributes method on Table that will allow us to support arbitrary UpdateExpressions to the underlying UpdateItem API endpoint.

The existing UpdateAttributes method on Table could potentially be modified to support a limited subset of UpdateExpressions, but it can't support semantics like "SET #key1.#key2=:val1" that are permitted by the DynamoDB API. Adding a new method rather than extending the existing interface will allow consuming code to avoid the complexity of writing an UpdateExpression unless it's actually needed to express operations on fields of a map (or multiple operations such as a SET and DELETE in the same call). Examples are provided in the associated test code.

Includes:

  • extended dynamo Attribute struct to include Map support
  • added recursive parsing of Maps in parseAttributes
  • follow nested structs during output to map[string]interface{} to allow Query/Scan operations to support maps
  • expanded testing of Query/Scan operations so that Map is covered

I've run the full dynamodb package's test suite via go test -check.vv -amazon -local

Includes:
- added TYPE_MAP constant
- added recursive parsing of map in parseAttributes
- follow nested structs during output to map[string]interface{}
- expanded testing of Query/Scan operations so that Map is covered
- added dummy interface for UpdateExpressionUpdateAttributes and
  failing unit test for it
… grammar

The existing UpdateAttributes method on Table could potentially be modified
to support a limited subset of UpdateExpressions, but it can't currently
support semantics like "SET #key1=:val1, DELETE #key2=:val2" that are permitted
by the DynamoDB API.

Providing a method that allows us to pass an UpdateExpression struct will support
arbitrary UpdateExpressions at the cost of the complexity to actually write one.
The added complexity only appears when we actually need it, so this seems like a
good trade off. Examples are provided in the associated test code.
@alimoeeny
Copy link
Contributor

Thanks @tgross , I'll be waiting for @cbroglie to comment. Please let me know when you are both happy and want to merge.
Also thanks for adding tests .

@cbroglie
Copy link
Contributor

It looks reasonable to me, but I'm really only intimately familiar with the Document/dynamizer functions.

@alimoeeny
Copy link
Contributor

Thanks @cbroglie .
@tgross let's merge then and we will hopefully get more feedback and contribution from people who will use it. Thanks again.

alimoeeny added a commit that referenced this pull request Feb 10, 2015
Expanded support for map type in DynamoDB
@alimoeeny alimoeeny merged commit c73835d into AdRoll:master Feb 10, 2015
@tgross
Copy link
Contributor Author

tgross commented Feb 10, 2015

I've got working code in my project using at least some of this now, so that's a good sign. Over the next week or two I'll be exercising it quite a bit, so hopefully if there are any bugs sneaking around I'll find them first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants