BigQuery: add samples for structs, timestamps, and arrays in query parameters. #730

tswast · 2016-12-20T00:53:08Z

Currently none of these work.

Blocked by:

theacodes · 2016-12-20T17:56:32Z

bigquery/cloud-client/sync_query_params.py

@@ -90,31 +92,115 @@ def sync_query_named_params(corpus, min_word_count):
    print_results(query_results)


+def sync_query_array_params(gender, states):
+    client = bigquery.Client()
+    query_results = client.run_sync_query(


For readability, can you make the query it's own variable?

query = """ ... """ query_results = client.run_sync_query(query, ...)

theacodes · 2016-12-20T18:26:54Z

bigquery/cloud-client/sync_query_params.py

@@ -90,31 +94,118 @@ def sync_query_named_params(corpus, min_word_count):
    print_results(query_results)


+def sync_query_array_params(gender, states):


General comment, no action necessary:

These samples seem .. cramped. It might help with readability if you put blank newlines between logical steps:

client = bigquery.Client() query = """ ... """ query_results = client.run_sync_query(...) query_results.use_legacy_sql = False query_results.run() print_results(query_results)

tswast · 2017-01-04T22:24:17Z

All blocking issues have been fixed. This PR should start working after the next release of google-cloud-biguery

ace-n · 2017-04-10T23:04:23Z

Any updates on this?

tswast · 2017-04-10T23:34:45Z

Didn't realize this was still open. I'll update to the latest libs and try it out.

@ace-n If you are thinking of using this as an example for the Node.js version, please talk to me first. This sample will be evolving with the BQ client library changes this quarter.

tswast · 2017-04-12T22:25:04Z

I just pushed a change to rebase on latest master. No other changes made.

This should be okay to merge as soon as the tests pass.

@ace-n Please do not follow this as an example for Node.js. "Synchronous" queries should not be used in new samples. I'm waiting on some changes to the Python libraries before I can clean all these up to follow best practices.

tswast · 2017-04-12T22:25:39Z

@dpebot merge when green

dpebot · 2017-04-12T22:25:40Z

Okay! I'll merge when all statuses are green and all reviewers approve.

tswast · 2017-04-13T21:49:27Z

Travis failure seems like it's a problem in the veneer libs for encoding timestamps.

- query using an array value as a parameter. - query using timestamps in named parameters. See: https://cloud.google.com/bigquery/querying-data#using_timestamps_in_parameterized_queries - query using a struct in query parameters.

tswast · 2017-04-21T23:54:29Z

I did some updates to debug what's going on. I think the backend API behavior has changed since those blocking bugs were closed (ugh). All the tests for these new samples are failing.

query_params_test.py::test_query_array_params FAILED
query_params_test.py::test_query_named_params PASSED
query_params_test.py::test_query_positional_params PASSED
query_params_test.py::test_query_struct_params FAILED
query_params_test.py::test_query_timestamp_params FAILED

I'm starting a thread with the BQ eng folks.

tswast · 2017-04-24T18:09:08Z

D'oh. The reason it was failing is that I hadn't updated the samples to the latest version of google-cloud-python. Has dpebot not been updating?

theacodes · 2017-04-24T18:20:38Z

Just fixed him.

…

On Mon, Apr 24, 2017 at 11:09 AM Tim Swast ***@***.***> wrote: D'oh. The reason it was failing is that I hadn't updated the samples to the latest version of google-cloud-python. Has dpebot not been updating? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#730 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc7A4X2EKDplNj4EYHWUTFwXAuzByks5rzOVFgaJpZM4LRWR1> .

Update to latest version of google-cloud-bigquery.

tswast · 2017-04-24T19:16:34Z

Thanks.

@jonparrott The test failure looks like it's a flake with list_projects, unrelated to this change. Okay to merge?

theacodes · 2017-04-24T19:55:18Z

bigquery/cloud-client/query_params.py

+def query_positional_params(corpus, min_word_count):
+    client = bigquery.Client()
+    query = """SELECT word, word_count
+    FROM `bigquery-public-data.samples.shakespeare`


Indent this by one, and move the SELECT into the heredoc (throughout, please).

theacodes · 2017-04-24T19:55:47Z

bigquery/cloud-client/query_params.py

+                # in the query).  Note that you cannot mix named and positional
+                # parameters.
+                None,
+                'STRING',


It doesn't seem necessary to break these into lines, you an just put None, 'STRING', corpus on the same line.

theacodes

Looks (mostly) good.

tswast added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 20, 2016

tswast requested a review from theacodes December 20, 2016 00:53

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 20, 2016

theacodes suggested changes Dec 20, 2016

View reviewed changes

theacodes reviewed Dec 20, 2016

View reviewed changes

theacodes approved these changes Dec 20, 2016

View reviewed changes

tswast force-pushed the tswast-bq branch 2 times, most recently from ebbe21a to fc794f3 Compare April 12, 2017 22:22

tswast removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 12, 2017

dpebot added the automerge Merge the pull request once unit tests and other checks pass. label Apr 12, 2017

dpebot self-assigned this Apr 12, 2017

tswast assigned tswast and unassigned dpebot Apr 13, 2017

tswast removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 13, 2017

BigQuery: named parameter query samples

3554dad

- query using an array value as a parameter. - query using timestamps in named parameters. See: https://cloud.google.com/bigquery/querying-data#using_timestamps_in_parameterized_queries - query using a struct in query parameters.

tswast force-pushed the tswast-bq branch from fc794f3 to 6a09207 Compare April 21, 2017 23:53

tswast added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 21, 2017

tswast force-pushed the tswast-bq branch from 6a09207 to cde39a4 Compare April 24, 2017 18:06

tswast removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 24, 2017

Rename query file and use async method.

89c01b8

Update to latest version of google-cloud-bigquery.

tswast force-pushed the tswast-bq branch from cde39a4 to 89c01b8 Compare April 24, 2017 18:24

theacodes reviewed Apr 24, 2017

View reviewed changes

theacodes approved these changes Apr 24, 2017

View reviewed changes

tswast added 2 commits April 24, 2017 15:10

Indent queries.

2347c48

Merge branch 'master' into tswast-bq

a8c5f42

tswast merged commit dd90f35 into master Apr 24, 2017

tswast deleted the tswast-bq branch April 24, 2017 23:16

		@@ -90,31 +94,118 @@ def sync_query_named_params(corpus, min_word_count):
		print_results(query_results)


		def sync_query_array_params(gender, states):

BigQuery: add samples for structs, timestamps, and arrays in query parameters. #730

BigQuery: add samples for structs, timestamps, and arrays in query parameters. #730

Uh oh!

Conversation

tswast commented Dec 20, 2016 • edited by dpebot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes Dec 20, 2016

Choose a reason for hiding this comment

Uh oh!

tswast Dec 20, 2016

Choose a reason for hiding this comment

Uh oh!

theacodes Dec 20, 2016

Choose a reason for hiding this comment

Uh oh!

tswast commented Jan 4, 2017

Uh oh!

ace-n commented Apr 10, 2017

Uh oh!

tswast commented Apr 10, 2017

Uh oh!

tswast commented Apr 12, 2017

Uh oh!

tswast commented Apr 12, 2017

Uh oh!

dpebot commented Apr 12, 2017

Uh oh!

tswast commented Apr 13, 2017

Uh oh!

tswast commented Apr 21, 2017

Uh oh!

tswast commented Apr 24, 2017

Uh oh!

theacodes commented Apr 24, 2017 via email

Uh oh!

tswast commented Apr 24, 2017

Uh oh!

theacodes Apr 24, 2017

Choose a reason for hiding this comment

Uh oh!

tswast Apr 24, 2017

Choose a reason for hiding this comment

Uh oh!

theacodes Apr 24, 2017

Choose a reason for hiding this comment

Uh oh!

tswast Apr 24, 2017

Choose a reason for hiding this comment

Uh oh!

theacodes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tswast commented Dec 20, 2016 •

edited by dpebot

Loading