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

None or empty array parameter values aren't handled in the dbapi #609

Closed
jimfulton opened this issue Apr 14, 2021 · 5 comments · Fixed by #626
Closed

None or empty array parameter values aren't handled in the dbapi #609

jimfulton opened this issue Apr 14, 2021 · 5 comments · Fixed by #626
Assignees
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. dbapi priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@jimfulton
Copy link
Contributor

jimfulton commented Apr 14, 2021

>>> import google.cloud.bigquery.dbapi
>>> conn = google.cloud.bigquery.dbapi.connect()
/home/jim/p/g/python-bigquery/google/cloud/bigquery/client.py:444: UserWarning: Cannot create BigQuery Storage client, the dependency google-cloud-bigquery-storage is not installed.
  warnings.warn(
>>> cursor = conn.cursor()
>>> cursor.execute("select %(x)s", dict(x=None))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jim/p/g/python-bigquery/google/cloud/bigquery/dbapi/_helpers.py", line 270, in with_closed_check
    return method(self, *args, **kwargs)
  File "/home/jim/p/g/python-bigquery/google/cloud/bigquery/dbapi/cursor.py", line 173, in execute
    query_parameters = _helpers.to_query_parameters(parameters)
  File "/home/jim/p/g/python-bigquery/google/cloud/bigquery/dbapi/_helpers.py", line 167, in to_query_parameters
    return to_query_parameters_dict(parameters)
  File "/home/jim/p/g/python-bigquery/google/cloud/bigquery/dbapi/_helpers.py", line 146, in to_query_parameters_dict
    param = scalar_to_query_parameter(value, name=name)
  File "/home/jim/p/g/python-bigquery/google/cloud/bigquery/dbapi/_helpers.py", line 53, in scalar_to_query_parameter
    raise exceptions.ProgrammingError(
google.cloud.bigquery.dbapi.exceptions.ProgrammingError: encountered parameter x with value None of unexpected type

The same issue applies to empty array ([]) parameters.

@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Apr 14, 2021
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Apr 15, 2021
@jimfulton jimfulton changed the title None parameter values aren't handled in the dbapi None or empty array parameter values aren't handled in the dbapi Apr 15, 2021
@plamut plamut added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Apr 16, 2021
@jimfulton jimfulton self-assigned this Apr 16, 2021
@jimfulton
Copy link
Contributor Author

FTR, I have a local fix that I'll finalize after I finish getting sqlalchemy compliance tests to pass.

@plamut
Copy link
Contributor

plamut commented Apr 16, 2021

@jimfulton If the fix touches the Cursor code and might cause conflicts, I suggest reviewing and merging two existing DB API PRs first, and then rebasing on top of that.

@jimfulton
Copy link
Contributor Author

But wait, there's more!

The dry-run trick fails with:

SELECT `date_table`.`id` AS `date_table_id` 
FROM `date_table` 
WHERE CASE WHEN (@`foo` IS NOT NULL) THEN @`foo` ELSE `date_table`.`date_data` END = `date_table`.`date_data`

when the value passed for foo is None.

The server side guesses type of foo twice, guessed differently, and then complains that the guesses were different:

Undeclared parameter 'foo' is used assuming different types (TIME vs INT64) at [3:18]

Arguably, a sophisticated analysis would decide the type of date_data (TIME in this case).

In the case of SQLAlchemy, it happens that we know the type of the parameter. If we could somehow get to BQ, we could avoid the dry-run call.

Or maybe this should be viewed as a BQ time-inference bug. I think I'll skip the relevant SQLAlchemy-dialect tests until I get a read on that. :)

@jimfulton
Copy link
Contributor Author

It seems, according to https://issuetracker.google.com/issues/185823434, that the dry-run feature isn't meant for determining parameter types.

@jimfulton
Copy link
Contributor Author

Because:

  • We can't use dry-run to determine type information.
  • We don't like making extra API calls.
  • Callers have type information.

We decided to extend the parameter syntax to provide type information.

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 googleapis/python-bigquery API. dbapi priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
3 participants