Repository navigation
fix: precision loss when decoding binary NUMERIC values - #285
Open
aminghadersohi wants to merge 1 commit into
Open
aminghadersohi wants to merge 1 commit into
aminghadersohi wants to merge 1 commit into
Conversation
numeric_in_binary returned Decimal(raw_value).scaleb(-scale), which rounds to the current decimal context (28 significant digits by default). NUMERIC values with more digits were silently rounded, e.g. NUMERIC(38,18) 12345678901234567890.123456789012345678 was read as 12345678901234567890.12345679. Scale with a 39-digit context, enough for any 16-byte value, so every digit is kept.
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
With the binary protocol,
numeric_in_binarydecodes NUMERIC values asDecimal(raw_value).scaleb(-scale).scalebrounds to the current decimal context, which is 28 significant digits by default. The fix passes a 39-digit context toscaleb. That is enough for any 8- or 16-byte NUMERIC value, so every digit is kept and the exponent is still-scale.Motivation and Context
A NUMERIC column with more than 28 significant digits is silently rounded on read:
This was reproduced against Redshift Serverless (1.0.434008). The value is stored exactly; psycopg2 reads it back exactly. Writes are unaffected because
numeric_outsends the full text form. There is no existing issue for this.Testing
test_numeric_in_binary_is_exact_beyond_28_digitsintest/unit/datatype/test_data_in.py. It covers 38-digit, negative, smallest-scale, 39-digit (int128 max) and 8-byte values, and asserts exact equality and the exponent. The existing numeric tests useisclose(rel_tol=1e-6), which cannot detect this.test_data_in.pypass.pytest test/unit: 2043 passed, 13 skipped. The 2 failures intest_browser_idc_auth_plugin.pyalso fail on the unmodified tree in that environment.Types of changes
Checklist
./build.shsucceeds (not run)pytest test/unitand they are passing (all pass except the 2 pre-existingtest_browser_idc_auth_plugin.pyfailures noted above)CHANGELOG.mdis not edited: it is maintained at release time.License