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

runQuery doesn't run inside transaction #905

Closed
pcostell opened this issue Oct 5, 2015 · 3 comments
Closed

runQuery doesn't run inside transaction #905

pcostell opened this issue Oct 5, 2015 · 3 comments
Assignees
Labels
api: datastore Issues related to the Datastore API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@pcostell
Copy link
Contributor

pcostell commented Oct 5, 2015

Sorry if this isn't right, I only noticed it through inspection.

It looks like Transaction extends from DatastoreRequest, which means it gets a runQuery method. However, it doesn't look like it overrides it. This means a user who thinks they are running a query in the transaction is actually going to get inconsistent results.

@pcostell pcostell added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: datastore Issues related to the Datastore API. labels Oct 5, 2015
@stephenplusplus
Copy link
Contributor

I think it will work as intended. When the API request is made, it goes through this method: https://github.com/GoogleCloudPlatform/gcloud-node/blob/69046d383af6bf67a316739c66650601228abc8a/lib/datastore/request.js#L759:

DatastoreRequest.prototype.makeReq_ = function(method, body, callback) {
  // ...

  if (method === 'lookup' && this.id) {
    body.read_options = body.read_options || {};
    body.read_options.transaction = this.id;
  }

  // ...

this.id is set to the transaction ID when using a Transaction object.

@pcostell
Copy link
Contributor Author

pcostell commented Oct 6, 2015

That's if the method is 'lookup'. But what if the method is 'runQuery'?

@stephenplusplus
Copy link
Contributor

Ah, woops. So I think we just need this.id && (method === 'lookup' || method === 'runQuery').

sofisl pushed a commit that referenced this issue Nov 11, 2022
Source-Link: googleapis/synthtool@b801276
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:ba3f2990fefe465f89834e4c46f847ddb141afa54daa6a1d462928fa679ed143

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants