fix: distinct doesn't work as a column wrapper #275
Conversation
product-auto-label
bot
added
the
api: bigquery
google-cla
bot
added
the
cla: yes
|
Note to reviewers: This bug occurred because the dialect copies The fix was to sync up with the version from sqlalchemy, to the extent possible given that it differs across different versions of SQLAlchemy. 5e4074d#diff-efe1987a73ae5b54aa817dc8c70541025df89d9d08601c8ab33fa84ad8c09de7R266-R271 in particular. This is needed so SQLAlchemy knows how to map subexpressions in queries (e.g. distinct) to columns. The long term fix will be to stop copying this method. |
There was a problem hiding this comment.
@jimfulton wrote:
The long term fix will be to stop copying this method.
Is there an issue open to track the long-term fix?
sqlalchemy_bigquery/base.py
Outdated
|
|
||
| if is_literal: | ||
| # note we are not currently accommodating for | ||
| # literal_column(quoted_name('ident', True)) here |
There was a problem hiding this comment.
Is there an issue open for addressing that gap?
There was a problem hiding this comment.
That comment is part of the copy from sqlalchemy. I'll delete it. :/
| @@ -691,3 +691,37 @@ def test_has_table(engine, engine_using_test_dataset, bigquery_dataset): | |||
| assert engine_using_test_dataset.has_table(f"{bigquery_dataset}.sample") is True | |||
|
|
|||
| assert engine_using_test_dataset.has_table("sample_alt") is False | |||
|
|
|||
|
|
|||
| def test_distinct_188(engine, bigquery_dataset, metadata): | |||
There was a problem hiding this comment.
The metadata fixture doesn't look to be used here.
There was a problem hiding this comment.
Right you are! I'll remove it.
|
|
@tseaver are you cool with this? |
To work around spurious test failures
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #188 🦕