Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions crates/core/src/expression/update_add_parent_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright 2026 ExtendDB contributors
// SPDX-License-Identifier: Apache-2.0

//! ADD on a nested path needs every element above the leaf to exist with the
//! container type the next element needs (measured 2026-10-26).

use super::*;
use crate::expression::tokenizer::tokenize;
use crate::expression::update_parser::parse_update;
use std::collections::HashMap;

const INVALID: &str = "The document path provided in the update expression is invalid for update";

fn n(v: &str) -> AttributeValue {
AttributeValue::N(v.to_owned())
}

fn m(entries: &[(&str, AttributeValue)]) -> AttributeValue {
AttributeValue::M(
entries
.iter()
.map(|(k, v)| ((*k).to_owned(), v.clone()))
.collect(),
)
}

fn add(expr: &str, item: &mut BTreeMap<String, AttributeValue>) -> Result<(), DynamoDbError> {
let actions = parse_update(&tokenize(expr)?)?;
let maps = ExpressionMaps::new(HashMap::new(), HashMap::from([("one".to_owned(), n("1"))]));
apply_update(&actions, item, &maps)
}

/// The callers discard the image on an error, so only the error is checked.
fn refused(expr: &str, mut item: BTreeMap<String, AttributeValue>) {
match add(expr, &mut item) {
Err(DynamoDbError::ValidationException(msg)) => assert_eq!(msg, INVALID, "{expr}"),
other => panic!("{expr}: {other:?}"),
}
}

#[test]
fn a_member_of_an_existing_map_is_added() {
let mut item = BTreeMap::from([("m".to_owned(), m(&[]))]);
add("ADD m.a :one", &mut item).unwrap();
assert_eq!(item["m"], m(&[("a", n("1"))]));
}

#[test]
fn a_missing_parent_is_refused() {
refused("ADD z.a :one", BTreeMap::new());
refused("ADD m.a.b :one", BTreeMap::from([("m".to_owned(), m(&[]))]));
refused(
"ADD x :one, z.a :one",
BTreeMap::from([("x".to_owned(), n("1"))]),
);
}

#[test]
fn a_parent_that_is_not_a_map_is_refused() {
let list = AttributeValue::L(vec![n("1")]);
refused("ADD s.a :one", BTreeMap::from([("s".to_owned(), n("1"))]));
refused(
"ADD l.a :one",
BTreeMap::from([("l".to_owned(), list.clone())]),
);
refused("ADD l[5].a :one", BTreeMap::from([("l".to_owned(), list)]));
refused(
"ADD m[0] :one",
BTreeMap::from([("m".to_owned(), m(&[("a", n("1"))]))]),
);
}

/// A list of maps, so an index clamped to the last element would be accepted.
#[test]
fn an_index_past_the_end_of_a_list_of_maps_is_refused() {
let maps = AttributeValue::L(vec![m(&[])]);
refused("ADD l[5].a :one", BTreeMap::from([("l".to_owned(), maps)]));
refused(
"ADD l[0].a :one",
BTreeMap::from([("l".to_owned(), AttributeValue::L(Vec::new()))]),
);
}

#[test]
fn a_map_inside_a_list_is_reached() {
let mut item = BTreeMap::from([("l".to_owned(), AttributeValue::L(vec![m(&[])]))]);
add("ADD l[0].a :one", &mut item).unwrap();
assert_eq!(item["l"], AttributeValue::L(vec![m(&[("a", n("1"))])]));
refused(
"ADD l[0].a.b :one",
BTreeMap::from([("l".to_owned(), AttributeValue::L(vec![m(&[])]))]),
);
}
72 changes: 29 additions & 43 deletions crates/core/src/expression/update_evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ fn apply_add(
value: AttributeValue,
maps: &ExpressionMaps,
) -> Result<(), DynamoDbError> {
let target = navigate_to_parent_map_or_create(item, path, maps)?;
let target = navigate_to_existing_parent_map(item, path, maps)?;
let attr_name = resolve_attr_name(
path.last()
.ok_or_else(|| DynamoDbError::ValidationException("Empty path in ADD".to_owned()))?,
Expand Down Expand Up @@ -369,61 +369,43 @@ fn apply_delete(
Ok(())
}

/// Navigate a document path to the parent map of the leaf element, creating
/// intermediate maps as needed. Used by ADD which must create the path if absent.
/// Navigate a document path to the parent map of the leaf element for ADD.
/// Every element above the leaf must exist with the container type the next
/// element needs: ADD creates no missing map (measured 2026-10-26).
///
/// For a single-element path, returns the item itself.
fn navigate_to_parent_map_or_create<'a>(
fn navigate_to_existing_parent_map<'a>(
item: &'a mut BTreeMap<String, AttributeValue>,
path: &[PathElement],
maps: &ExpressionMaps,
) -> Result<&'a mut BTreeMap<String, AttributeValue>, DynamoDbError> {
let invalid = || {
DynamoDbError::ValidationException(
"The document path provided in the update expression is invalid for update".to_owned(),
)
};
if path.len() <= 1 {
return Ok(item);
}
let first_name = resolve_attr_name(&path[0], maps)?;
let mut current = item
.entry(first_name)
.or_insert_with(|| AttributeValue::M(BTreeMap::new()));
.get_mut(&resolve_attr_name(&path[0], maps)?)
.ok_or_else(invalid)?;
for element in &path[1..path.len() - 1] {
match element {
PathElement::Attribute(_) => {
let name = resolve_attr_name(element, maps)?;
if let AttributeValue::M(map) = current {
current = map
.entry(name)
.or_insert_with(|| AttributeValue::M(BTreeMap::new()));
} else {
return Err(DynamoDbError::ValidationException(
"The document path provided in the update expression is invalid for update"
.to_owned(),
));
}
}
PathElement::Index(idx) => {
if let AttributeValue::L(list) = current {
if *idx < list.len() {
current = &mut list[*idx];
} else {
return Err(DynamoDbError::ValidationException(
"The provided expression refers to an attribute that does not exist in the item"
.to_owned(),
));
}
} else {
return Err(DynamoDbError::ValidationException(
"The document path provided in the update expression is invalid for update"
.to_owned(),
));
}
current = match (element, current) {
(PathElement::Attribute(_), AttributeValue::M(map)) => map
.get_mut(&resolve_attr_name(element, maps)?)
.ok_or_else(invalid)?,
(PathElement::Index(idx), AttributeValue::L(list)) => {
list.get_mut(*idx).ok_or_else(invalid)?
}
}
_ => return Err(invalid()),
};
}
match current {
AttributeValue::M(map) => Ok(map),
_ => Err(DynamoDbError::ValidationException(
"The document path provided in the update expression is invalid for update".to_owned(),
)),
// A list element as the leaf is not supported here: `ADD l[0] :n` is
// refused for every parent, the service's answer when the parent is a map.
match (path.last(), current) {
(Some(PathElement::Attribute(_)), AttributeValue::M(map)) => Ok(map),
_ => Err(invalid()),
}
}

Expand Down Expand Up @@ -640,3 +622,7 @@ fn resolve_path_to_value<'a>(
#[cfg(test)]
#[path = "update_evaluator_tests.rs"]
mod tests;

#[cfg(test)]
#[path = "update_add_parent_tests.rs"]
mod add_parent_tests;
155 changes: 155 additions & 0 deletions tests/test_update_add_missing_parent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# Copyright 2026 ExtendDB contributors
# SPDX-License-Identifier: Apache-2.0

"""UpdateExpression ADD on a nested path whose parent is missing.

Amazon DynamoDB adds to a member of a map that exists, but creates no missing
map on the way: every element above the leaf must exist and be the container
the next element needs (a map for a name, a list for an index). Otherwise the
update answers ValidationException ``The document path provided in the update
expression is invalid for update`` and writes nothing, the other actions of
the request included. A failing condition is reported first. Measured on
2026-10-26 at 02:38, 02:54 and 03:33 UTC (us-east-1), each case twice.

Dual-target against Amazon DynamoDB and extenddb.
"""

from __future__ import annotations

import re

import pytest
from botocore.exceptions import ClientError

from conftest import scoped_table

INVALID = "The document path provided in the update expression is invalid for update"
ONE = {":one": {"N": "1"}}
SS = {":ss": {"SS": ["a"]}}


def names_of(*texts: str) -> dict:
return {t: t[1:] for text in texts for t in re.findall(r"#\w+", text)}


@pytest.fixture(scope="module")
def table(dynamodb_client):
with scoped_table(
dynamodb_client,
attribute_definitions=[
{"AttributeName": "pk", "AttributeType": "S"},
{"AttributeName": "sk", "AttributeType": "S"},
],
key_schema=[
{"AttributeName": "pk", "KeyType": "HASH"},
{"AttributeName": "sk", "KeyType": "RANGE"},
],
) as name:
yield name


def _key(case: str) -> dict:
return {"pk": {"S": "add-parent"}, "sk": {"S": case}}


def _put(client, table, case, attrs):
key = _key(case)
if attrs is not None:
client.put_item(TableName=table, Item={**key, "x": {"S": "x"}, **attrs})
return key


def _stored(client, table, key):
item = client.get_item(TableName=table, Key=key, ConsistentRead=True).get("Item")
return None if item is None else {k: v for k, v in item.items() if k not in ("pk", "sk")}


def _update(client, table, key, expr, values=ONE, **kw):
return client.update_item(
TableName=table, Key=key, UpdateExpression=expr,
ExpressionAttributeNames=names_of(expr, kw.get("ConditionExpression", "")),
ExpressionAttributeValues=values, **kw,
)


EMPTY_MAP = {"m": {"M": {}}}
LIST_OF_ONE = {"l": {"L": [{"N": "1"}]}}

# (case, item attributes or None for no item, update expression, values)
REFUSED = [
("two_missing_maps", EMPTY_MAP, "ADD #m.#a.#b :one", ONE),
("missing_top", {}, "ADD #z.#a :one", ONE),
("missing_top_set", {}, "ADD #z.#a :ss", SS),
("absent_item", None, "ADD #z.#a :one", ONE),
("parent_is_string", {"s": {"S": "str"}}, "ADD #s.#a :one", ONE),
("parent_is_number", {"s": {"N": "1"}}, "ADD #s.#a :one", ONE),
("parent_is_list", LIST_OF_ONE, "ADD #l.#a :one", ONE),
("map_by_index", {"m": {"M": {"a": {"N": "1"}}}}, "ADD #m[0] :one", ONE),
("index_of_string", {"s": {"S": "str"}}, "ADD #s[0] :one", ONE),
("index_of_missing", {}, "ADD #z[0] :one", ONE),
("under_index_past_end", LIST_OF_ONE, "ADD #l[5].#a :one", ONE),
("under_list_member_missing_map", {"l": {"L": [{"M": {}}]}}, "ADD #l[0].#a.#b :one", ONE),
("index_past_end_of_maps", {"m": {"M": {"l": {"L": [{"M": {}}]}}}}, "ADD #m.#l[3].#a :one", ONE),
("index_of_empty_list", {"l": {"L": []}}, "ADD #l[0].#a :one", ONE),
("with_set_before", {}, "SET #x = :one ADD #z.#a :one", ONE),
("with_set_after", {}, "ADD #z.#a :one SET #x = :one", ONE),
("second_of_two", {"x": {"N": "1"}}, "ADD #x :one, #z.#a :one", ONE),
]


@pytest.mark.parametrize(("case", "attrs", "expr", "values"), REFUSED, ids=[c[0] for c in REFUSED])
def test_add_under_a_missing_parent_is_refused(dynamodb_client, table, case, attrs, expr, values):
key = _put(dynamodb_client, table, f"no-{case}", attrs)
before = _stored(dynamodb_client, table, key)
with pytest.raises(ClientError) as exc_info:
_update(dynamodb_client, table, key, expr, values)
err = exc_info.value.response["Error"]
assert (err["Code"], err["Message"]) == ("ValidationException", INVALID)
assert _stored(dynamodb_client, table, key) == before


# (case, item attributes, update expression, values, attribute changed, its value after)
APPLIED = [
("member_of_existing_map", EMPTY_MAP, "ADD #m.#a :one", ONE, "m", {"M": {"a": {"N": "1"}}}),
("set_member_of_existing_map", EMPTY_MAP, "ADD #m.#a :ss", SS, "m", {"M": {"a": {"SS": ["a"]}}}),
("member_of_map_in_list", {"l": {"L": [{"M": {}}]}}, "ADD #l[0].#a :one", ONE,
"l", {"L": [{"M": {"a": {"N": "1"}}}]}),
]


@pytest.mark.parametrize(("case", "attrs", "expr", "values", "name", "after"), APPLIED,
ids=[c[0] for c in APPLIED])
def test_add_under_an_existing_map_is_applied(dynamodb_client, table, case, attrs, expr, values, name, after):
key = _put(dynamodb_client, table, f"ok-{case}", attrs)
_update(dynamodb_client, table, key, expr, values)
assert _stored(dynamodb_client, table, key)[name] == after


def test_a_failing_condition_is_reported_first(dynamodb_client, table):
key = _put(dynamodb_client, table, "cond-fails", {})
with pytest.raises(ClientError) as exc_info:
_update(dynamodb_client, table, key, "ADD #z.#a :one", ConditionExpression="attribute_exists(#b)")
assert exc_info.value.response["Error"]["Code"] == "ConditionalCheckFailedException"


def test_a_passing_condition_does_not_change_the_answer(dynamodb_client, table):
key = _put(dynamodb_client, table, "cond-passes", {})
with pytest.raises(ClientError) as exc_info:
_update(dynamodb_client, table, key, "ADD #z.#a :one", ConditionExpression="attribute_exists(#x)")
err = exc_info.value.response["Error"]
assert (err["Code"], err["Message"]) == ("ValidationException", INVALID)
assert _stored(dynamodb_client, table, key) == {"x": {"S": "x"}}


def test_transact_write_items_update_is_cancelled(dynamodb_client, table):
key = _put(dynamodb_client, table, "tx", {})
with pytest.raises(ClientError) as exc_info:
dynamodb_client.transact_write_items(TransactItems=[{"Update": {
"TableName": table, "Key": key, "UpdateExpression": "ADD #z.#a :one",
"ExpressionAttributeNames": {"#z": "z", "#a": "a"}, "ExpressionAttributeValues": ONE,
}}])
response = exc_info.value.response
assert response["Error"]["Code"] == "TransactionCanceledException"
assert [(r["Code"], r.get("Message")) for r in response["CancellationReasons"]] == [
("ValidationError", INVALID)]
assert _stored(dynamodb_client, table, key) == {"x": {"S": "x"}}
Loading