Repository navigation
fix(mongodb): store SET values as sent in the numeric ADD pipeline update - #589
Open
LeeroyHannigan wants to merge 2 commits into
Open
LeeroyHannigan wants to merge 2 commits into
LeeroyHannigan wants to merge 2 commits into
Conversation
…date
An unconditional UpdateItem on a MongoDB table with no index and no stream is
built as a native update while the server records that the table has no index.
With a numeric ADD it is an aggregation pipeline, and each SET value went into
its `$set` stage as it was. In a pipeline that value is an expression:
- A string that starts with `$` is a field path or a variable, so
`SET #s = :dollar ADD #n :one` with `:dollar` = `{"S": "$item_data"}`
stored the item_data document in `s.S`; the update and every later read of
the item answered InternalServerError, and so did every Query of its
partition and every Scan of its table. `$$NOW` stored a date and `$$REMOVE`
left `s` empty; `$`, a member named `$k` and a member whose name holds `.`
made MongoDB refuse the update.
- A document value merges into the embedded document it replaces, so a SET of
a map over a map kept the old members, and a SET that changed the
attribute's type stored two type tags and answered InternalServerError.
Each SET value in the pipeline stage is now wrapped in `$literal`, which
stores it as a constant. Amazon DynamoDB stores every SET value as sent and
replaces the attribute it overwrites, with or without the ADD (measured
2026-10-25 at 23:15 UTC and 2026-10-26 at 00:55 UTC, each case twice).
…meric ADD Dual-target wire tests from the 2026-10-25 23:15 UTC and 2026-10-26 00:55 UTC measurements: strings `$item_data`, `$$ROOT`, `$` and `$$REMOVE`, a string set and a list holding them, maps whose values hold them, maps with members named `$k`, `$literal` and names holding `.`, and the other scalar types, each stored as sent; a SET over an attribute the item already holds, including a type change; and a REMOVE in the same update as the ADD. Each test uses a table of its own and writes an item before its update, so on MongoDB the updates take the native update path.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 26, 2026 01:23
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.
Fixes #573.
On MongoDB, an UpdateItem that SETs a value and also ADDs a number could store something other than the value sent. A
$string was read as an expression and a map merged into the map it replaced, and several of these left an item every read answered with InternalServerError. The value is now stored as sent.What happened on main
An unconditional UpdateItem on a table with no index and no stream is built as a native MongoDB update while the server records that the table has no index (60 seconds after a write). With a numeric ADD,
try_build_native_updateincrates/storage-mongodb/src/data_engine.rsbuilds an aggregation pipeline and put each SET value into its$setstage as it was. In a pipeline that value is an expression:$is a field path or a variable.{"S": "$item_data"}stored theitem_datadocument ins.S, which no longer converts back to an item (S value must be a stringin the server log), and$$NOWstored a date. A string set, a list or a map value holding such a string did the same. The update answered InternalServerError, every later GetItem of the item did too, and so did every Query of its partition and every Scan of its table, because one unreadable item fails the page.$$REMOVEleftsas an empty document.$, a map member named$k,$literalor$add, and a map member whose name holds.made MongoDB refuse the update, which answered InternalServerError and left the item unchanged.A SET without a numeric ADD uses an operator update, where values are literal, and was not affected.
The change
Each SET value in the pipeline stage is wrapped in
$literal, which makes it a constant. The code change is one line indata_engine.rs, plus the comment beside it.What the service answers
Measured in us-east-1, each case twice: with
SET #s = :s ADD #n :oneon 2026-10-25 at 23:15 UTC, and without the ADD, over existing attributes and with REMOVE on 2026-10-26 at 00:55 UTC. The service stores every SET value as sent and replaces the attribute it overwrites, with or without the ADD:$item_data,$$ROOT,$,$$REMOVE, the text{"$literal": 1}, a string set and a list holding$strings, a map whose value is$item_data.pk, maps with members named$k,$literal,a.band$c.d, and N, BOOL, NULL and B values. A REMOVE in the same update as the ADD removes its attribute.Tests
tests/test_update_item_dollar_values.py: 39 dual-target wire tests. The 15 values with and without the ADD, four SETs over an attribute the item holds (map over map, nested map over nested map, a type change from S to N and from L to M) with and without the ADD, and a REMOVE in the same update as the ADD. Each test uses a table of its own and writes an item right before its update, so on MongoDB the update runs on the native path.Run locally against MongoDB 7 (single-node replica set on a loopback port):
main: 15 failed, 24 passed. The 15 are the updates with the ADD whose value holds a$string, a$or dotted member name, or overwrites an existing attribute, plus the REMOVE with the ADD; every update without the ADD passes onmain.tests/: 91 passed, 1 skipped.SET #s = :dollar ADD #n :onecalls to one key end withn= 1280 andsas sent, every call on the pipeline path.$fails the four SET-over-existing tests with the ADD, and wrapping the$$REMOVEof a REMOVE fails the REMOVE test.cargo +1.97.0 fmt --all -- --check,cargo +1.97.0 clippy --all-targets -- -D warnings,cargo test --workspaceon stable and on 1.88.0 (1,767 passed each), and the MongoDB--comprehensivesuite (331 passed).CI runs the new file in
run-integration,run-integration-sqlite,mongodb-pytestandcassandra-pytest.Not changed here
When SET or REMOVE and ADD in one update name the same attribute, the service answers ValidationException
Two document paths overlap. ExtendDB does not refuse it: MongoDB answers InternalServerError and SQLite applies both. That is onmainand unchanged.Files
crates/storage-mongodb/src/data_engine.rsis 4,431 lines onmainand grows by three, the comment. 12 of the 13 open PRs that change it merge clean with this branch; #416 conflicts withmainalready.