Skip to content

fix(engine): validate numbers on every key and item path - #620

Open
LeeroyHannigan wants to merge 7 commits into
fix/616-number-exponent-overflowfrom
fix/618-number-validation-all-ops
Open

LeeroyHannigan wants to merge 7 commits into
fix/616-number-exponent-overflowfrom
fix/618-number-validation-all-ops

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #618. Stacked on #619 (target branch fix/616-number-exponent-overflow), whose exponent rules these checks use.

What changed

Only the PutItem item and UpdateItem's update values ran number validation. AttributeValue keeps an N value it cannot read as written (crates/core/src/types/attribute_value.rs:141-144), so every other operation used values such as abc and 1E126 as written: BatchWriteItem and TransactWriteItems stored them, GetItem and DeleteItem looked them up, and an N partition key written that way became a partition of its own.

This PR checks the N and NS values on these paths, with the text DynamoDB gives on each:

Path Message on DynamoDB and here
GetItem, BatchGetItem and TransactGetItems keys the number message, no prefix
DeleteItem and UpdateItem keys, PutItem and DeleteItem ExpressionAttributeValues 1 validation error detected: and the number message
BatchWriteItem puts (keys, attributes, NS members, nested L and M) and delete keys the number message, no prefix
TransactWriteItems Put items, Update, Delete and ConditionCheck keys the number message, no prefix
TransactWriteItems ExpressionAttributeValues ExpressionAttributeValues contains invalid value: <message> for key :v

The new checks are in crates/core/src/validation/number/operations.rs, with one or two call sites in each of eight engine handlers. validation/mod.rs is not changed.

BatchWriteItem, TransactGetItems and TransactWriteItems read a number with a 32-bit exponent and scale on DynamoDB, and the other operations with a 64-bit exponent. So 10E2147483647 is Number underflow on the first three and Number overflow on the rest, and 1E2147483648 cannot be converted on the first three. validate_number_i32_scale reproduces the 32-bit reading, and a unit test runs both readers over 4,000 generated strings and fails if they answer differently outside the four documented differences.

Order against a second fault

Each check is placed where DynamoDB reports it, unless that position would change the answer to a two-fault request that main already answers as DynamoDB does:

  • GetItem, DeleteItem, UpdateItem, BatchGetItem and BatchWriteItem report a malformed number ahead of the table lookup; PutItem reports item numbers ahead of the mixed-parameter check and the lookup, and value numbers after the mixed-parameter check.
  • TransactGetItems checks each item's key right after its projection; TransactWriteItems checks each action right after its expressions are parsed. The ClientRequestToken check now runs before the actions, as on DynamoDB.
  • Where DynamoDB reports the number first and main reports the other fault, this branch keeps main's answer: a duplicate item or a missing table in TransactGetItems, a missing table in an earlier TransactWriteItems action, and an expression fault in the same action.
  • BatchWriteItem and BatchGetItem visit tables in name order. DynamoDB's order for two tables was neither body nor name order on four sends, and not stable for BatchGetItem.
  • In a PutItem item, numbers are checked before nesting depth, as on main. DynamoDB follows the attribute order of the request body, which Item does not keep.

Measured

DynamoDB in us-east-1, 2026-10-27, every request sent twice with the same answer both times: 20 paths with abc, 1E126, 0E126 and a control; 31 paths with 24 values; three probes of two-fault order; token, exponent marker and fraction rows. A refused batch or transaction writes nothing on DynamoDB and on this branch; main stored the whole request, invalid values included.

Against main, on the same probes: no request that main answers as DynamoDB does is answered differently here, by exact text or with the 1 validation error detected: prefix ignored, apart from Unicode digits below.

Related behaviour

  • Zeros such as 0E126 and 0.0E-130 are refused on DynamoDB and stay accepted as 0. AttributeValue normalizes them before any check runs, and refusing them there makes Query pk = 0E126 miss the item at 0 and ImportTable store 0E126 as written, because Query, Scan (fix(query): check Query values against the key schema and the number rules #544, fix(scan): check the numbers in ExpressionAttributeValues as the service does #562) and ImportTable do not validate numbers yet. BatchWriteItem, TransactWriteItems and other operations store N values that fail number validation as written #618 stays open for them. The unit test zero_with_i32_exponent_stays_accepted_until_query_scan_and_import_validate marks the place to change.
  • Unicode decimal digits (5, ١) are accepted by DynamoDB and stored as ASCII. Main refuses them on PutItem and stored them as written on the paths it did not check; this branch refuses them on every path it checks. Unicode decimal digits in N values: refused on writes, compared as text on reads #565 covers reading them.
  • An NS with two equal malformed members answers the duplicates error, where DynamoDB answers the number: the duplicate check runs while AttributeValue is read.
  • crates/server/src/leading_keys.rs authorizes a BatchGetItem or BatchWriteItem against the keys it can read and leaves out one it cannot. The engine now refuses such a request before any data is read or written, and that refusal is what keeps the partial key set from reaching data.
  • ImportTable imports an invalid N as written; DynamoDB's answer there was not measured. An item already stored under such a key, by ImportTable or by a BatchWriteItem or TransactWriteItems before this change, can no longer be read, updated or deleted through GetItem, UpdateItem, DeleteItem or the batch and transaction paths above, which refuse the key before the lookup. Scan still returns it.
  • PutItem item errors and UpdateItem ExpressionAttributeValues keep their current text.

Tests

  • Unit: 18 tests in operations/tests.rs. In number.rs the two zero tests gain the measured rows and the guard test is renamed.
  • Wire: number_validation_all_ops.rs (27 tests) and number_validation_order.rs (11). Against SQLite dev-mode servers, the number_validation filter passes 50 of 50 here and fails 31 on fix(core): range check number exponents as DynamoDB does #619's head.
  • Full wire suite, one thread, with main at abf8082: 4 tests fail here and 35 on fix(core): range check number exponents as DynamoDB does #619's first revision; none fails here that passes there. The 4 fail on both (capacity throttling and a vector index test).
  • cargo fmt --all -- --check and cargo +1.97.0 clippy --all-targets -- -D warnings are clean. With main at abf8082, cargo test --workspace --exclude extenddb-storage-cassandra: 1,817 passed, and cargo test -p extenddb-storage-cassandra --lib: 61 passed. On the rebased branch, cargo +1.88.0 test -p extenddb-core: 721 passed.

Merging with open PRs

With main at abf8082, 13 of the 17 then open PRs that touched these files merged cleanly. #433 conflicted in put_item.rs and transact_write_items.rs; both sides can be kept. #399, #507 and #597 conflicted in delete_item.rs and put_item.rs on comment lines, where both sides can be kept, and in transact_write_items.rs, where #399 splits the action loop into phases. There the per-action check goes at the head of phase 1's per-item step; with it, three rows where this branch answers the number ahead of a nested value too deep move to the nesting error, because #399 checks nesting in a later phase.

Rebase on #619 at bfde5fd

#619 was rebased onto main 817b240 after this branch was cut, and this branch is now stacked on #619's new head instead of its first revision (95997b6). The two commits of this PR apply to the new base without conflicts and with the same patch IDs, so their code is unchanged. The conflict GitHub showed in crates/core/src/validation/number.rs came from #619's first revision, which this branch no longer carries.

The new base adds 15 main commits. Two of them touch files this PR changes or depends on:

Main is now at f49c5f8, seven commits past 817b240 (#395, #412, #638, #511, #680, #473, #429). None of them changes a file this PR touches, and this branch merges with it without conflicts.

On the rebased branch, the number_validation filter passes 50 of 50 against a SQLite server and a PostgreSQL server built from it. With only this PR's wire files applied to #619's new head it fails 31 of 50, the same counts as before the rebase. tests/test_update_item_number_validation.py and tests/test_item_operations.py pass 134 of 134 on each backend. cargo +1.88.0 test -p extenddb-core passes 721 tests.

CI passed all 29 checks on 41bc6e43a.

Only the PutItem item and UpdateItem's update values were checked.
AttributeValue keeps a number it cannot read as written, so GetItem,
DeleteItem and UpdateItem keys, BatchGetItem and TransactGetItems keys,
BatchWriteItem writes, every TransactWriteItems action, and PutItem and
DeleteItem condition values used values such as abc and 1E126 as
written, where DynamoDB refuses them with a ValidationException.

Each of those paths now checks its N and NS values with the text that
operation answers. PutItem, UpdateItem and DeleteItem start the message
with "1 validation error detected: ", the other operations do not, and
a TransactWriteItems value names its placeholder. BatchWriteItem,
TransactGetItems and TransactWriteItems read a number with a 32-bit
exponent and scale, as DynamoDB does on those operations, so
10E2147483647 is a Number underflow there and a Number overflow on the
others.

Each check is placed where DynamoDB reports it, unless that position
would change the answer to a request with a second fault that main
already answers as DynamoDB does. The ClientRequestToken check now runs
before the actions, as on DynamoDB.

A zero whose exponent fits in an i32, such as 0E126, stays accepted as
0 until Query, Scan and ImportTable validate numbers. A zero whose
exponent does not fit is range checked on its magnitude, which makes
0E9223372036854775807 a Number underflow, as on DynamoDB.

Part of #618
number_validation_all_ops covers each validated path with DynamoDB's
message for abc, 1E126 and the 32-bit edge values, a refused batch or
transaction writing nothing, and valid zeros stored as 0.
number_validation_order pins the position of each check against a
second fault in the same request: an absent table, a mixed-parameter
error, an unused name, a projection or condition syntax error in another
item, a duplicate key and a long ClientRequestToken. Each expectation
is a DynamoDB answer measured twice on 2026-10-27.

Part of #618
@LeeroyHannigan
LeeroyHannigan force-pushed the fix/618-number-validation-all-ops branch from 909482b to 41bc6e4 Compare October 31, 2026 17:00
@LeeroyHannigan LeeroyHannigan added the v1 Needed for the 1.0 release label Nov 3, 2026
… comparison

SET a = b + c, SET a = b - c and a number ADD parsed both operands with
bigdecimal and range checked only the result. bigdecimal aligns operands
to the larger scale before adding them, so the cost of a sum grows with
the gap between their exponents, and an operand read from an item stored
without the number checks (#618) could build an integer of about 10^9
digits.

Each operand is now range checked and parsed from its normalized text. A
stored operand that is not a number answers the service's text without
the stored value. The result is stored as the service spells it (2.25 -
0.25 is 2, not 2.00), and a result out of range keeps the Number overflow
answer, as measured on DynamoDB on 2026-11-03.

Numbers in condition and filter comparisons are parsed the same way,
which bounds bigdecimal's quadratic parse for values that are not checked
before the expression maps are built. A placeholder the check refuses is
recorded once per request, and a number the service cannot store compares
as a value of another type, so only <> is true for it.
… 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 and others added 3 commits November 3, 2026 22:42
The MongoDB fast path for a number ADD adds a Decimal128 in an update
pipeline and does not read the stored value first. A Decimal128 keeps 34
digits and holds magnitudes DynamoDB refuses. After 5E125 + 5E125,
GetItem returned 1.000000000000000000000000000000000E+126, where the
update evaluator answers Number overflow. After 38 nines + 1, it
returned 99999999999999999999999999999999990000, where the exact sum is
1E38.

The fast path now takes a delta whose normalized text is an integer of
at most 33 digits, or has at most 20 digits before the decimal point and
13 after it. Its filter admits a stored value written without an
exponent, within limits that keep the sum within 34 digits: an integer
of up to 33 digits when both operands are integers, and up to 20 and 13
digits when either has a fraction. Any other ADD takes the session path,
where the update evaluator range checks both operands and the sum. The
delta is the Decimal128 of its normalized text, so 1E3 is added as 1000.

On MongoDB 7, fifty threads sending 20 ADD calls each to one item, with
SDK retries off, gave no errors and the exact total for deltas of 1, 0.5
and 0.001, for 1 added to a stored 0.5 and to a stored 21-digit integer,
and for a 25-digit delta.
fix(expression): range check numbers before bigdecimal arithmetic and comparison
fix(import): check the numbers in each ImportTable row as the service does

This branch has not been deployed

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

Labels

v1 Needed for the 1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants