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

Properties not provided in upsert should be $unset #66

Open
SamVerschueren opened this issue Jul 8, 2019 · 1 comment
Open

Properties not provided in upsert should be $unset #66

SamVerschueren opened this issue Jul 8, 2019 · 1 comment
Labels

Comments

@SamVerschueren
Copy link
Owner

If we have an employee like this

{
	"Organisation": "Amazon",
	"Email": "[email protected]",
	"FirstName": "Foo",
	"Name": "Bar",
	"Hobbies": [
		"programming"
	]
}

And we call an upsert like this

Employee.upsert({
	Organisation: 'Amazon', 
	Email: '[email protected]'
}, {
	FirstName: 'Unicorn', 
	Name: 'Rainbow'
});

The result will be

{
	"Organisation": "Amazon",
	"Email": "[email protected]",
	"FirstName": "Unicorn",
	"Name": "Rainbow",
	"Hobbies": [
		"programming"
	]
}

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

{
	"Organisation": "Amazon",
	"Email": "[email protected]",
	"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
Copy link
Owner Author

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.

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

No branches or pull requests

1 participant