SamVerschueren / dynongo

MongoDB like syntax for DynamoDB
MIT License
58 stars 16 forks source link

Properties not provided in `upsert` should be $unset #66

Open SamVerschueren opened 5 years ago

SamVerschueren commented 5 years ago

If we have an employee like this

{
    "Organisation": "Amazon",
    "Email": "foo.bar@amazon.com",
    "FirstName": "Foo",
    "Name": "Bar",
    "Hobbies": [
        "programming"
    ]
}

And we call an upsert like this

Employee.upsert({
    Organisation: 'Amazon', 
    Email: 'foo.bar@amazon.com'
}, {
    FirstName: 'Unicorn', 
    Name: 'Rainbow'
});

The result will be

{
    "Organisation": "Amazon",
    "Email": "foo.bar@amazon.com",
    "FirstName": "Unicorn",
    "Name": "Rainbow",
    "Hobbies": [
        "programming"
    ]
}

Which is not correct, the Hobbies property should be removed.

{
    "Organisation": "Amazon",
    "Email": "foo.bar@amazon.com",
    "FirstName": "Unicorn",
    "Name": "Rainbow"
}

Solution

The solution would be to make all buildRawQuery() methods async. The reason is that for this to work, we need to retrieve the record first (with the key provided), and crossmatch the properties. All the properties that are not provided in the new object, should be unset. So for this to work, we need to call an async action in the buildRawQuery() method. To make it consistent, we should mark them all as async.

At first, we thought about implementing it as follows. Remove the original record and insert a new record within a transaction to make sure we don't loose data. The reason it doesn't work is that this would not allow you to use a upsert call within a transaction because a transaction within a transaction is not supported.

SamVerschueren commented 4 years ago

When reading this again, it made me wonder that actually reading every record first before doing an upsert might not be very good performance wise, especially when doing multiple upsert calls at once for instance. Have to skim through the dynamodb documentation to see if there are other ways to solve this.