Skip to content

fix: Random row validation issues for large decimal primary keys - #1824

Open
nj1973 wants to merge 17 commits into
developfrom
1822-random-row-fails-for-large-decimal-primary-keys
Open

nj1973 wants to merge 17 commits into
developfrom
1822-random-row-fails-for-large-decimal-primary-keys

Conversation

@nj1973

@nj1973 nj1973 commented Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

Description of changes

This PR fixes three issues with random row validation issues for large decimal primary keys

1. Preserve Decimal PK Types in Random Row Filters

Added _prepare_pk_filter_values in data_validation/data_validation.py to coerce sampled PK values into decimal.Decimal when the column type is decimal, preventing Ibis int64 bounds errors when drivers return large integers.

2. Support Large Decimal Literals in BigQuery

Implemented a custom format_literal translator for BigQuery in third_party/ibis/ibis_bigquery/registry.py that formats decimals as fixed-point strings (avoiding invalid scientific notation) and emits BIGNUMERIC when integer precision exceeds standard NUMERIC limits.

3. Prevent Decimal-to-Float Precision Loss in SQLAlchemy Backends

Overrode BaseAlchemyBackend.fetch_from_cursor (_dvt_fetch_from_cursor in third_party/ibis/ibis_addon/operations.py) with coerce_float=False to prevent pandas from casting Decimal objects to float64 and losing digits beyond 15-17 places.

Issues to be closed

Closes #1822

Checklist

  • I have followed the CONTRIBUTING Guide.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated any relevant documentation to reflect my changes, if applicable
  • I have added unit and/or integration tests relevant to my change as needed
  • I have already checked locally that all unit tests and linting are passing (use the tests/local_check.sh script)
  • I have manually executed end-to-end testing (E2E) with the affected databases/engines

@nj1973 nj1973 linked an issue Sep 3, 2026 that may be closed by this pull request
@nj1973
nj1973 marked this pull request as ready for review September 3, 2026 15:00
@pull-request-size pull-request-size Bot added size/XL and removed size/L labels Sep 3, 2026
@sundar-mudupalli-work

Copy link
Copy Markdown
Collaborator

Neil,

If there isn't a customer waiting on this, I suggest we pause this change. See my comments in PR 1823.

Sundar Mudupalli

@nj1973

nj1973 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

/gcbrun

decimal.Decimal(str(_))
if not pandas.isna(_) and isinstance(_, (int, float, str))
else _
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neil,

I want to review this code a bit carefully - specifically lines 154 to 159. Looking at the code, it appears that for a decimal type pandas could be returning a float and you may be converting it to decimal. That could result in a loss of precision and create the same issue that we had. There are a couple of ways to avoid this - materialize the results in pyArrow - which will give you the real decimal types and you don't have a conversion and/or specify that ibis use the PyArrow data types instead (available in Pandas 2.x).

On the other hand, you may have come up with a good workaround. I want to test it with different data values to ensure I am not missing something. Apologies, I could not get to it today.

Sundar Mudupalli

@nj1973 nj1973 Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, no problem. In case it helps:

  • The change described in item 3 in the PR description is intended to prevent exactly what you describe, to stop Pandas coercing decimals to floats.
  • Reviewing SQL for pso_data_validator.dvt_large_decimals will show you the types of values we already test with. Previously the test was flawed but I believe it can now be trusted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked an AI agent for a second opinion on my work and it flagged a potential further issue that probably requires a new issue:

Oracle can still deliver floats. By default, python-oracledb returns NUMBER(p,s>0) as float (fetch_decimals defaults to False). So an Oracle decimal key with a fractional part and more than ~15 significant digits loses precision inside the driver, unrelated to Pandas. coerce_float=False can't fix that.

So we probably need a new test table with fractional primary key values and a new issue. I would prefer for that to not be combined with this fix which I believe is independent.

@nj1973

nj1973 commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/gcbrun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Random Row fails for DECIMAL primary keys with 31 digits

2 participants