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.

Sign up for GitHub

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

Jump to bottom

bigquery: big.Rat limited to 9 digits #4704

Closed
Thomasdezeeuw opened this issue Aug 31, 2021 · 4 comments · Fixed by #6596
Closed

bigquery: big.Rat limited to 9 digits #4704

Thomasdezeeuw opened this issue Aug 31, 2021 · 4 comments · Fixed by #6596
Assignees
@alvarowolfx
Labels
api: bigquery Issues related to the BigQuery API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@Thomasdezeeuw
Copy link

Client

BigQuery

Environment

N/A

Go Environment

go version go1.16.6 darwin/arm64

GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/thomas/Library/Caches/go-build"
GOENV="/Users/thomas/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/thomas/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/thomas/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.16.6/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.16.6/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.16.6"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lv/qbz3lh7j6gdc43dd1p22bcc40000gn/T/go-build541126118=/tmp/go-build -gno-record-gcc-switches -fno-common"

Code

https://pkg.go.dev/cloud.google.com/go/bigquery#InferSchema documents that big.Rat types are limited to 9 digits after then decimal, this also seems to be the case for parameters to query. However BigQuery supports much more precision (up to 76, https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#numeric-type). It would be nice to able to query using these high precision numbers.

I guess that the parameters are send as strings so you have to make a decision, but could the limit be increased? Alternatively can we send our own string (with our own defined precision) as number?

Also related: golang/go#47911: Support precision of -1 in Rat.FloatString (that will use the required number of digits to represent the number).

Expected behavior

I expected large precision numbers to be support.

Actual behavior

Large numbers are limited to 9 digits after the decimal.
@Thomasdezeeuw Thomasdezeeuw added the triage me I really want to be triaged. label Aug 31, 2021
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the BigQuery API. label Aug 31, 2021
@shollyman
Copy link
Contributor

Some background: The underlying issue here is the big.Rat doesn't report the value's precision or scale, so we have to pick the most appropriate type. And for compatibility sake, we continue to infer the NUMERIC type, as to do otherwise might break existing InferSchema users in surprising ways.

Much like Relax() on a schema changes all required fields to optional, one could do a similar traversal to alter all inferred NUMERIC types to your desired type. Are you using consistent destination BQ type when you infer? Would a method that converts all instances of type A to type B in a schema be sufficient?

@shollyman shollyman added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Sep 13, 2021
@Thomasdezeeuw
Copy link
Author

Some background: The underlying issue here is the big.Rat doesn't report the value's precision or scale, so we have to pick the most appropriate type.

Something like golang/go#47911 would help with this I think? Or maybe something like big.Float.MinPrec?

Also see #4703 (comment).

@shollyman shollyman assigned alvarowolfx and unassigned shollyman Aug 26, 2022
@shollyman
Copy link
Contributor

Alvaro, mind taking a look at this one?

I suspect the resolution here (and in some related issues) is to do the work to enable explicit query parameter construction. There are multiple cases where we have ambiguous type conversions, so the goal would be to allow users to construct a query parameter explicitly, rather than the current behavior which infers everything from the Go type.

A source of potential complexity here is complex types (e.g. struct types), which are completely valid for query parameters. The interesting questions are whether we allow any type inferencing of the leaf types, or whether everything must be explicit etc.

@alvarowolfx alvarowolfx mentioned this issue Aug 31, 2022
Merged
alvarowolfx added a commit that referenced this issue Sep 16, 2022
@alvarowolfx
feat(bigquery): add support for explicit query parameter type (#6596)
d59b5b2 
Allow users to pass query parameter with an explicit data types instead of relying just on type inference. 

Resolves #4704
@release-please release-please bot mentioned this issue Sep 16, 2022
Merged
@Thomasdezeeuw
Copy link
Author

Thanks @alvarowolfx!

gcf-merge-on-green bot pushed a commit that referenced this issue Sep 21, 2022
@release-please
chore(main): release bigquery 1.42.0 (#6675)
fee0f68 
🤖 I have created a release *beep* *boop*
---


## [1.42.0](bigquery/v1.41.0...bigquery/v1.42.0) (2022-09-21)


### Features

* **bigquery/analyticshub:** Start generating apiv1 ([#6707](#6707)) ([feb7d7d](feb7d7d))
* **bigquery/datapolicies:** Start generating apiv1beta1 ([#6697](#6697)) ([f5443e8](f5443e8))
* **bigquery/reservation/apiv1beta1:** add REST transport ([f7b0822](f7b0822))
* **bigquery/storage/managedwriter:** Define append retry predicate ([#6650](#6650)) ([478b8dd](478b8dd))
* **bigquery/storage:** add proto annotation for non-ascii field mapping ([ec1a190](ec1a190))
* **bigquery:** Add reference file schema option for federated formats ([#6693](#6693)) ([3d26091](3d26091))
* **bigquery:** Add support for explicit query parameter type ([#6596](#6596)) ([d59b5b2](d59b5b2)), refs [#4704](#4704)


### Bug Fixes

* **bigquery/connection:** integrate  gapic-generator-python-1.4.1 and enable more py_test targets ([ec1a190](ec1a190))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Sep 27, 2022
Closed
Closed
Closed
@Thomasdezeeuw Thomasdezeeuw mentioned this issue Oct 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alvarowolfx alvarowolfx

Labels
api: bigquery Issues related to the BigQuery API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(bigquery): add support for explicit query parameter type alvarowolfx/google-cloud-go
3 participants
@shollyman @alvarowolfx @Thomasdezeeuw