Skip to content

fix(athena): preserve decimal precision with the Arrow backend - #3497

Merged
kukushking merged 4 commits into
aws:mainfrom
rioyu123:fix/athena-arrow-decimal-precision
Oct 25, 2026
Merged

kukushking merged 4 commits into
aws:mainfrom
rioyu123:fix/athena-arrow-decimal-precision

Conversation

@rioyu123

@rioyu123 rioyu123 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Feature or Bugfix

  • Bugfix

Detail

  • With dtype_backend="pyarrow", regular Athena CSV results and managed query results parse decimal(p,s) columns as double[pyarrow]. This can silently change values, for example 123456789012345.67 to 123456789012345.69, or raise ArrowInvalid for integers outside binary64's exact range.
  • Parse these columns with the existing Decimal converter, then convert them to decimal128(p, s)[pyarrow] using Athena's declared precision and scale. This preserves exact values across chunked reads, nulls, all-null and empty results. Explicit categories requests still take precedence.
  • On these paths, the resulting dtype intentionally changes from double[pyarrow] to Arrow decimal. The default numpy_nullable behavior, global type mapping, CTAS/UNLOAD reads, cached-result reads and write paths are unchanged.

Validation:

  • Added 52 regression/control cases through the public read_sql_query API. The tests use the real result metadata and CSV parsing code, with AWS calls and their boundary wrappers mocked. Against the base, 24 fail and 28 pass; all 52 pass with the fix.
  • These tests plus selected adjacent offline tests pass on both pandas 2.0.3 / PyArrow 12.0.1 and pandas 3.0.6 / PyArrow 25.0.1: 88 passed in each environment.
  • Repository Ruff, formatting and uv lock --check pass. Focused mypy on the two changed modules reports the same 10 errors as the base. Doc8 reports the same existing carriage-return errors on both trees.
  • No live AWS integration tests or full-repository mypy run were performed.

Relates

  • N/A

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kukushking kukushking left a comment

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.

Thanks, looks good. I restructured tests since it mixed unit and integration tests and replaced unit test monkeypatching with slightly cleaner patch obects.

@kukushking
kukushking merged commit 7ad2512 into aws:main Oct 25, 2026
32 checks passed
@rioyu123

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing and merging this, and for splitting the unit and integration tests. That makes the coverage much clearer.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants