Repository navigation
fix(import): check the numbers in each ImportTable row as the service does - #804
Open
LeeroyHannigan wants to merge 1 commit into
Open
LeeroyHannigan wants to merge 1 commit into
LeeroyHannigan wants to merge 1 commit into
Conversation
… does ImportTable stored a row's numbers as written. Each row's numbers are now read as BatchWriteItem reads them, before the other row checks: a value that is not a number ends the import of the file, and a number out of range skips the row. Either counts as one error, and an import with any error answers FAILED with ItemValidationError and keeps the rows it imported, as measured on DynamoDB on 2026-11-03. Each refused row is logged with the import ARN, the table name and its item number, with the error text escaped. A zero with an out-of-range exponent and a Unicode decimal digit are still read differently from the service; both are recorded in docs/differences-from-dynamodb.md.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
November 3, 2026 17:30
This branch has not been deployed
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.
Stacked on #620, so it merges after #620 and #619.
ImportTablestored the numbers in each row as written, so a row could store1E126,abcor1E999999999as anNvalue. Arithmetic on such a value is unbounded until #803 merges, and the SQLite sort-key encoder assumes every number is in range.This change checks every number in a row before the other row checks, as the service does:
BatchWriteItemreads them, with a 32-bit exponent and scale, so1E2147483648is not a number and1E2147483647is aNumber underflow.ImportStatusFAILEDwithFailureCodeItemValidationErrorand keeps the rows it imported. Before, it answeredCOMPLETEDwith an error count. The service'sFailureMessagesends the reader to its error log in CloudWatch; ExtendDB's names the server log.ImportTabledoes not check the table name; the access log line and the SQLite table worker still write it as given (fix(engine): escape the table name and path in the import/export access log #772 escapes the access log, and ImportTable creates a table under a name CreateTable refuses #771 tracks checking the name asCreateTabledoes).Measured on DynamoDB in us-east-1 on 2026-11-03, one import from S3 per case, each file holding a valid row, the row under test and another valid row. The mixed cases (a number out of range and a value that is not a number in one row, a row with no key, a five-row file) were measured the same way.
Two values are still read differently, and both are recorded in
docs/differences-from-dynamodb.md. A zero whose exponent fits in 32 bits but is out of range, such as0E126, is imported as0, where the service skips the row. A Unicode decimal digit (\uff15), which the service imports as5, ends the import of the file, since no number check in ExtendDB reads Unicode digits. The comment and test innumber.rsthat hold back range checking those zeros now name onlyQueryandScan(#544, #562).The file is still parsed before any row is imported, so a JSON error after the row that ends the import fails the request, where the service would stop before reading it. That case was not measured.
Tests: unit tests for
import_rowinoperations/tests.rs, andTestImportTableNumbersintests/test_import_export.py, which asserts the measured counts, rows and status. Both pass on PostgreSQL and SQLite. With the row check removed, 21 of the 22 import tests fail; the one that passes imports only valid numbers.