dynamoose: Wrong behavior of $DELETE and modifying DynamoDB maps

I did some testing and found that updating with $ADD works as expected, i.e. I was able to successfully append an element to a list of type [String] by running:

Test.update(
  { id: 1 },
  { $ADD:
    { myList: ['new-item'] } 
  }
);

Next I tried deleting that element by changing $ADD to $DELETE in the above call. That call, however, deleted the whole attribute myList, not just the item. This behavior is different from DELETE in DynamoDB Update Expressions, as you can see here.

$DELETE also deletes a counter instead of decrementing it, though I am not sure if that’s different from Update Expressions behavior. I think it makes sense to have an expression that reverses the effect of $ADD. $REMOVE, perhaps?

Also, $ADD does not add a key-value pair to a DynamoDB map. With Update Expressions you can do it by using SET, you can see an example here. It would be very useful to be able to add and remove items from a map with Dynamoose. I suggest updating $ADD to support this and adding $REMOVE as an opposite of $ADD. If you think it makes sense, I might submit a PR, though it might take a while, cause my time is very limited.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 21 (15 by maintainers)

Commits related to this issue

feat(model-update): support $DELETE for delete item in set BREAKING CHANGE: Change old $DELETE to $REMOVE, and new implement $DELETE to follow AWS DynamoDB re #398 — committed to CodeBanBan/dynamoose by tatthep-hoteliers 5 years ago

Most upvoted comments

@fishcharlie it actually is, DynamoDB has REMOVE and DELETE.

The DELETE in this library does what REMOVE is supposed to do in DynamoDB, hence instead of allowing you remove elements in a Set it removes the entire field.

What I’ve done in my current project for now is to write an update expressing and use the AWS SDK to run it directly for that part of my project.

Yes, the problem here is, that the dynamoose library does not differentiate between these two commands and a $DELETE in dynamoose actually sends a “REMOVE”-Command to aws dynamodb.

Furthermore, REMOVE can either delete a whole attribute from an item OR remove an item from a LIST (not SET (!)). Since Arrays are saved as SET-Types when using dynamoose, there is no hope for the $DELETE operation in this library to remove an item from an Array. Even if one explicitly uses the LIST type, the REMOVE operation for a single item is not implemented.

To correct the behavior of this library we need to implement $REMOVE and $DELETE according to the AWS Docs. I think I might be able to submit a PR after some more research but any help would be appreciated. I wonder why this hasn’t got a priority for implementation yet. I don’t think it’s hard to do and without it the library is useless in some critical use-cases.

allesklarbeidir on Apr 12, 2019

I FINALLY got around to looking at this further. I think I’m in FULL support of this issue after taking some time to dig into it and the DynamoDB documentation. I have been playing around with the code base and trying to understand this on a deeper level as well.

I’m trying my hardest for now to build a solution that will not be considered a breaking change. That being said, I don’t know if it’s going to be possible.

My biggest concern with my solution is that if you are trying to remove a property and aren’t passing in null, this is for sure a breaking change. The documentation (https://dynamoosejs.com/api/model/#modelupdatekey-update-options-callback) says to use null. But I’m not sure if it’s explicit enough to be clear.

Also we have a test called Update $DELETE with saveUnknown enabled that doesn’t use null where it should be. Which is kinda encouraging bad use of it.

Below you can find an attachment of my solution I tried.

I have not committed this due to the fact that I’m not sure if it’s a breaking change, and I’m not sure if we should go forward with this solution.

What are everyone’s thoughts on this? If we decide my solution above should be considered a breaking change (which is what I’m thinking it is), then I’m more for doing a larger breaking change that adds a $REMOVE property, and completely changes the behavior of $DELETE. Sadly, I’m not sure if I’m gonna have time to do that larger change effort (but it looks like @CodeBanBan made a commit on a branch to do that large change tho, if @CodeBanBan is willing to submit a PR for it).

I know everyone is eager to get this support added, so any help with PRs and getting this across the finish line would be much appreciated.

fishcharlie on Jun 27, 2019

@dolsem With our new deployment system, if the commits are marked with “breaking change”, it’ll auto deploy as 2.0.0. So 2.0.0 will just have this PR.

I’m gonna review #644 now, if you’d like to as well and we can base it off of that. Looking forward to getting this in!

fishcharlie on Jun 27, 2019