Skip to content

Commit 64254a1

Browse files
fix: SET snapshot semantics and parenthesised arithmetic
1 parent 3d33aeb commit 64254a1

4 files changed

Lines changed: 138 additions & 2 deletions

File tree

crates/core/src/expression/update_evaluator.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@ pub fn apply_update(
2727
item: &mut BTreeMap<String, AttributeValue>,
2828
maps: &ExpressionMaps,
2929
) -> Result<(), DynamoDbError> {
30-
// DynamoDB applies actions in order: SET, REMOVE, ADD, DELETE
30+
// DynamoDB evaluates all SET RHS values against the pre-update snapshot,
31+
// then applies the results. This means `SET a = :v, b = a` assigns `b`
32+
// the *original* value of `a`, not the value just written by the first clause.
33+
let snapshot = item.clone();
3134
for action in actions {
3235
if let UpdateAction::Set { path, value } = action {
33-
let resolved_value = evaluate_set_value(value, item, maps)?;
36+
let resolved_value = evaluate_set_value(value, &snapshot, maps)?;
3437
set_path(item, path, resolved_value, maps)?;
3538
}
3639
}

crates/core/src/expression/update_evaluator_tests.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,26 @@ fn set_intermediate_map_path_missing_fails() {
341341
let result = apply("SET a.b.c = :v", &mut item, HashMap::new(), values);
342342
assert!(result.is_err());
343343
}
344+
345+
#[test]
346+
fn set_snapshot_semantics_second_clause_reads_old_value() {
347+
let mut item = BTreeMap::new();
348+
item.insert("pk".into(), AttributeValue::S("key1".into()));
349+
item.insert("a".into(), AttributeValue::S("OLD".into()));
350+
let mut values = HashMap::new();
351+
values.insert("v".into(), AttributeValue::S("NEW".into()));
352+
apply("SET a = :v, b = a", &mut item, HashMap::new(), values).unwrap();
353+
assert_eq!(item.get("a"), Some(&AttributeValue::S("NEW".into())));
354+
assert_eq!(item.get("b"), Some(&AttributeValue::S("OLD".into())));
355+
}
356+
357+
#[test]
358+
fn set_parenthesised_arithmetic() {
359+
let mut item = BTreeMap::new();
360+
item.insert("pk".into(), AttributeValue::S("key1".into()));
361+
item.insert("c".into(), AttributeValue::N("10".into()));
362+
let mut values = HashMap::new();
363+
values.insert("v".into(), AttributeValue::N("3".into()));
364+
apply("SET c = (c - :v)", &mut item, HashMap::new(), values).unwrap();
365+
assert_eq!(item.get("c"), Some(&AttributeValue::N("7".into())));
366+
}

crates/core/src/expression/update_parser.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ use crate::error::DynamoDbError;
2727
///
2828
/// Returns `ValidationException` for syntax errors.
2929
pub fn parse_update(tokens: &[Token]) -> Result<Vec<UpdateAction>, DynamoDbError> {
30+
parser_common::check_redundant_parens(tokens).map_err(|msg| {
31+
DynamoDbError::ValidationException(format!("Invalid UpdateExpression: {msg}"))
32+
})?;
33+
3034
let mut pos = 0;
3135
let mut actions = Vec::new();
3236

@@ -70,6 +74,10 @@ pub fn parse_update_from(
7074
tokens: &[Token],
7175
source: &str,
7276
) -> Result<Vec<UpdateAction>, DynamoDbError> {
77+
parser_common::check_redundant_parens(tokens).map_err(|msg| {
78+
DynamoDbError::ValidationException(format!("Invalid UpdateExpression: {msg}"))
79+
})?;
80+
7381
let mut pos = 0;
7482
let mut actions = Vec::new();
7583

@@ -239,6 +247,17 @@ fn parse_set_operand(tokens: &[Token], pos: &mut usize) -> Result<Expr, DynamoDb
239247
}
240248
}
241249
Token::NameRef(_) => parser_common::parse_path(tokens, pos).map(Expr::Path),
250+
Token::LParen => {
251+
*pos += 1; // consume '('
252+
let expr = parse_value_expr(tokens, pos)?;
253+
if *pos >= tokens.len() || tokens[*pos] != Token::RParen {
254+
return Err(validation_err(
255+
"Invalid UpdateExpression: expected closing ')'",
256+
));
257+
}
258+
*pos += 1; // consume ')'
259+
Ok(expr)
260+
}
242261
_ => Err(validation_err(
243262
"Invalid UpdateExpression: expected path, placeholder, or function",
244263
)),
@@ -513,4 +532,19 @@ mod tests {
513532
panic!("Expected Set action");
514533
}
515534
}
535+
536+
#[test]
537+
fn redundant_parens_rejected() {
538+
let err = parse("SET c = ((c - :v))").unwrap_err();
539+
assert!(
540+
matches!(&err, DynamoDbError::ValidationException(msg) if msg.contains("redundant parentheses")),
541+
"Expected redundant parens error, got: {err:?}"
542+
);
543+
}
544+
545+
#[test]
546+
fn single_parens_accepted() {
547+
let actions = parse("SET c = (c - :v)").unwrap();
548+
assert!(matches!(&actions[0], UpdateAction::Set { .. }));
549+
}
516550
}

tests/test_set_evaluation.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Copyright 2026 ExtendDB contributors
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""Tests for SET evaluation snapshot semantics and parenthesised arithmetic."""
5+
6+
from __future__ import annotations
7+
8+
import uuid
9+
10+
import pytest
11+
12+
13+
@pytest.fixture()
14+
def hash_table(dynamodb_client):
15+
name = f"set_eval_{uuid.uuid4().hex[:8]}"
16+
dynamodb_client.create_table(
17+
TableName=name,
18+
KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}],
19+
AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}],
20+
BillingMode="PAY_PER_REQUEST",
21+
)
22+
from conftest import wait_for_active
23+
wait_for_active(dynamodb_client, name)
24+
yield name
25+
dynamodb_client.delete_table(TableName=name)
26+
27+
28+
class TestSetSnapshotSemantics:
29+
"""SET clauses evaluate RHS against the pre-update item snapshot."""
30+
31+
def test_second_set_reads_old_value(self, dynamodb_client, hash_table):
32+
dynamodb_client.put_item(
33+
TableName=hash_table,
34+
Item={"pk": {"S": "x"}, "a": {"S": "OLD"}},
35+
)
36+
resp = dynamodb_client.update_item(
37+
TableName=hash_table,
38+
Key={"pk": {"S": "x"}},
39+
UpdateExpression="SET a = :v, b = a",
40+
ExpressionAttributeValues={":v": {"S": "NEW"}},
41+
ReturnValues="ALL_NEW",
42+
)
43+
assert resp["Attributes"]["a"]["S"] == "NEW"
44+
assert resp["Attributes"]["b"]["S"] == "OLD"
45+
46+
47+
class TestParenthesisedArithmetic:
48+
"""SET supports parenthesised arithmetic expressions."""
49+
50+
def test_subtraction_in_parens(self, dynamodb_client, hash_table):
51+
dynamodb_client.put_item(
52+
TableName=hash_table,
53+
Item={"pk": {"S": "y"}, "c": {"N": "10"}},
54+
)
55+
resp = dynamodb_client.update_item(
56+
TableName=hash_table,
57+
Key={"pk": {"S": "y"}},
58+
UpdateExpression="SET c = (c - :v)",
59+
ExpressionAttributeValues={":v": {"N": "3"}},
60+
ReturnValues="ALL_NEW",
61+
)
62+
assert resp["Attributes"]["c"]["N"] == "7"
63+
64+
def test_addition_in_parens(self, dynamodb_client, hash_table):
65+
dynamodb_client.put_item(
66+
TableName=hash_table,
67+
Item={"pk": {"S": "z"}, "n": {"N": "5"}},
68+
)
69+
resp = dynamodb_client.update_item(
70+
TableName=hash_table,
71+
Key={"pk": {"S": "z"}},
72+
UpdateExpression="SET n = (n + :v)",
73+
ExpressionAttributeValues={":v": {"N": "2"}},
74+
ReturnValues="ALL_NEW",
75+
)
76+
assert resp["Attributes"]["n"]["N"] == "7"

0 commit comments

Comments
 (0)