-
Notifications
You must be signed in to change notification settings - Fork 308
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
feat: Support parameterized NUMERIC, BIGNUMERIC, STRING, and BYTES types #673
Conversation
Move new parameterized-type code from _parse_schema_resource to from_api_repr and implement _parse_schema_resource in terms of from_api_repr.
Because that's more direct and it uncovered duplicate code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few minor remarks that were supposed (?) to be caught by the linters.
The only more significant one is about the (non) cleanup in system tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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 #668 🦕
Some notes:
select *
loses type-parameter information. :( butclient.list_rows
doesn't.SELECT
loses type information, it's mostly unavailable. IDK how often people uselist_rows
.2
and a value123
,BQ sends
123
rather than123.00
, so the Python Decimal doesn't have the right number of trailing digits.This is easy to fix, but incurs some performance overhead. IDK if we want to do this. If we do, we can do it
in a follow-on PR.