Fix deserialization of numbers with trailing zeros from DynamoDB by normalizing first #4694
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in DynamoDB number deserialization where numbers with trailing zeros that exceed 38 total digits (but have ≤38 significant digits) would fail to deserialize. The fix normalizes the decimal representation to remove trailing zeros before applying DynamoDB's 38-digit precision constraint.
Key Changes:
- Modified
_deserialize_n()to normalize decimal values before creating them in DYNAMODB_CONTEXT - Added test case for deserializing numbers with trailing zeros
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| boto3/dynamodb/types.py | Updated _deserialize_n method to normalize decimal values before creating them with DYNAMODB_CONTEXT, removing trailing zeros that would otherwise cause precision errors |
| tests/unit/dynamodb/test_types.py | Added test case to verify deserialization of large numbers with trailing zeros works correctly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def test_deserialize_decimal(self): | ||
| assert self.deserializer.deserialize({'N': '1.25'}) == Decimal('1.25') | ||
| assert self.deserializer.deserialize({'N': '1234567891234560000000000000000000000000'}) == Decimal('1.23456789123456E+39') |
There was a problem hiding this comment.
The test uses a different number than the one mentioned in the linked issue (#4693). The issue reports 1234567895171680000000000000000000000000 but this test uses 1234567891234560000000000000000000000000. Consider adding a test case with the exact number from the issue to ensure it's properly fixed.
|
|
||
| def test_deserialize_decimal(self): | ||
| assert self.deserializer.deserialize({'N': '1.25'}) == Decimal('1.25') | ||
| assert self.deserializer.deserialize({'N': '1234567891234560000000000000000000000000'}) == Decimal('1.23456789123456E+39') |
There was a problem hiding this comment.
Consider adding test cases for additional edge cases with trailing zeros to ensure comprehensive coverage:
- Numbers with trailing zeros after the decimal point (e.g., '1.2500')
- Negative numbers with trailing zeros (e.g., '-1234567891234560000000000000000000000000')
- Numbers already in scientific notation (e.g., '1.23E+39')
- Very small numbers with trailing zeros (e.g., '0.00001000')
|
There is already a pull request for this without needing the normalization step: #2913 I highly recommend it be considered for reopening. |
|
@JustinTArthur not sure if the maintainers see comments on closed issues/PRs but yeah I think getting rid of This library should be able to read all values that DynamoDB accepts. Really there shouldn't be any validation of values coming from DynamoDB. |
2 participants
Fixes #4693
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.