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

Cannot use json: true in Transaction.run query options #374

Closed
howdyjessie opened this issue Oct 16, 2018 · 1 comment
Closed

Cannot use json: true in Transaction.run query options #374

howdyjessie opened this issue Oct 16, 2018 · 1 comment
Assignees
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@howdyjessie
Copy link

howdyjessie commented Oct 16, 2018

It appears transaction.run ignores the json boolean that can be passed into query options. Is this intended? Seeing as transaction.read and database.run works with query.json, I expected transaction.run to also allow this flag.

Environment details

  • OS: macOS 10.14
  • Node.js version: 8.9.0
  • npm version: 6.4.1
  • @google-cloud/spanner version: 2.0.0

Steps to reproduce

  1. Call transaction.run
transaction.run({
      sql: `SELECT * FROM Singers`,
      json: true,
    });
  1. PartialResultStream doesn't receive query options in transaction.js
    runStream(query) {
    if (is.string(query)) {
    query = {
    sql: query,
    };
    }
    const reqOpts = extend(
    {
    transaction: {},
    },
    codec.encodeQuery(query)
    );
    if (this.id) {
    reqOpts.transaction.id = this.id;
    } else {
    reqOpts.transaction.singleUse = {
    readOnly: this.options || {},
    };
    }
    const makeRequest = resumeToken => {
    return this.requestStream({
    client: 'SpannerClient',
    method: 'executeStreamingSql',
    reqOpts: extend({}, reqOpts, {resumeToken: resumeToken}),
    });
    };
    return new PartialResultStream(makeRequest);
    }
  2. Rows return in the usual array of arrays format instead of JSON.

It seems like PartialResultStream just needs the additional arg as written in TransactionRequest. transaction.run works as expected when the query options are passed in.

return new PartialResultStream(makeRequest, {
json: query.json,
jsonOptions: query.jsonOptions,
});

@howdyjessie howdyjessie changed the title Cannot use json: true in Transaction.run Cannot use json: true in Transaction.run query options Oct 16, 2018
@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. 🚨 This issue needs some love. labels Oct 17, 2018
@crwilcox
Copy link
Contributor

The method runStream(query) in transaction.js does not forward query.jsonOptions to the partial result stream as is done in createReadStream(table, query) in transaction-request.js. Add handling of these options for calls to runStream(query)

@crwilcox crwilcox removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 23, 2018
@JustinBeckwith JustinBeckwith added 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 23, 2018
@google-cloud-label-sync google-cloud-label-sync bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Jan 31, 2020
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. 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

5 participants