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

bigquery: support SQL parameters #1854

Merged
merged 12 commits into from
Dec 6, 2016

Conversation

stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Dec 2, 2016

Fixes #1787

To Dos

  • Docs
  • Tests
    • System
      • Positional
        • Arrays
        • Booleans
        • Floats
        • Integers
        • Strings
        • Timestamps
        • Structs
        • Bytes
        • Date
        • Datetime
        • Time
      • Named
        • Arrays
        • Booleans
        • Dates
        • Floats
        • Integers
        • Strings
        • Timestamps
        • Structs
        • Bytes
        • Date
        • Datetime
        • Time
    • Unit

This introduces a nice wrapper around SQL parameter queries, allowing positional and named parameters:

bigquery.query({
  query: 'SELECT * FROM `github` WHERE owner = ?',
  params: ['stephenplusplus']
}, callback);

bigquery.query({
  query: 'SELECT * FROM `github` WHERE owner = @owner',
  params: {
    owner: 'stephenplusplus'
  }
}, callback);

Helpful resources:

@stephenplusplus stephenplusplus added api: bigquery Issues related to the BigQuery API. enhancement labels Dec 2, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 2, 2016
@stephenplusplus
Copy link
Contributor Author

@blowmage shout out for your system tests, which helped me a lot while putting this together. Did you ever find a good query for structs?


if (type === 'TIMESTAMP') {
// @TODO - Not sure why .toJSON() doesn't just work.
value = new Date(value).toJSON().replace(/(.*)T(.*)Z$/, '$1 $2');

This comment was marked as spam.

This comment was marked as spam.

@blowmage
Copy link
Contributor

blowmage commented Dec 2, 2016

@stephenplusplus Not yet. We are looking for one though.

});
});

it.skip('should work with structs', function(done) {

This comment was marked as spam.

This comment was marked as spam.

@callmehiphop
Copy link
Contributor

@stephenplusplus should I wait to merge for the struct stuff? Everything else looks good.

@blowmage
Copy link
Contributor

blowmage commented Dec 6, 2016

Probably worth noting that there are more types that can be supported. Our initial implementation missed DATETIME, TIME, and BYTES. (See googleapis/google-cloud-ruby#1100)

@stephenplusplus
Copy link
Contributor Author

Thanks for the heads up!

@callmehiphop
Copy link
Contributor

@stephenplusplus do you want to get those other types in before merge?

@stephenplusplus
Copy link
Contributor Author

Yeah, I'll have a PR soon for them. In the testing phase now.

@stephenplusplus
Copy link
Contributor Author

Alright, ready for another look @callmehiphop

* }
* }, callback);
*
* //-

This comment was marked as spam.

This comment was marked as spam.

});
});

it.skip('should work with DATETIME types', function(done) {

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


/**
* A `DATETIME` data type represents a point in time. Unlike a `TIMESTAMP`, a
* this does not refer to an absolute instance in time. Instead, it is the civil

This comment was marked as spam.

return 'STRUCT';
}

return 'STRING';

This comment was marked as spam.

* @param {object|*[]} options.params - For positional SQL parameters, provide
* an array of values. For named SQL parameters, provide an object which
* maps each named parameter to its value. The supported types are integers,
* floats, Date objects, Strings, Booleans, and Objects.

This comment was marked as spam.

This comment was marked as spam.

@@ -450,6 +724,39 @@ BigQuery.prototype.job = function(id) {
* });
*
* //-
* // Positional and named SQL parameters are supported.

This comment was marked as spam.

27,
28,
29
]

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor Author

Changes made!

*/
BigQuery.valueToQueryParameter_ = function(value) {
if (is.date(value)) {
value = this.timestamp(value);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@callmehiphop callmehiphop merged commit f5a25e7 into googleapis:master Dec 6, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 4616c59 on stephenplusplus:spp--1787 into ac87e18 on GoogleCloudPlatform:master.

@c0b
Copy link
Contributor

c0b commented Dec 13, 2016

@blowmage 11 days ago Collaborator
Not a bug AFAIK. BigQuery uses a very specific date format.
https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#canonical-format_3

is there anyway to file an issue to BigQuery design? my app use both BigQuery and Datastore, and Datastore's DATETIME apprarently accepts RFC3339 format; there is no reason BigQuery server side can't accept this ISO standard format
https://cloud.google.com/datastore/docs/reference/gql_reference#synthetic_literals

@blowmage
Copy link
Contributor

@c0b I am unable to answer that, clients like google-cloud-node consume the API and we don't have access to the design decisions behind it. We don't have access to Google's internal bug tracker.

cc @danoscarmike

@tswast
Copy link
Contributor

tswast commented Dec 14, 2016

@c0b @blowmage BigQuery uses a public issue tracker. I have filed an issue for this. https://code.google.com/p/google-bigquery/issues/detail?id=842 Please "star" the issue to show your support and receive status updates.

@blowmage
Copy link
Contributor

Great. Thanks @tswast!

@jcondit
Copy link

jcondit commented Dec 14, 2016

Can you be more specific about where you'd like this format to be accepted? It's already accepted for load jobs with JSON or CSV, for example.

@jcondit
Copy link

jcondit commented Dec 14, 2016

Ah, never mind, just noticed this is in query parameters.

value = value || new Date();

if (is.date(value)) {
value = value.toJSON().replace(/^(.*)T(.*)Z$/, '$1 $2');

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants