dynamodb: fix Number attribute size calculation to use significant digits - #10182
Open
ylemiesa57 wants to merge 1 commit into
Open
dynamodb: fix Number attribute size calculation to use significant digits#10182ylemiesa57 wants to merge 1 commit into
ylemiesa57 wants to merge 1 commit into
Conversation
…gits DynamoType.size() computed a Number attribute's size as len(str(value)), the length of its decimal string representation. Real DynamoDB uses a compact encoding based on significant digits: approximately (1 byte per two significant digits) + (1 byte), with leading/trailing zeroes trimmed, per https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/CapacityUnitCalculations.html This made moto's 400KB item-size check stricter than real DynamoDB for any Number whose decimal string is longer than its significant-digit count (most numbers), rejecting items DynamoDB itself would accept. For example a 10-digit timestamp like 1786461547 was sized as 10 bytes instead of the documented 6. Adds _number_size(), verified against AWS's documented worked examples (27 -> 2 bytes, -27 -> 3 bytes, 461 -> 3 bytes) plus zero-trimming and negative-number cases, and a regression test for the originally reported timestamp case.
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.
Found this one while poking around moto's DynamoDB code, seemed like a good self-contained bug to learn the item-size logic a bit better.
Issue: #10176
What I changed
DynamoType.size()was computing a Number attribute's size aslen(str(value)), basically just the length of the decimal string. Real DynamoDB actually uses a more compact encoding based on significant digits, roughly 1 byte per two significant digits plus 1 byte, with leading and trailing zeros trimmed first. This is documented here: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/CapacityUnitCalculations.htmlSo moto was overcounting the size of most numbers, which made its 400KB item-size check stricter than the real thing and could reject items that real DynamoDB would accept. The issue's example was a 10-digit timestamp like
1786461547, sized as 10 bytes by moto instead of the documented 6.I added a small
_number_size()helper that strips the sign, drops the decimal point, trims leading/trailing zeros, and applies the "1 byte per 2 significant digits + 1 byte" formula, and swapped it in for the oldlen(str(value))call.Testing
tests/test_dynamodb/models/test_dynamo_type_size.pywith parametrized cases checked against AWS's own worked examples (27 -> 2 bytes, -27 -> 3 bytes, 461 -> 3 bytes, zero-trimming cases, the 38-significant-digit max), plus a direct regression test for the timestamp from the issue and a NumberSet sum-of-members check. 12 new tests, all passing.tests/test_dynamodb/suite (718 tests) locally, all still passing, including the existingtest_item_size_is_under_400KBtest.ruff check/ruff format --checkon the touched files come back clean (double-checked the pre-existing lint warnings elsewhere indynamo_type.pyare unrelated to this change, they're on upstream master too).mypyon the touched file: no issues.One honest caveat: I didn't try to reproduce the exact
put_itemcall from the issue end-to-end, since moto intentionally sets its internal size cap a bit below the real 400KB/409600-byte limit as a safety margin (there's a comment inLimitedSizeDictabout this), so the issue's specific byte counts don't line up perfectly with moto's cap either way. The actual bug, the wrong Number size formula, is what I focused on fixing and testing directly.Let me know if I should tweak anything, still getting familiar with this codebase.