Skip to content
Merged
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
7 changes: 5 additions & 2 deletions crates/core/src/expression/update_evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@ pub fn apply_update(
item: &mut BTreeMap<String, AttributeValue>,
maps: &ExpressionMaps,
) -> Result<(), DynamoDbError> {
// DynamoDB applies actions in order: SET, REMOVE, ADD, DELETE
// DynamoDB evaluates all SET RHS values against the pre-update snapshot,
// then applies the results. This means `SET a = :v, b = a` assigns `b`
// the *original* value of `a`, not the value just written by the first clause.
let snapshot = item.clone();
for action in actions {
if let UpdateAction::Set { path, value } = action {
let resolved_value = evaluate_set_value(value, item, maps)?;
let resolved_value = evaluate_set_value(value, &snapshot, maps)?;
set_path(item, path, resolved_value, maps)?;
}
}
Expand Down
23 changes: 23 additions & 0 deletions crates/core/src/expression/update_evaluator_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,26 @@ fn set_intermediate_map_path_missing_fails() {
let result = apply("SET a.b.c = :v", &mut item, HashMap::new(), values);
assert!(result.is_err());
}

#[test]
fn set_snapshot_semantics_second_clause_reads_old_value() {
let mut item = BTreeMap::new();
item.insert("pk".into(), AttributeValue::S("key1".into()));
item.insert("a".into(), AttributeValue::S("OLD".into()));
let mut values = HashMap::new();
values.insert("v".into(), AttributeValue::S("NEW".into()));
apply("SET a = :v, b = a", &mut item, HashMap::new(), values).unwrap();
assert_eq!(item.get("a"), Some(&AttributeValue::S("NEW".into())));
assert_eq!(item.get("b"), Some(&AttributeValue::S("OLD".into())));
}

#[test]
fn set_parenthesised_arithmetic() {
let mut item = BTreeMap::new();
item.insert("pk".into(), AttributeValue::S("key1".into()));
item.insert("c".into(), AttributeValue::N("10".into()));
let mut values = HashMap::new();
values.insert("v".into(), AttributeValue::N("3".into()));
apply("SET c = (c - :v)", &mut item, HashMap::new(), values).unwrap();
assert_eq!(item.get("c"), Some(&AttributeValue::N("7".into())));
}
55 changes: 55 additions & 0 deletions crates/core/src/expression/update_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ use crate::error::DynamoDbError;
///
/// Returns `ValidationException` for syntax errors.
pub fn parse_update(tokens: &[Token]) -> Result<Vec<UpdateAction>, DynamoDbError> {
parser_common::check_redundant_parens(tokens).map_err(|msg| {
DynamoDbError::ValidationException(format!("Invalid UpdateExpression: {msg}"))
})?;

let mut pos = 0;
let mut actions = Vec::new();

Expand Down Expand Up @@ -70,6 +74,10 @@ pub fn parse_update_from(
tokens: &[Token],
source: &str,
) -> Result<Vec<UpdateAction>, DynamoDbError> {
parser_common::check_redundant_parens(tokens).map_err(|msg| {
DynamoDbError::ValidationException(format!("Invalid UpdateExpression: {msg}"))
})?;

let mut pos = 0;
let mut actions = Vec::new();

Expand Down Expand Up @@ -239,6 +247,17 @@ fn parse_set_operand(tokens: &[Token], pos: &mut usize) -> Result<Expr, DynamoDb
}
}
Token::NameRef(_) => parser_common::parse_path(tokens, pos).map(Expr::Path),
Token::LParen => {
*pos += 1; // consume '('
let expr = parse_value_expr(tokens, pos)?;
if *pos >= tokens.len() || tokens[*pos] != Token::RParen {
return Err(validation_err(
"Invalid UpdateExpression: expected closing ')'",
));
}
*pos += 1; // consume ')'
Ok(expr)
}
_ => Err(validation_err(
"Invalid UpdateExpression: expected path, placeholder, or function",
)),
Expand Down Expand Up @@ -513,4 +532,40 @@ mod tests {
panic!("Expected Set action");
}
}

#[test]
fn redundant_parens_rejected() {
let err = parse("SET c = ((c - :v))").unwrap_err();
assert!(
matches!(&err, DynamoDbError::ValidationException(msg) if msg.contains("redundant parentheses")),
"Expected redundant parens error, got: {err:?}"
);
}

#[test]
fn single_parens_accepted() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wanted bonus points, could check for error on unmatched parens.

let actions = parse("SET c = (c - :v)").unwrap();
assert!(matches!(&actions[0], UpdateAction::Set { .. }));
}

#[test]
fn unmatched_paren_rejected() {
// Opening paren with no matching close must be a ValidationException,
// not a panic or a silently-accepted expression.
let err = parse("SET c = (c - :v").unwrap_err();
assert!(
matches!(&err, DynamoDbError::ValidationException(msg) if msg.contains("closing")),
"Expected unmatched-paren error, got: {err:?}"
);
}

#[test]
fn unmatched_close_paren_rejected() {
// A stray closing paren with no matching open must also be rejected.
let err = parse("SET c = c - :v)").unwrap_err();
assert!(
matches!(&err, DynamoDbError::ValidationException(_)),
"Expected validation error for stray ')', got: {err:?}"
);
}
}
76 changes: 76 additions & 0 deletions tests/test_set_evaluation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Copyright 2026 ExtendDB contributors
# SPDX-License-Identifier: Apache-2.0

"""Tests for SET evaluation snapshot semantics and parenthesised arithmetic."""

from __future__ import annotations

import uuid

import pytest


@pytest.fixture()
def hash_table(dynamodb_client):
name = f"set_eval_{uuid.uuid4().hex[:8]}"
dynamodb_client.create_table(
TableName=name,
KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}],
AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}],
BillingMode="PAY_PER_REQUEST",
)
from conftest import wait_for_active
wait_for_active(dynamodb_client, name)
yield name
dynamodb_client.delete_table(TableName=name)


class TestSetSnapshotSemantics:
"""SET clauses evaluate RHS against the pre-update item snapshot."""

def test_second_set_reads_old_value(self, dynamodb_client, hash_table):
dynamodb_client.put_item(
TableName=hash_table,
Item={"pk": {"S": "x"}, "a": {"S": "OLD"}},
)
resp = dynamodb_client.update_item(
TableName=hash_table,
Key={"pk": {"S": "x"}},
UpdateExpression="SET a = :v, b = a",
ExpressionAttributeValues={":v": {"S": "NEW"}},
ReturnValues="ALL_NEW",
)
assert resp["Attributes"]["a"]["S"] == "NEW"
assert resp["Attributes"]["b"]["S"] == "OLD"


class TestParenthesisedArithmetic:
"""SET supports parenthesised arithmetic expressions."""

def test_subtraction_in_parens(self, dynamodb_client, hash_table):
dynamodb_client.put_item(
TableName=hash_table,
Item={"pk": {"S": "y"}, "c": {"N": "10"}},
)
resp = dynamodb_client.update_item(
TableName=hash_table,
Key={"pk": {"S": "y"}},
UpdateExpression="SET c = (c - :v)",
ExpressionAttributeValues={":v": {"N": "3"}},
ReturnValues="ALL_NEW",
)
assert resp["Attributes"]["c"]["N"] == "7"

def test_addition_in_parens(self, dynamodb_client, hash_table):
dynamodb_client.put_item(
TableName=hash_table,
Item={"pk": {"S": "z"}, "n": {"N": "5"}},
)
resp = dynamodb_client.update_item(
TableName=hash_table,
Key={"pk": {"S": "z"}},
UpdateExpression="SET n = (n + :v)",
ExpressionAttributeValues={":v": {"N": "2"}},
ReturnValues="ALL_NEW",
)
assert resp["Attributes"]["n"]["N"] == "7"
Loading