fix: unnest failed in some cases (with table references failed when there were no other references to refrenced tables in a query) #290
Conversation
|
Reviewer: This PR is explained pretty well, I think, by the comment here: Mostly unrelated, I added the missing |
And expand comment
| } | ||
|
|
||
| # By convention, dialect-provided types are spelled with all upper case. | ||
| STRING = _type_map["STRING"] | ||
| BOOL = _type_map["BOOL"] | ||
| ARRAY = _type_map["ARRAY"] |
There was a problem hiding this comment.
Does the new array type count as a feature? Perhaps it'd look better in the changelog as a separate PR?
If it is a feature and it doesn't make sense to split out, we could also try the multi-change commit message feature when we squash and merge this: https://github.com/googleapis/release-please#what-if-my-pr-contains-multiple-fixes-or-features
There was a problem hiding this comment.
It's not a new feature. It was just missing from the types list.
SQLAlchemy has basic array support that works with BigQuery (thanks to some previous effort on our part).
| @@ -34,6 +34,7 @@ | |||
| import datetime | |||
| import decimal | |||
|
|
|||
| sqlalchemy_version_info = tuple(map(int, sqlalchemy.__version__.split("."))) | |||
There was a problem hiding this comment.
This works, though I've been trying to use https://packaging.pypa.io/en/latest/version.html which is the canonical version parser. We already have it pulled in via a transitive dependency through setuptools, I believe.
There was a problem hiding this comment.
Nice. I'll switch to that.
WRT how we get it, IMO, anything we import should be a direct dependency.
There was a problem hiding this comment.
Sounds good. I'm fine explicitly including it in our dependencies / test dependencies.
Co-authored-by: Tim Swast <swast@google.com>
…-sqlalchemy into fix-unnest-283
|
@tswast are you cool with this? :) |
|
I have to more small PRs coming that will build on this. |
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 #283 🦕