Skip to content

Column type selection for BIGNUMERIC vs NUMERIC is incorrect #1164

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

Open
claytonjroberts opened this issue Feb 14, 2025 · 7 comments · May be fixed by #1165
Open

Column type selection for BIGNUMERIC vs NUMERIC is incorrect #1164

claytonjroberts opened this issue Feb 14, 2025 · 7 comments · May be fixed by #1165
Assignees
Labels
api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API.

Comments

@claytonjroberts
Copy link

claytonjroberts commented Feb 14, 2025

Description

There is a discrepancy between the documentation for numeric types and what appears when creating a numeric column:

Field [some_field] has precision [some_precision] and scale [some_scale] but precision and scale for NUMERIC must be: 0 <= precision - scale <= 29

from the BigQuery Console

Environment details

  • OS type and version:
  • Python version: Python 3.11.10
  • pip version: pip 24.0
  • sqlalchemy-bigquery version: Version: 1.12.0

Steps to reproduce

  1. Attempt to make a table with a column of type NUMERIC with precision=37 and scale=0 using SQLAlchemy

Code example

import sqlalchemy as sa
from sqlalchemy_bigquery._types import NUMERIC


engine = sa.create_engine("bigquery://")

table = sa.Table(
    "test", sa.MetaData(...), sa.Column("id", NUMERIC(37, 0), primary_key=True)
)

table.create(engine)

Stack trace

DatabaseError: (google.cloud.bigquery.dbapi.exceptions.DatabaseError) 400 POST https://bigquery.googleapis.com/bigquery/v2/projects/govis-sandbox/queries?prettyPrint=false: In NUMERIC(P, 0), P must be between
1 and 29 at [3:27]
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API. label Feb 14, 2025
@Linchin
Copy link
Contributor

Linchin commented Feb 18, 2025

Thank you, I think you are using the parameterized decimal type here (meaning you specify the precision and scale), and in this case, the max precision available is 29. So I think the documentation is correct here, but we can sure try to catch this in the library, so it's less confusing.

@claytonjroberts
Copy link
Author

@Linchin no, the logic for setting the column type (in the generated BQ SQL) is incorrect. See my PR. Its setting the type as NUMERIC when it should be BIGNUMERIC.

@Linchin
Copy link
Contributor

Linchin commented Feb 18, 2025

@claytonjroberts The problem is visit_NUMERIC() covers the case of non-parameterized NUMERIC and BIGNUMERIC too, and in that situation the old logic apply.

@claytonjroberts
Copy link
Author

claytonjroberts commented Feb 18, 2025

Ah, I see what you mean about the documentation. I apologize, I should have scrolled down, lol.

For other readers:

Parameterized Type Description
NUMERIC(P[,S])DECIMAL(P[,S]) A NUMERIC or DECIMAL type with a maximum precision of P and maximum scale of S, where P and S are INT64 types. S is interpreted to be 0 if unspecified.Maximum scale range: 0 ≤ S ≤ 9Maximum precision range: max(1, S) ≤ P ≤ S + 29
BIGNUMERIC(P[, S])BIGDECIMAL(P[, S]) A BIGNUMERIC or BIGDECIMAL type with a maximum precision of P and maximum scale of S, where P and S are INT64 types. S is interpreted to be 0 if unspecified.Maximum scale range: 0 ≤ S ≤ 38Maximum precision range: max(1, S) ≤ P ≤ S + 38

But, because the visit_NUMERIC covers all cases of parameterized or non-parameterized NUMERIC and BIGNUMERIC cases, and when you provide parameterized value(s), it still uses the visit_NUMERIC function, and the logic inside is incorrect.

This lib will (incorrectly) try to use NUMERIC, even when the requirements for scale and precision are exceeded for the NUMERIC type (and thus BIGNUMERIC should be used).

@Linchin
Copy link
Contributor

Linchin commented Feb 18, 2025

Yeah, we need to change it so it covers both the default NUMERIC/BIGNUMERIC and the parameterized NUMERIC/BIGNUMERIC.

@claytonjroberts If you are interested, feel free to update your PR :) Otherwise we can take care of it too.

@claytonjroberts
Copy link
Author

I think it does, doesn't it? Can you explain what needs to be changed in my code? Happy to do it. :)

@Linchin
Copy link
Contributor

Linchin commented Feb 18, 2025

I will comment in the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API.
Projects
None yet
3 participants