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

Some SQL stores will silently convert Flux uint to SQL BIGINT #3067

Closed
wolffcm opened this issue Jul 31, 2020 · 1 comment
Closed

Some SQL stores will silently convert Flux uint to SQL BIGINT #3067

wolffcm opened this issue Jul 31, 2020 · 1 comment
Labels
kind/bug Something isn't working no-issue-activity team/query

Comments

@wolffcm
Copy link

wolffcm commented Jul 31, 2020 •
edited
Loading

BIGINT is generally a signed 64-bit integer in SQL. In the SQL sources supported by Flux, for some of them we will convert a Flux uint into a BIGINT when writing to that DB via sql.to(). This could result silently writing an unexpected value to the SQL DB, if the uint is greater than 2^63 - 1. To avoid this, we should throw an error when a uint column is passed to to() for insertion and the target DB does not have an integer type that can store those values.

As an example, this is the current behavior for our Postgres support:

flux/stdlib/sql/postgres.go

Line 152 in cb41c28

flux.TUInt.String(): "BIGINT",

Users can work around this by explicitly converting the values in a uint column to int.

Before we make these changes to Flux behavior we should ensure that conversions that would overflow will produce an error. The issue for that is #3066
@wolffcm wolffcm added kind/bug Something isn't working team/query labels Jul 31, 2020
@wolffcm wolffcm mentioned this issue Jul 31, 2020
Merged
2 tasks
alespour added a commit to bonitoo-io/flux that referenced this issue Aug 3, 2020
@alespour
fix: remove TUint mapping (influxdata#3067)
45f6ddc
rockstar pushed a commit that referenced this issue Aug 31, 2020
@alespour
feat(stdlib/sql): add BigQuery support (with no protobuf dep update i…
7ea0431 
…n influxdb) (#3116)

* feat: add BigQuery support

* test: improving code coverage

* test: improve code coverage more

* chore: use updated driver version

* style: remove commented out code and extended comment

* test: column name rename to reflect type

* chore: go mod tidy

* fix: remove TUint mapping (#3067)

* chore: use bigquery 1.4.0 driver (older) to prevent protobuf dep update to 1.4.x
@alespour alespour mentioned this issue Sep 16, 2020
Open
8 tasks
@github-actions GitHub Actions
Copy link

This issue has had no recent activity and will be closed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Something isn't working no-issue-activity team/query
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@wolffcm