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

datastore: Allow inserts not just upserts #886

Closed
lc-chrisbarton opened this issue Feb 6, 2018 · 9 comments
Closed

datastore: Allow inserts not just upserts #886

lc-chrisbarton opened this issue Feb 6, 2018 · 9 comments
Assignees
Labels
api: datastore Issues related to the Datastore API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lc-chrisbarton
Copy link

lc-chrisbarton commented Feb 6, 2018

We're processing orders using datastore and pubsub and some times we have an order followed by a cancellation and sometimes we receive duplicate orders from pubsub. We would like to ensure the cancel fails if we perform a put and the order doesn't exist and duplicated orders are rejected. So basically we want create and update not just upsert. The node.js client has this feature. We work around this using a transaction with a get on the order id but I guess it would be faster if the Go client exposed this functionality?

@jba jba self-assigned this Feb 7, 2018
@jba jba added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: datastore Issues related to the Datastore API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Feb 7, 2018
@apstndb
Copy link
Contributor

apstndb commented Feb 8, 2018

#872

@piotrkowalczuk
Copy link
Contributor

piotrkowalczuk commented Feb 8, 2018

I'm facing serious contention problem because I am not able to do simple insert. Instead of that I have to perform a transaction. I know Key.ID upfront so Put method does Upsert instead of Insert mutation. Being able to control this behavior outside of Put would solve my problem.

Here is small API change proposal that address this issue: https://code-review.googlesource.com/#/c/gocloud/+/22970/

@jba
Copy link
Contributor

jba commented Feb 8, 2018

@zombiezen

We do want to support this, and it is not a lot of work. The problem with the proposal in the CL is that it changes the signature of Put, and we can't do that. An obvious alternative is separate Insert and Update methods, but we'd need eight new methods (two each for Client.Put, Client.PutMulti, Transaction.Put and Transaction.PutMulti). Is there a nicer way?

@zombiezen
Copy link
Contributor

(Handwavy and untested.) You could introduce something like:

// Update wraps an argument to Put and causes the operation to fail if the entity does not exist.
type Update struct {
  Src interface{}
}

A user would use this like:

myOrder := Order{Canceled: true}
datastore.Put(datastore.IDKey("Order", 42, nil), datastore.Update{Src: myOrder})

Seems like that would work without breaking compatibility.

@lc-chrisbarton
Copy link
Author

@zombiezen I like that approach :)

@apstndb
Copy link
Contributor

apstndb commented Feb 9, 2018

I'm worried about complicating Put(and PutMulti) method.
I prefer to introduce generic mutation because insert, upsert(a.k.a put), update and delete are type of mutation in Cloud Datastore API.
https://cloud.google.com/datastore/docs/reference/data/rest/v1/projects/commit?hl=en#request-body

So we may be able to batch them in single method.

client.MutateMulti(ctx context.Context, keys, []datastore.Mutation{datastore.Update{...}, datastore.Upsert{...}, datastore.Delete{...}}) ([]*Key, error)

or fluent style(like firestore)

mutations := datastore.Mutations().
    Update(key1, val1).
    Upsert(key2, val2)
client.Batch(mutations)

@lc-chrisbarton
Copy link
Author

lc-chrisbarton commented Feb 9, 2018

@apstndb well I would just have create(), read(), update(), delete(), upsert() since that is simple and obvious and the user can choose what they want

@jba
Copy link
Contributor

jba commented Feb 9, 2018

I went with the mutation idea. It is the most general. Please take a look at https://code-review.googlesource.com/#/c/gocloud/+/23170.

@jba
Copy link
Contributor

jba commented Feb 12, 2018

CL submitted.

@jba jba closed this as completed Feb 12, 2018
gcf-owl-bot bot added a commit that referenced this issue Feb 29, 2024
Source-Link: googleapis/googleapis@0df0f51

Source-Link: googleapis/googleapis-gen@b0b8f14
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjBiOGYxNDdhNzcxNjJiYmM2MDNjYzFlMDQ3NTJhMzYyMWUxMmViNSJ9
gcf-merge-on-green bot pushed a commit that referenced this issue Feb 29, 2024
…9485)

- [ ] Regenerate this pull request now.

Source-Link: https://togithub.com/googleapis/googleapis/commit/0df0f51c86a9316e5bff871ec9ba79de1050b29c

Source-Link: https://togithub.com/googleapis/googleapis-gen/commit/b0b8f147a77162bbc603cc1e04752a3621e12eb5
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjBiOGYxNDdhNzcxNjJiYmM2MDNjYzFlMDQ3NTJhMzYyMWUxMmViNSJ9
BEGIN_NESTED_COMMIT
feat(spanner/admin/instance): Add instance partition support to spanner instance proto
PiperOrigin-RevId: 611127452

Source-Link: https://togithub.com/googleapis/googleapis/commit/618d47cf1e3dc4970aaec81e417039fc9d62bfdc

Source-Link: https://togithub.com/googleapis/googleapis-gen/commit/92d855588828430e8b428ed78219e23ee666da78
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTJkODU1NTg4ODI4NDMwZThiNDI4ZWQ3ODIxOWUyM2VlNjY2ZGE3OCJ9
END_NESTED_COMMIT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

5 participants