diff --git a/crates/core/src/expression/resolver.rs b/crates/core/src/expression/resolver.rs index fa14081ec..699cd4fdf 100755 --- a/crates/core/src/expression/resolver.rs +++ b/crates/core/src/expression/resolver.rs @@ -26,6 +26,10 @@ use super::ast::{CompareOp, Expr, PathElement}; pub struct ExpressionMaps { pub names: HashMap, pub values: HashMap, + /// Set by the engine when the update arrived as legacy `AttributeUpdates` + /// and was desugared into an update expression. The service sizes that + /// form against the item limit differently from an expression. + pub legacy_attribute_updates: bool, /// Pre-parsed `BigDecimal` values for numeric placeholders. /// Populated eagerly by `new()` and refreshed by `pre_parse_numerics()`. parsed_numerics: HashMap, @@ -40,6 +44,7 @@ impl ExpressionMaps { let mut maps = Self { names, values, + legacy_attribute_updates: false, parsed_numerics: HashMap::new(), }; maps.pre_parse_numerics(); diff --git a/crates/core/src/types/item.rs b/crates/core/src/types/item.rs index 0d31edb72..e363cca46 100755 --- a/crates/core/src/types/item.rs +++ b/crates/core/src/types/item.rs @@ -388,35 +388,75 @@ pub fn attribute_value_size(value: &AttributeValue) -> usize { /// Calculate the `DynamoDB` size of a number in bytes. /// -/// `DynamoDB` number sizing: approximately 1 byte per 2 significant digits + 1 byte. -/// Zero is 1 byte. Negative numbers add 1 byte. Max 21 bytes. +/// Measured against the service (us-east-1, 2026-10-19) by accept/refuse pairs +/// either side of the 409,600-byte item limit: +/// +/// ```text +/// 1 + ceil(integer significant digits / 2) + ceil(fraction significant digits / 2) +/// + 1 if negative +/// ``` +/// +/// The two halves are rounded up separately, so digits that straddle the +/// decimal point cost one byte more than the same digits without a point: +/// `1.5` is 3 bytes where `15` is 2, and `3.14159` is 5 where `123456` is 4. +/// Leading zeros of the integer half and trailing zeros of the fraction half +/// are not significant; trailing zeros of the integer half are significant +/// when a fraction follows them (`100` is 2, `100.5` is 4) and a zero between +/// two significant digits always is (`1010` is 3). The exponent is not +/// counted (`1E125` is 2). Zero is 1 byte however it is spelled, and is never +/// negative (`-0` is 1). fn dynamodb_number_size(n: &str) -> usize { - let s = n.trim_start_matches('-'); - let is_zero = s.chars().all(|c| c == '0' || c == '.'); - if is_zero { + let (negative, body) = match n.strip_prefix('-') { + Some(rest) => (true, rest), + None => (false, n.strip_prefix('+').unwrap_or(n)), + }; + let (mantissa, exponent) = match body.find(['e', 'E']) { + Some(pos) => (&body[..pos], body[pos + 1..].parse::().unwrap_or(0)), + None => (body, 0), + }; + let (int_text, frac_text) = match mantissa.find('.') { + Some(pos) => (&mantissa[..pos], &mantissa[pos + 1..]), + None => (mantissa, ""), + }; + let digits: String = int_text.chars().chain(frac_text.chars()).collect(); + if !digits.bytes().any(|b| (b'1'..=b'9').contains(&b)) { return 1; } - - let significant = if let Some(dot_pos) = s.find('.') { - let (int_part, frac_part) = s.split_at(dot_pos); - let frac = &frac_part[1..]; - let int_trimmed = int_part.trim_start_matches('0'); - let frac_trimmed = frac.trim_end_matches('0'); - if int_trimmed.is_empty() { - let frac_sig = frac.trim_start_matches('0'); - frac_sig.trim_end_matches('0').len() - } else { - format!("{int_trimmed}{frac_trimmed}").len() - } + // Where the decimal point sits in `digits` once the exponent is applied. + let point = i64::try_from(int_text.len()) + .unwrap_or(i64::MAX) + .saturating_add(exponent); + let len = i64::try_from(digits.len()).unwrap_or(i64::MAX); + let (int_part, frac_part): (String, String) = if point <= 0 { + ( + String::new(), + "0".repeat(usize::try_from(-point).unwrap_or(0)) + &digits, + ) + } else if point >= len { + ( + digits.clone() + &"0".repeat(usize::try_from(point - len).unwrap_or(0)), + String::new(), + ) } else { - let trimmed = s.trim_start_matches('0').trim_end_matches('0'); - if trimmed.is_empty() { 1 } else { trimmed.len() } + let split = usize::try_from(point).unwrap_or(0); + (digits[..split].to_owned(), digits[split..].to_owned()) }; - - let mut size = significant.div_ceil(2) + 1; - if n.starts_with('-') { - size += 1; - } + let int_part = int_part.trim_start_matches('0'); + let frac_part = frac_part.trim_end_matches('0'); + // Trailing zeros of the integer half fall away only when no fraction follows. + let int_significant = if frac_part.is_empty() { + int_part.trim_end_matches('0').len() + } else { + int_part.len() + }; + // Leading zeros of the fraction half fall away only when no integer precedes. + let frac_significant = if int_part.is_empty() { + frac_part.trim_start_matches('0').len() + } else { + frac_part.len() + }; + let size = + 1 + int_significant.div_ceil(2) + frac_significant.div_ceil(2) + usize::from(negative); size.min(21) } @@ -428,6 +468,32 @@ mod tests { AttributeValue::S(v.to_owned()) } + #[test] + fn number_size_matches_the_service_for_measured_literals() { + // Each figure measured against the service (us-east-1, 2026-10-19) by an + // accept/refuse pair either side of the 409,600-byte item limit. + for (literal, bytes) in [ + ("0", 1), + ("-0", 1), + ("1", 2), + ("15", 2), + ("100", 2), + ("1E125", 2), + ("0.15", 2), + ("1.5", 3), + ("1.2", 3), + ("1.200", 3), + ("1010", 3), + ("-42", 3), + ("1.234", 4), + ("100.5", 4), + ("123456", 4), + ("3.14159", 5), + ] { + assert_eq!(dynamodb_number_size(literal), bytes, "literal {literal}"); + } + } + #[test] fn scalar_sizes_unchanged() { assert_eq!(attribute_value_size(&s("hello")), 5); diff --git a/crates/core/src/types/transaction.rs b/crates/core/src/types/transaction.rs index 5010cbb47..2f311abfe 100755 --- a/crates/core/src/types/transaction.rs +++ b/crates/core/src/types/transaction.rs @@ -315,8 +315,88 @@ impl CancellationReason { item: None, } } + + /// Internal reason for a wrong-typed key found while preparing a + /// transaction member: a table key element that does not match the + /// schema, or a secondary-index key (on a written item or as a literal + /// operand) that does not match its attribute definition. Never reaches a + /// client: [`Self::resolve_key_faults`] rewrites it to `ValidationError`. + #[must_use] + pub fn key_type_mismatch(msg: impl Into) -> Self { + Self { + code: KEY_TYPE_MISMATCH_CODE.to_owned(), + message: Some(msg.into()), + item: None, + } + } + + /// Internal reason for a secondary-index key left empty by a transaction + /// member. Never reaches a client: [`Self::resolve_key_faults`] turns it + /// into a top-level `ValidationException` message. + #[must_use] + pub fn index_key_empty_abort(msg: impl Into) -> Self { + Self { + code: INDEX_KEY_EMPTY_ABORT_CODE.to_owned(), + message: Some(msg.into()), + item: None, + } + } + + /// Whether this reason is one of the internal key faults that + /// [`Self::resolve_key_faults`] gives precedence to. A backend that runs + /// members out of request order can stop once the earliest such member + /// in request order is known: a wrong-typed key there outranks every later + /// member, and an empty index key there outranks every later member that + /// is not itself a wrong-typed key. + #[must_use] + pub fn is_key_fault(&self) -> bool { + self.code == KEY_TYPE_MISMATCH_CODE || self.code == INDEX_KEY_EMPTY_ABORT_CODE + } + + /// Apply the precedence `DynamoDB` gives key faults across the members of + /// a `TransactWriteItems` request, measured on the service. A wrong-typed + /// key (table or secondary index) on any member cancels the transaction + /// with the first such member's `ValidationError` in position and `None` + /// in every other position, whatever those members' own faults were. + /// Otherwise an empty secondary-index key on any member aborts the whole + /// request with a top-level `ValidationException`, outranking failed + /// conditions and unapplicable statements on the other members. Otherwise + /// the reasons are returned as they are. + /// + /// # Errors + /// + /// Returns `Err(message)` when the request must be answered with a + /// top-level `ValidationException` carrying `message`. + pub fn resolve_key_faults(reasons: Vec) -> Result, String> { + let first_type_mismatch = reasons + .iter() + .position(|r| r.code == KEY_TYPE_MISMATCH_CODE); + let mut reasons = reasons; + for (i, reason) in reasons.iter_mut().enumerate() { + match first_type_mismatch { + Some(first) if i == first => reason.code = "ValidationError".to_owned(), + Some(_) => *reason = Self::none(), + None if reason.code == INDEX_KEY_EMPTY_ABORT_CODE => { + if let Some(message) = reason.message.take() { + return Err(message); + } + *reason = Self::none(); + } + None => {} + } + } + Ok(reasons) + } } +/// Internal cancellation code for a wrong-typed key. See +/// [`CancellationReason::key_type_mismatch`]. +const KEY_TYPE_MISMATCH_CODE: &str = "ExtendDbKeyTypeMismatch"; + +/// Internal cancellation code for an empty secondary-index key. See +/// [`CancellationReason::index_key_empty_abort`]. +const INDEX_KEY_EMPTY_ABORT_CODE: &str = "ExtendDbIndexKeyEmptyAbort"; + #[cfg(test)] mod tests { use super::*; @@ -337,4 +417,60 @@ mod tests { let json = serde_json::to_value(&resp).unwrap(); assert_eq!(json["Item"]["pk"]["S"], "val"); } + + fn codes(reasons: &[CancellationReason]) -> Vec<&str> { + reasons.iter().map(|r| r.code.as_str()).collect() + } + + #[test] + fn no_key_fault_leaves_the_reasons_untouched() { + let reasons = vec![ + CancellationReason::condition_check_failed_with_item(None), + CancellationReason::validation_error("cannot apply"), + CancellationReason::none(), + ]; + let out = CancellationReason::resolve_key_faults(reasons).unwrap(); + assert_eq!( + codes(&out), + ["ConditionalCheckFailed", "ValidationError", "None"] + ); + } + + #[test] + fn an_empty_index_key_becomes_a_top_level_message_over_other_member_faults() { + let reasons = vec![ + CancellationReason::condition_check_failed_with_item(None), + CancellationReason::index_key_empty_abort("empty key"), + CancellationReason::validation_error("cannot apply"), + ]; + assert_eq!( + CancellationReason::resolve_key_faults(reasons).unwrap_err(), + "empty key" + ); + } + + #[test] + fn a_type_mismatch_masks_every_other_member_including_an_empty_key() { + let reasons = vec![ + CancellationReason::condition_check_failed_with_item(None), + CancellationReason::index_key_empty_abort("empty key"), + CancellationReason::key_type_mismatch("type"), + CancellationReason::validation_error("cannot apply"), + ]; + let out = CancellationReason::resolve_key_faults(reasons).unwrap(); + assert_eq!(codes(&out), ["None", "None", "ValidationError", "None"]); + assert_eq!(out[2].message.as_deref(), Some("type")); + } + + #[test] + fn only_the_first_type_mismatch_by_position_is_reported() { + let reasons = vec![ + CancellationReason::none(), + CancellationReason::key_type_mismatch("second"), + CancellationReason::key_type_mismatch("third"), + ]; + let out = CancellationReason::resolve_key_faults(reasons).unwrap(); + assert_eq!(codes(&out), ["None", "ValidationError", "None"]); + assert_eq!(out[1].message.as_deref(), Some("second")); + } } diff --git a/crates/core/src/validation/mod.rs b/crates/core/src/validation/mod.rs index 3decb4666..d11d54643 100755 --- a/crates/core/src/validation/mod.rs +++ b/crates/core/src/validation/mod.rs @@ -1524,6 +1524,34 @@ pub fn validate_index_key_types( Ok(()) } +/// Validate that secondary-index key attributes present in the item an +/// UpdateExpression produced match their declared scalar type, reporting with +/// the wording `DynamoDB` uses for a value computed from the existing item. +/// +/// The service checks literal operands before the condition with the +/// per-index `Type mismatch for Index Key` wording (see +/// [`validate_index_key_literal_operands`]). A value that comes from an +/// attribute reference or the fallback of `if_not_exists` is only known once +/// the statement has been applied, and is then reported with this message, +/// which names neither the attribute nor the index. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` on the first wrong-typed +/// index key attribute, indexes in name order. +pub fn validate_index_key_types_after_update( + item: &Item, + indexes: &[IndexKeyRef<'_>], + attr_defs: &[AttributeDefinition], +) -> Result<(), DynamoDbError> { + validate_index_key_types(item, indexes, attr_defs).map_err(|_| { + DynamoDbError::ValidationException( + "The update expression attempted to update the secondary index key to unsupported type" + .to_owned(), + ) + }) +} + /// Validate that no secondary-index key attribute in `item` is an empty string /// or empty binary value. /// @@ -1594,6 +1622,131 @@ fn ordered_indexes<'b, 'a>(indexes: &'b [IndexKeyRef<'a>]) -> Vec<&'b IndexKeyRe ordered } +/// The type an update operand produces when it is built from literals alone, +/// `None` when any part of it reads the item (an attribute path, or a +/// function whose result depends on one such as `if_not_exists`). +/// +/// `list_append` of two literals is a list; arithmetic on two literals is a +/// number. Anything else that is literal-only is a placeholder. +fn literal_operand_type( + expr: &crate::expression::Expr, + maps: &crate::expression::ExpressionMaps, +) -> Option<&'static str> { + use crate::expression::Expr; + match expr { + Expr::Placeholder(name) => maps.values.get(name).map(attribute_value_type_tag), + Expr::Function { name, args } if name.eq_ignore_ascii_case("list_append") => { + let all_literal = args.iter().all(|a| literal_operand_type(a, maps).is_some()); + all_literal.then_some("L") + } + Expr::Arithmetic { left, right, .. } => { + let both = literal_operand_type(left, maps).is_some() + && literal_operand_type(right, maps).is_some(); + both.then_some("N") + } + _ => None, + } +} + +/// Validate, before a condition is evaluated or a statement applied, that every +/// `SET`, `ADD` or `DELETE` action whose target is a secondary-index key +/// attribute and whose operand is built from literals alone carries a value of +/// the key's declared type. +/// +/// `DynamoDB` runs this check on the statement text ahead of the condition and +/// ahead of applying the statement, so it is reported even when the condition +/// would fail or another action in the statement could not be applied. A value +/// that depends on the existing item (an attribute reference, the fallback of +/// `if_not_exists`) is not checked here; the storage layer checks the written +/// item afterwards and reports it with the computed-value wording. +/// +/// `ADD` of a number onto a number key keeps the key's type and passes. `ADD` +/// or `DELETE` of a set onto any index key, and `ADD` of a number onto a +/// string or binary key, are type mismatches, because the result cannot be a +/// scalar of the declared type. Indexes are checked in name order and actions +/// in statement order. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the index-key type +/// mismatch wording naming the attribute, the expected and actual types and +/// the alphabetically-first index that keys the attribute. +pub fn validate_index_key_literal_operands( + actions: &[crate::expression::UpdateAction], + indexes: &[IndexKeyRef<'_>], + attr_defs: &[AttributeDefinition], + maps: &crate::expression::ExpressionMaps, +) -> Result<(), DynamoDbError> { + use crate::expression::{PathElement, UpdateAction}; + if indexes.is_empty() { + return Ok(()); + } + let ordered = ordered_indexes(indexes); + for action in actions { + let (path, operand, is_set) = match action { + UpdateAction::Set { path, value } => (path, value, true), + UpdateAction::Add { path, value } | UpdateAction::Delete { path, value } => { + (path, value, false) + } + UpdateAction::Remove { .. } => continue, + }; + // Only a top-level attribute can be an index key. + if path.len() != 1 { + continue; + } + let Some(PathElement::Attribute(raw)) = path.first() else { + continue; + }; + let attr: &str = match raw.strip_prefix('#') { + Some(alias) => maps.resolve_name(alias)?, + None => raw.as_str(), + }; + let Some(actual) = literal_operand_type(operand, maps) else { + continue; + }; + for idx in &ordered { + for ks in idx.key_schema { + if ks.attribute_name != attr { + continue; + } + let Some(expected) = attr_defs + .iter() + .find(|ad| ad.attribute_name == attr) + .map(|ad| ad.attribute_type) + else { + continue; + }; + // SET must supply the declared type. ADD of a number onto a + // number key keeps the type and passes here (a string operand + // to ADD is an expression-level error the parser reports); + // ADD or DELETE of a set, or ADD of a number onto a string or + // binary key, cannot leave a scalar of the declared type. + let ok = if is_set { + scalar_type_tag(expected) == actual + } else { + match actual { + "N" => expected == ScalarAttributeType::N, + "SS" | "NS" | "BS" => false, + // Any other operand type is an expression-level + // operand error the evaluator reports. + _ => true, + } + }; + if !ok { + return Err(DynamoDbError::ValidationException(format!( + "One or more parameter values were invalid: Type mismatch for Index Key {} Expected: {} Actual: {} IndexName: {}", + attr, + scalar_type_tag(expected), + actual, + idx.index_name + ))); + } + } + } + } + Ok(()) +} + /// Readable flags for the `is_query` parameter of [`validate_select_projection`]. /// Real DynamoDB prepends `1 validation error detected: ` to some rejections for /// Query but not for Scan, so callers pass one of these instead of a bare bool. @@ -1876,6 +2029,257 @@ pub fn validate_item_size(item: &Item, max_bytes: usize) -> Result<(), DynamoDbE Ok(()) } +/// The wording the service uses when an update, standalone or transacted, would +/// exceed the item size limit. +pub const ITEM_SIZE_TO_UPDATE_EXCEEDED: &str = + "Item size to update has exceeded the maximum allowed size"; + +/// Fixed cost the service charges an `UpdateItem` statement, before its clauses. +pub const UPDATE_STATEMENT_BASE_COST: usize = 3; +/// Fixed cost per `SET` or `ADD` clause, on top of the written path and operands. +pub const UPDATE_SET_CLAUSE_COST: usize = 18; +/// Fixed cost of a function call in a `SET` operand, plus the function name's length. +pub const UPDATE_FUNCTION_COST: usize = 26; +/// Fixed cost of a `+` in a `SET` operand (26 plus the length of "addition"). +pub const UPDATE_ADDITION_COST: usize = 34; +/// Fixed cost of a `-` in a `SET` operand (26 plus the length of "subtraction"). +pub const UPDATE_SUBTRACTION_COST: usize = 37; +/// Cost of the first element of a document path used as a function or arithmetic +/// operand, plus the attribute name's length. The attribute's stored value is not +/// charged. +pub const UPDATE_PATH_OPERAND_FIRST_COST: usize = 15; +/// Cost of a later attribute element of such a path, plus the name's length. +pub const UPDATE_PATH_OPERAND_NESTED_COST: usize = 14; +/// Cost of a list index element of such a path. +pub const UPDATE_PATH_OPERAND_INDEX_COST: usize = 15; +/// Extra cost of a clause whose target path ends in a list index. +pub const UPDATE_LIST_INDEX_TARGET_COST: usize = 1; + +/// Size a standalone `UpdateItem` the way the service does. +/// +/// Measured against the service (us-east-1, 2026-10-19) by accept/refuse pairs +/// either side of the item limit, for whole-attribute and document-path +/// targets, aliases, one to three clauses on the same or different attributes, +/// `if_not_exists`, `list_append`, nested functions, arithmetic, `ADD`, +/// `REMOVE`, `DELETE`, and the legacy `AttributeUpdates` form. The figure the +/// limit applies to is: +/// +/// * a base of 3; +/// * 18 per `SET` or `ADD` clause, nothing per `REMOVE` or `DELETE`; +/// * the size of a skeleton item holding only the written paths: each map +/// entry its name plus one, each map or list 3, each list element one more, +/// siblings the statement does not touch left out. A `SET` or `ADD` leaf is +/// charged the sizes of the clause's value placeholders; a `REMOVE` or +/// `DELETE` leaf is charged nothing, and the `DELETE` operand is not charged; +/// * one more for a clause whose target path ends in a list index; +/// * for operands inside a `SET` value: 26 plus the name length per function +/// call, 34 per `+` and 37 per `-`, and for a document path 15 plus the +/// name for its first element, 14 plus the name for a later attribute, 15 +/// for an index, with the attribute's stored value not charged. +/// +/// The legacy `AttributeUpdates` form is charged 3 plus, for each attribute +/// it names that is present afterwards, the name and the finished value's +/// bytes; an attribute it removes costs nothing. +/// +/// The key and untouched attributes are left out, so at a short key the largest +/// finished item an update can produce sits below the limit, and a long key or +/// an untouched attribute buys the headroom back up to the limit itself, which +/// the finished item never exceeds. +/// +/// `finished` is the whole finished item; `actions` are the statement's clauses +/// with `#name` placeholders still in place, resolved here through `maps`. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the update wording when +/// either the statement figure or the finished item exceeds `max_bytes`. +pub fn validate_update_item_size( + finished: &Item, + actions: &[crate::expression::UpdateAction], + maps: &crate::expression::ExpressionMaps, + max_bytes: usize, +) -> Result<(), DynamoDbError> { + let statement = if maps.legacy_attribute_updates { + legacy_update_statement_size(finished, actions, maps)? + } else { + update_statement_size(actions, maps)? + }; + if statement > max_bytes || item_size_bytes(finished) > max_bytes { + return Err(DynamoDbError::ValidationException( + ITEM_SIZE_TO_UPDATE_EXCEEDED.to_owned(), + )); + } + Ok(()) +} + +/// The bytes the service charges the operand expression of a `SET` clause: +/// value placeholders at their size, function calls, arithmetic operators and +/// document paths at their fixed costs. +fn update_operand_size( + expr: &crate::expression::Expr, + maps: &crate::expression::ExpressionMaps, +) -> Result { + use crate::expression::{Expr, PathElement, resolve_element_name}; + Ok(match expr { + Expr::Placeholder(name) => maps + .values + .get(name) + .map_or(0, crate::types::attribute_value_size), + Expr::Path(elements) => { + let mut bytes = 0; + for (i, element) in elements.iter().enumerate() { + bytes += match element { + PathElement::Index(_) => UPDATE_PATH_OPERAND_INDEX_COST, + PathElement::Attribute(_) => { + let name = resolve_element_name(element, maps)?; + name.len() + + if i == 0 { + UPDATE_PATH_OPERAND_FIRST_COST + } else { + UPDATE_PATH_OPERAND_NESTED_COST + } + } + }; + } + bytes + } + Expr::Function { name, args } => { + let mut bytes = UPDATE_FUNCTION_COST + name.len(); + for arg in args { + bytes += update_operand_size(arg, maps)?; + } + bytes + } + Expr::Arithmetic { left, op, right } => { + let operator = match op { + crate::expression::ArithOp::Add => UPDATE_ADDITION_COST, + crate::expression::ArithOp::Sub => UPDATE_SUBTRACTION_COST, + }; + operator + update_operand_size(left, maps)? + update_operand_size(right, maps)? + } + _ => 0, + }) +} + +/// A written path as a tree: map entries by name, list elements by index. +#[derive(Default)] +struct WrittenNode { + leaf_bytes: usize, + entries: std::collections::BTreeMap, + elements: std::collections::BTreeMap, +} + +fn written_node_size(node: &WrittenNode) -> usize { + let maps: usize = if node.entries.is_empty() { + 0 + } else { + 3 + node + .entries + .iter() + .map(|(name, child)| name.len() + written_node_size(child) + 1) + .sum::() + }; + let lists: usize = if node.elements.is_empty() { + 0 + } else { + 3 + node + .elements + .values() + .map(|child| written_node_size(child) + 1) + .sum::() + }; + node.leaf_bytes + maps + lists +} + +/// Build the skeleton the statement writes and size it with the clause costs. +fn update_statement_size( + actions: &[crate::expression::UpdateAction], + maps: &crate::expression::ExpressionMaps, +) -> Result { + use crate::expression::{PathElement, UpdateAction, resolve_element_name}; + let mut root = WrittenNode::default(); + let mut bytes = UPDATE_STATEMENT_BASE_COST; + for action in actions { + let (path, leaf) = match action { + UpdateAction::Set { path, value } | UpdateAction::Add { path, value } => { + bytes += UPDATE_SET_CLAUSE_COST; + (path, update_operand_size(value, maps)?) + } + UpdateAction::Remove { path } | UpdateAction::Delete { path, .. } => (path, 0), + }; + if matches!(path.last(), Some(PathElement::Index(_))) { + bytes += UPDATE_LIST_INDEX_TARGET_COST; + } + let mut node = &mut root; + for element in path { + node = match element { + PathElement::Index(i) => node.elements.entry(*i).or_default(), + PathElement::Attribute(_) => { + let name = resolve_element_name(element, maps)?; + node.entries.entry(name.into_owned()).or_default() + } + }; + } + node.leaf_bytes += leaf; + } + // The item itself is a map whose 3 bytes the base already covers. + Ok(bytes + written_node_size(&root).saturating_sub(3)) +} + +/// The legacy `AttributeUpdates` form: 3 plus, for each named attribute still +/// present in the finished item, its name and finished value bytes. Nothing per +/// action, no entry byte, nothing for an attribute the update removed. +fn legacy_update_statement_size( + finished: &Item, + actions: &[crate::expression::UpdateAction], + maps: &crate::expression::ExpressionMaps, +) -> Result { + use crate::expression::{UpdateAction, resolve_element_name}; + let mut names: Vec = Vec::new(); + for action in actions { + let path = match action { + UpdateAction::Set { path, .. } + | UpdateAction::Add { path, .. } + | UpdateAction::Remove { path } + | UpdateAction::Delete { path, .. } => path, + }; + if let Some(first) = path.first() { + let name = resolve_element_name(first, maps)?; + if !names.iter().any(|n| n == name.as_ref()) { + names.push(name.into_owned()); + } + } + } + Ok(UPDATE_STATEMENT_BASE_COST + + names + .iter() + .filter_map(|name| finished.get(name).map(|value| (name, value))) + .map(|(name, value)| name.len() + crate::types::attribute_value_size(value)) + .sum::()) +} + +/// Size a transacted `Update` the way the service does: the finished item +/// against the limit, with the update wording. Measured against the service +/// (us-east-1, 2026-10-19): a transacted Update accepts a finished item of +/// exactly the limit and cancels one byte over with this wording, so the +/// statement-sized rule of a standalone `UpdateItem` does not apply. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the update wording when the +/// finished item exceeds `max_bytes`. +pub fn validate_transact_update_item_size( + finished: &Item, + max_bytes: usize, +) -> Result<(), DynamoDbError> { + if item_size_bytes(finished) > max_bytes { + return Err(DynamoDbError::ValidationException( + ITEM_SIZE_TO_UPDATE_EXCEEDED.to_owned(), + )); + } + Ok(()) +} + /// Validate all number values in an item are within `DynamoDB` limits. pub fn validate_item_numbers(item: &Item) -> Result<(), DynamoDbError> { for value in item.values() { @@ -3801,3 +4205,117 @@ mod tests { assert!(validate_conditional_operator_usage(false, 1).is_ok()); } } + +#[cfg(test)] +mod update_size_tests { + //! Direct unit tests for the UpdateItem statement sizer. The byte figures + //! are the ones measured against the service in us-east-1 on 2026-10-19 and + //! exercised end to end by tests/test_item_size_at_limit.py; these assert + //! the same numbers at the function boundary so a developer can run them + //! with `cargo test` and no server. + + use std::collections::HashMap; + + use super::{ + ITEM_SIZE_TO_UPDATE_EXCEEDED, legacy_update_statement_size, update_statement_size, + validate_update_item_size, + }; + use crate::expression::{ExpressionMaps, UpdateAction, parse_update, tokenize}; + use crate::types::{AttributeValue, Item}; + + fn s(v: &str) -> AttributeValue { + AttributeValue::S(v.to_owned()) + } + + /// Parse an update expression into actions plus a maps table whose values + /// are keyed as the parser keys them (the `:name` placeholder without its + /// colon). + fn build(expr: &str, values: &[(&str, AttributeValue)]) -> (Vec, ExpressionMaps) { + let actions = parse_update(&tokenize(expr).unwrap()).unwrap(); + let mut map = HashMap::new(); + for (name, value) in values { + map.insert((*name).to_owned(), value.clone()); + } + (actions, ExpressionMaps::new(HashMap::new(), map)) + } + + fn statement(expr: &str, values: &[(&str, AttributeValue)]) -> usize { + let (actions, maps) = build(expr, values); + update_statement_size(&actions, &maps).unwrap() + } + + #[test] + fn single_set_is_base_clause_path_and_value() { + // 3 base + 18 clause + the written path `p` (name 1, two container + // bytes) + the 4-byte value. + assert_eq!(statement("SET p = :v", &[("v", s("wxyz"))]), 27); + } + + #[test] + fn two_set_clauses_each_cost_their_clause_and_path() { + assert_eq!( + statement("SET a = :v, b = :w", &[("v", s("wxyz")), ("w", s("y"))]), + 48, + ); + } + + #[test] + fn a_nested_path_sizes_each_level_once() { + assert_eq!(statement("SET m.a = :v", &[("v", s("wxyz"))]), 32); + } + + #[test] + fn a_list_index_target_adds_one_byte() { + // `p[1]` writes the attribute `p` (name 1, two container bytes) holding + // a one-element list (three container bytes, one per element) with the + // 4-byte value, plus 18 clause and 1 for writing through a list index: + // 3 + 18 + 1 + 10 = 32. One byte more than the same write to `p[0]`'s + // plain attribute would be without the list, which is the list-index + // target byte. + assert_eq!(statement("SET p[1] = :v", &[("v", s("wxyz"))]), 32); + } + + #[test] + fn remove_costs_its_path_skeleton_only() { + // No clause cost for REMOVE: 3 base + the path `p`. + assert_eq!(statement("REMOVE p", &[]), 5); + } + + #[test] + fn statement_over_the_limit_is_refused_with_the_update_wording() { + // SET p = :v with a ten-byte value is a 33-byte statement; the finished + // item ({p: 10 bytes}) is smaller, so the statement figure binds. + let (actions, maps) = build("SET p = :v", &[("v", s("0123456789"))]); + let mut finished = Item::new(); + finished.insert("p".to_owned(), s("0123456789")); + + assert!(validate_update_item_size(&finished, &actions, &maps, 33).is_ok()); + let err = validate_update_item_size(&finished, &actions, &maps, 32).unwrap_err(); + assert_eq!(err.to_string(), ITEM_SIZE_TO_UPDATE_EXCEEDED); + } + + #[test] + fn an_oversized_finished_item_is_refused_even_when_the_statement_fits() { + // A small statement (REMOVE q, 7 bytes) cannot rescue a finished item + // that is itself over the limit. + let (actions, maps) = build("REMOVE q", &[]); + let mut finished = Item::new(); + finished.insert("p".to_owned(), s("0123456789")); + assert!(validate_update_item_size(&finished, &actions, &maps, 5).is_err()); + } + + #[test] + fn legacy_attribute_updates_size_the_finished_value_not_the_statement() { + // The legacy form charges 3 plus the name and finished value of each + // attribute it names that remains in the item. + let (actions, mut maps) = build("SET p = :v", &[("v", s("wxyz"))]); + maps.legacy_attribute_updates = true; + let mut finished = Item::new(); + finished.insert("p".to_owned(), s("wxyz")); + // 3 base + name `p` (1) + value (4) = 8. + assert_eq!( + legacy_update_statement_size(&finished, &actions, &maps).unwrap(), + 8 + ); + } +} diff --git a/crates/engine/src/update_item.rs b/crates/engine/src/update_item.rs index 7f74c205c..d01d93e77 100755 --- a/crates/engine/src/update_item.rs +++ b/crates/engine/src/update_item.rs @@ -137,7 +137,7 @@ pub async fn handle_update_item( input.expected.as_ref().map_or(0, HashMap::len), )?; - let (condition, maps) = resolve_condition( + let (condition, mut maps) = resolve_condition( input.condition_expression.as_deref(), effective_expr_names.as_ref(), effective_expr_values.as_ref(), @@ -145,6 +145,9 @@ pub async fn handle_update_item( input.conditional_operator, &ctx.limits, )?; + // The storage layer sizes a desugared legacy update against the item limit + // the way the service sizes `AttributeUpdates`, not an expression. + maps.legacy_attribute_updates = input.attribute_updates.is_some(); // No UpdateExpression and no AttributeUpdates: no-op upsert. // Some("") still errors via parse_update_expr. @@ -227,6 +230,30 @@ pub async fn handle_update_item( // Validate that no update action targets a key attribute (REQ-DATA-003) validate_no_key_updates(&actions, &key_info, &maps)?; + // A literal operand assigned to a secondary-index key must carry the key's + // declared type. DynamoDB checks this on the statement itself, ahead of + // the condition and ahead of applying any action, so it is reported even + // when the condition would fail or another action could not be applied. + // A value computed from the existing item is checked by the storage layer + // on the written item instead. + { + let idx_refs: Vec> = key_info + .global_secondary_indexes + .iter() + .chain(key_info.local_secondary_indexes.iter()) + .map(|idx| extenddb_core::validation::IndexKeyRef { + index_name: &idx.index_name, + key_schema: &idx.key_schema, + }) + .collect(); + extenddb_core::validation::validate_index_key_literal_operands( + &actions, + &idx_refs, + &key_info.attribute_definitions, + &maps, + )?; + } + let return_old = matches!( input.return_values, ReturnValues::AllOld | ReturnValues::UpdatedOld diff --git a/crates/server/src/serve.rs b/crates/server/src/serve.rs index 9ebbaccd4..a41daea21 100644 --- a/crates/server/src/serve.rs +++ b/crates/server/src/serve.rs @@ -289,6 +289,7 @@ async fn serve_inner(params: ServeParams, port: u16) -> anyhow::Result<()> { extenddb_storage::server_components::ServerComponentsOptions::default(); component_options.bootstrap_if_uninitialized = dev_mode; + component_options.max_item_size_bytes = app_config.limits.max_item_size_bytes; // Wire the node's own bind address as the instance ID so each ExtendDB node // has a distinct node ID (to support logical clocks for sequences, etc.). diff --git a/crates/storage-cassandra/src/data/index.rs b/crates/storage-cassandra/src/data/index.rs index 4ebd72dac..e054b49d6 100644 --- a/crates/storage-cassandra/src/data/index.rs +++ b/crates/storage-cassandra/src/data/index.rs @@ -35,6 +35,20 @@ pub struct IndexMeta { pub propagation_delay_ms: Option, } +/// Build [`validation::IndexKeyRef`] views over the table's indexes for +/// secondary-index key validation on a written or updated item. +pub(crate) fn index_key_refs( + indexes: &[IndexMeta], +) -> Vec> { + indexes + .iter() + .map(|idx| extenddb_core::validation::IndexKeyRef { + index_name: &idx.index_name, + key_schema: &idx.key_schema, + }) + .collect() +} + /// Fetch all index metadata for a table from the catalog. pub async fn fetch_indexes_for_table( table_id: &str, diff --git a/crates/storage-cassandra/src/data/put_get_item.rs b/crates/storage-cassandra/src/data/put_get_item.rs index aae7567a5..0f3f93d08 100644 --- a/crates/storage-cassandra/src/data/put_get_item.rs +++ b/crates/storage-cassandra/src/data/put_get_item.rs @@ -160,16 +160,9 @@ impl CassandraEngine { // Validate index key types and emptiness before any write work. if !indexes.is_empty() { - let index_refs: Vec> = indexes - .iter() - .map(|idx| extenddb_core::validation::IndexKeyRef { - index_name: &idx.index_name, - key_schema: &idx.key_schema, - }) - .collect(); extenddb_core::validation::validate_index_keys( &item, - &index_refs, + &super::index::index_key_refs(&indexes), &key_info.attribute_definitions, ) .map_err(|e| StorageError::Validation(e.to_string()))?; diff --git a/crates/storage-cassandra/src/data/transactions.rs b/crates/storage-cassandra/src/data/transactions.rs index 639aad7e4..d9744f7b1 100644 --- a/crates/storage-cassandra/src/data/transactions.rs +++ b/crates/storage-cassandra/src/data/transactions.rs @@ -62,6 +62,13 @@ fn prepare_order_key(op: &TransactWriteOp<'_>) -> String { ) } +/// Map a key-type validation failure in PREPARE (table key, or a +/// secondary-index key checked on a written item or a literal operand) to the +/// internal reason the shared resolver recognises. +fn key_type_mismatch_cancel(e: extenddb_core::error::DynamoDbError) -> CancellationReason { + CancellationReason::key_type_mismatch(e.to_string()) +} + /// Map a `StorageError` raised while reading an item during PREPARE into a /// `CancellationReason`. A `TransactionConflict` (e.g. a concurrent transaction /// still holds the row, surfaced by `fetch_item_for_transaction` via @@ -80,6 +87,129 @@ fn prepare_read_error_reason(e: StorageError) -> CancellationReason { } impl CassandraEngine { + /// [`validation::IndexKeyRef`] views over the indexes `key_info` carries, + /// GSIs first then LSIs, for secondary-index key validation inside a + /// transaction without a catalog read. + fn transact_index_key_refs(key_info: &TableKeyInfo) -> Vec> { + key_info + .global_secondary_indexes + .iter() + .chain(key_info.local_secondary_indexes.iter()) + .map(|idx| validation::IndexKeyRef { + index_name: &idx.index_name, + key_schema: &idx.key_schema, + }) + .collect() + } + + /// A wrong-typed secondary-index key on the item a transaction Put member + /// writes is a per-item cancellation reason. The service reports it before + /// it evaluates the member's condition. Carried with the + /// internal key-type-mismatch reason and rewritten to `ValidationError` at + /// the boundary. + fn cancel_on_index_key_type_mismatch( + key_info: &TableKeyInfo, + written: &Item, + ) -> Result<(), CancellationReason> { + let idx_refs = Self::transact_index_key_refs(key_info); + if idx_refs.is_empty() { + return Ok(()); + } + validation::validate_index_key_types(written, &idx_refs, &key_info.attribute_definitions) + .map_err(key_type_mismatch_cancel) + } + + /// A literal operand an Update member assigns to a secondary-index key + /// must carry the key's declared type. The service checks this on the + /// statement itself, ahead of the member's condition and ahead of + /// applying any action. Same carrier code as the Put form. + fn cancel_on_index_key_literal_mismatch( + key_info: &TableKeyInfo, + actions: &[expression::UpdateAction], + maps: &ExpressionMaps, + ) -> Result<(), CancellationReason> { + let idx_refs = Self::transact_index_key_refs(key_info); + if idx_refs.is_empty() { + return Ok(()); + } + validation::validate_index_key_literal_operands( + actions, + &idx_refs, + &key_info.attribute_definitions, + maps, + ) + .map_err(key_type_mismatch_cancel) + } + + /// A secondary-index key left empty on the item a transaction member + /// would write is a top-level `ValidationException` that aborts the whole + /// transaction. It is carried out of PREPARE as an internal reason and + /// converted by the caller once the markers placed so far have been rolled + /// back. + fn abort_on_empty_index_key( + key_info: &TableKeyInfo, + written: &Item, + ctx: validation::SecondaryIndexEmptyContext, + ) -> Result<(), CancellationReason> { + let idx_refs = Self::transact_index_key_refs(key_info); + if idx_refs.is_empty() { + return Ok(()); + } + validation::validate_index_key_not_empty(written, &idx_refs, ctx) + .map_err(|e| CancellationReason::index_key_empty_abort(e.to_string())) + } + + /// Compute the item a transacted Update member writes over `existing`, + /// running the checks in the order the service applies them: a literal + /// operand of the wrong type on a secondary-index key (a cancellation + /// reason) ahead of the member's condition, then the condition against the + /// pre-update image, then the statement itself, then on the written item a + /// computed index-key value of the wrong type (a cancellation reason) and + /// an index key left empty (a top-level abort). + /// + /// Used on the first PREPARE pass and again on every create-race retry, so + /// the two paths cannot drift apart. + fn compute_transact_update( + key_info: &TableKeyInfo, + key: &Item, + existing: Option<&Item>, + actions: &[expression::UpdateAction], + condition: Option<&expression::Expr>, + maps: &ExpressionMaps, + return_values_on_ccf: ReturnValuesOnConditionCheckFailure, + ) -> Result { + Self::cancel_on_index_key_literal_mismatch(key_info, actions, maps)?; + let empty = Item::new(); + eval_condition( + condition, + existing.unwrap_or(&empty), + maps, + return_values_on_ccf, + existing, + )?; + let mut item = existing.cloned().unwrap_or_else(|| key.clone()); + expression::apply_update_validated(actions, &mut item, maps, &[], &[]) + .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + let idx_refs = Self::transact_index_key_refs(key_info); + if !idx_refs.is_empty() { + // A value computed from the existing item (an attribute reference, + // the fallback of if_not_exists) is only known now, and the service + // reports it with its own wording as a cancellation reason. + validation::validate_index_key_types_after_update( + &item, + &idx_refs, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + Self::abort_on_empty_index_key( + key_info, + &item, + validation::SecondaryIndexEmptyContext::UpdateExpression, + )?; + } + Ok(item) + } + /// Implementation of `DataEngine::transact_get_items`. /// /// Uses a two-phase read protocol to ensure serializability: @@ -361,7 +491,14 @@ impl CassandraEngine { ) .await; } - Err(StorageError::TransactionCanceled(reasons)) + // Key faults leave PREPARE under internal reason codes; the + // shared resolver applies the service's precedence across + // members and decides between a cancellation and a top-level + // ValidationException. + match CancellationReason::resolve_key_faults(reasons) { + Ok(reasons) => Err(StorageError::TransactionCanceled(reasons)), + Err(message) => Err(StorageError::Validation(message)), + } } } } @@ -453,7 +590,7 @@ impl CassandraEngine { &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + .map_err(key_type_mismatch_cancel)?; // Read existing item let mut existing = self @@ -468,6 +605,16 @@ impl CassandraEngine { existing.is_some() ); + // On a Put member both secondary-index key checks run ahead of + // the condition: a wrong type is a cancellation reason, an + // empty value aborts the whole transaction. + Self::cancel_on_index_key_type_mismatch(key_info, item)?; + Self::abort_on_empty_index_key( + key_info, + item, + validation::SecondaryIndexEmptyContext::Item, + )?; + // Evaluate condition let empty = Item::new(); eval_condition( @@ -559,7 +706,7 @@ impl CassandraEngine { &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + .map_err(key_type_mismatch_cancel)?; // Read existing item let existing = self @@ -660,7 +807,7 @@ impl CassandraEngine { &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + .map_err(key_type_mismatch_cancel)?; // Read existing item let mut existing = self @@ -668,27 +815,17 @@ impl CassandraEngine { .await .map_err(prepare_read_error_reason)?; - // Create item from key if not exists - let mut item = existing.clone().unwrap_or_else(|| (*key).clone()); - - // Evaluate condition against empty item if non-existent - let condition_item = if existing.is_some() { - &item - } else { - &std::collections::BTreeMap::new() - }; - eval_condition( + // Index-key checks, condition and apply, in the service's order. + let mut item = Self::compute_transact_update( + key_info, + key, + existing.as_ref(), + actions, *condition, - condition_item, maps, *return_values_on_ccf, - existing.as_ref(), )?; - // Apply update actions - expression::apply_update_validated(actions, &mut item, maps, &[], &[]) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; - // Validate item size const MAX_ITEM_SIZE_BYTES: usize = 400 * 1024; validation::validate_item_size(&item, MAX_ITEM_SIZE_BYTES) @@ -714,39 +851,19 @@ impl CassandraEngine { { Ok(()) => break, Err(r) if r.code == "TransactionConflict" && attempt < 5 => { - let committed = self.wait_for_commit_and_read(key_info, key).await?; - if let Some(winner) = committed { - eval_condition( - *condition, - &winner, - maps, - *return_values_on_ccf, - Some(&winner), - )?; - // Re-apply expression on top of winner's item. - item = winner.clone(); - expression::apply_update_validated( - actions, - &mut item, - maps, - &[], - &[], - ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; - existing = Some(winner); - } else { - // Winner rolled back; retry the insert. - existing = None; - item = (*key).clone(); - expression::apply_update_validated( - actions, - &mut item, - maps, - &[], - &[], - ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; - } + // Recompute over the winner's committed image (or + // over the key alone if the winner rolled back), + // through the same sequence as the first pass. + existing = self.wait_for_commit_and_read(key_info, key).await?; + item = Self::compute_transact_update( + key_info, + key, + existing.as_ref(), + actions, + *condition, + maps, + *return_values_on_ccf, + )?; } // Retries exhausted: surface as a conflict, matching // DynamoDB's behaviour for persistent write contention. @@ -772,7 +889,7 @@ impl CassandraEngine { &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| CancellationReason::validation_error(e.to_string()))?; + .map_err(key_type_mismatch_cancel)?; // Read existing item let mut existing = self diff --git a/crates/storage-cassandra/src/data/update_item.rs b/crates/storage-cassandra/src/data/update_item.rs index f90dd4bba..dfd52017e 100644 --- a/crates/storage-cassandra/src/data/update_item.rs +++ b/crates/storage-cassandra/src/data/update_item.rs @@ -153,6 +153,28 @@ impl CassandraEngine { .map_err(|e| StorageError::Validation(e.to_string()))?; validation::validate_item_size(&item, MAX_ITEM_SIZE_BYTES) .map_err(|e| StorageError::Validation(e.to_string()))?; + // Secondary-index keys on the written item, once the condition has + // passed and the statement is applied. Literal operands were + // checked by the engine ahead of the condition; what remains here + // is a value computed from the existing item (an attribute + // reference, the fallback of if_not_exists) of the wrong type, and + // a key left empty. Without this the value reaches the index row + // write and the malformed row breaks later writes to the same item. + if !indexes.is_empty() { + let idx_refs = super::index::index_key_refs(&indexes); + validation::validate_index_key_types_after_update( + &item, + &idx_refs, + &key_info.attribute_definitions, + ) + .map_err(|e| StorageError::Validation(e.to_string()))?; + validation::validate_index_key_not_empty( + &item, + &idx_refs, + validation::SecondaryIndexEmptyContext::UpdateExpression, + ) + .map_err(|e| StorageError::Validation(e.to_string()))?; + } let new_item = if return_new { Some(item.clone()) } else { None }; diff --git a/crates/storage-mongodb/src/data_engine.rs b/crates/storage-mongodb/src/data_engine.rs index bb75560d3..ebdf9c311 100644 --- a/crates/storage-mongodb/src/data_engine.rs +++ b/crates/storage-mongodb/src/data_engine.rs @@ -953,11 +953,12 @@ impl MongoEngine { ) .map_err(|e| TxErr::Fatal(StorageError::Validation(e.to_string())))?; - // Reject wrong-type or empty index-key attributes on - // the resulting item — D-M10, RFC-0003 §2.3. Same - // shape as put_item's up-front check, but the - // post-update item is what actually gets written. - self.validate_index_keys_for_item(key_info, &new_item) + // Secondary-index keys on the written item. Literal operands + // were checked by the engine ahead of the condition; a value + // computed from the existing item is only known now and + // carries the service's computed-value wording, and a key the + // statement leaves empty carries the update-expression wording. + self.validate_index_keys_after_update(key_info, &new_item) .await .map_err(TxErr::Fatal)?; @@ -1855,6 +1856,40 @@ impl MongoEngine { .map_err(|e| StorageError::Validation(e.to_string())) } + /// Secondary-index key checks on the item an UpdateExpression produced: + /// a computed value of the wrong type, reported with the service's + /// post-apply wording, and a key left empty, reported with the + /// update-expression wording. + async fn validate_index_keys_after_update( + &self, + key_info: &TableKeyInfo, + item: &Item, + ) -> Result<(), StorageError> { + let idx_pairs = self.fetch_index_key_schemas(&key_info.table_id).await?; + if idx_pairs.is_empty() { + return Ok(()); + } + let refs: Vec> = idx_pairs + .iter() + .map(|(name, ks)| extenddb_core::validation::IndexKeyRef { + index_name: name.as_str(), + key_schema: ks.as_slice(), + }) + .collect(); + extenddb_core::validation::validate_index_key_types_after_update( + item, + &refs, + &key_info.attribute_definitions, + ) + .map_err(|e| StorageError::Validation(e.to_string()))?; + extenddb_core::validation::validate_index_key_not_empty( + item, + &refs, + extenddb_core::validation::SecondaryIndexEmptyContext::UpdateExpression, + ) + .map_err(|e| StorageError::Validation(e.to_string())) + } + async fn sync_indexes_in_session( &self, key_info: &TableKeyInfo, @@ -2352,7 +2387,14 @@ impl MongoEngine { } Ok(AttemptOutcome::CanceledReasons(reasons)) => { let _ = session.abort_transaction().await; - return Err(StorageError::TransactionCanceled(reasons)); + // Key faults left the members as internal reasons; the + // shared resolver applies the service's cross-member + // precedence and decides between a cancellation and a + // top-level ValidationException. + return Err(match CancellationReason::resolve_key_faults(reasons) { + Ok(reasons) => StorageError::TransactionCanceled(reasons), + Err(message) => StorageError::Validation(message), + }); } Ok(AttemptOutcome::Retry) => { let _ = session.abort_transaction().await; @@ -2407,12 +2449,13 @@ impl MongoEngine { &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) })?; - // Index-key type/empty faults inside a transaction - // surface as per-item cancellation reasons (matches - // postgres data/transactions.rs). D-M10. + // Secondary-index key faults, both ahead of the condition on a + // Put member. They leave as internal reasons; the caller applies + // the service's cross-member precedence and decides between a + // cancellation and a top-level ValidationException. let idx_pairs = self .fetch_index_key_schemas(&key_info.table_id) .await @@ -2425,13 +2468,25 @@ impl MongoEngine { key_schema: ks.as_slice(), }) .collect(); - extenddb_core::validation::validate_index_keys( + extenddb_core::validation::validate_index_key_types( item, &idx_refs, &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch( + e.to_string(), + )) + })?; + extenddb_core::validation::validate_index_key_not_empty( + item, + &idx_refs, + extenddb_core::validation::SecondaryIndexEmptyContext::Item, + ) + .map_err(|e| { + TransactOpError::Cancel(CancellationReason::index_key_empty_abort( + e.to_string(), + )) })?; } @@ -2531,7 +2586,7 @@ impl MongoEngine { &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) })?; let coll_name = data_collection_name(&key_info.table_id); @@ -2623,7 +2678,31 @@ impl MongoEngine { &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; + + // A literal operand of the wrong type on a secondary-index key + // is reported on the statement, ahead of the condition and + // ahead of applying any action. + let idx_pairs = self + .fetch_index_key_schemas(&key_info.table_id) + .await + .map_err(TransactOpError::Storage)?; + let idx_refs: Vec> = idx_pairs + .iter() + .map(|(n, ks)| extenddb_core::validation::IndexKeyRef { + index_name: n.as_str(), + key_schema: ks.as_slice(), + }) + .collect(); + extenddb_core::validation::validate_index_key_literal_operands( + actions, + &idx_refs, + &key_info.attribute_definitions, + maps, + ) + .map_err(|e| { + TransactOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) })?; let coll_name = data_collection_name(&key_info.table_id); @@ -2681,28 +2760,30 @@ impl MongoEngine { TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) })?; - // Validate index-key types/emptiness on the post-update - // item; a violation here surfaces as a per-item - // cancellation reason. D-M10, RFC-0003 §2.3. - let idx_pairs = self - .fetch_index_key_schemas(&key_info.table_id) - .await - .map_err(TransactOpError::Storage)?; - if !idx_pairs.is_empty() { - let idx_refs: Vec> = idx_pairs - .iter() - .map(|(n, ks)| extenddb_core::validation::IndexKeyRef { - index_name: n.as_str(), - key_schema: ks.as_slice(), - }) - .collect(); - extenddb_core::validation::validate_index_keys( + // Secondary-index keys on the written item: a value computed + // from the existing item (an attribute reference, the fallback + // of if_not_exists) of the wrong type, reported with the + // service's computed-value wording, and a key left empty. + if !idx_refs.is_empty() { + extenddb_core::validation::validate_index_key_types_after_update( &item, &idx_refs, &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch( + e.to_string(), + )) + })?; + extenddb_core::validation::validate_index_key_not_empty( + &item, + &idx_refs, + extenddb_core::validation::SecondaryIndexEmptyContext::UpdateExpression, + ) + .map_err(|e| { + TransactOpError::Cancel(CancellationReason::index_key_empty_abort( + e.to_string(), + )) })?; } @@ -2765,7 +2846,7 @@ impl MongoEngine { &key_info.attribute_definitions, ) .map_err(|e| { - TransactOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TransactOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) })?; let coll_name = data_collection_name(&key_info.table_id); diff --git a/crates/storage-postgres/src/data/transactions.rs b/crates/storage-postgres/src/data/transactions.rs index e86c00a90..2eae73048 100644 --- a/crates/storage-postgres/src/data/transactions.rs +++ b/crates/storage-postgres/src/data/transactions.rs @@ -150,9 +150,32 @@ impl PostgresEngine { // M-3: Collect old/new items from each op for async GSI enqueue after commit. let mut op_items: Vec<(Option, Option)> = vec![(None, None); ops.len()]; let mut any_failed = false; - let mut first_invalid: Option<(usize, String)> = None; let mut failed_op: Option<(usize, StorageError)> = None; - let mut ran = vec![false; ops.len()]; + + // Key checks first, for every member, before any row is locked. They + // need only the request and the table metadata, and a fault among them + // decides the request under the service's precedence (a wrong-typed + // key on any member, else an empty index key on any member), so a + // request that is already decided never waits on a row another + // transaction holds. + { + let mut key_reasons: Vec = + vec![CancellationReason::none(); ops.len()]; + let mut any_key_fault = false; + for (i, op) in ops.iter().enumerate() { + let indexes = &table_indexes[transact_op_table_name(op)]; + if let Err(r) = transact_op_key_checks(op, indexes) { + any_key_fault = true; + key_reasons[i] = r; + } + } + if any_key_fault { + return Err(match CancellationReason::resolve_key_faults(key_reasons) { + Ok(reasons) => StorageError::TransactionCanceled(reasons), + Err(message) => StorageError::Validation(message), + }); + } + } // Run the ops in key order, not request order, so every transaction // locks its items in one global order and two cannot deadlock on each @@ -174,39 +197,15 @@ impl PostgresEngine { any_failed = true; reasons[i] = r; } - Err(TxnOpError::Validation(msg)) => { - // Up-front input validation (e.g. empty secondary-index key) - // fails the request with a top-level ValidationException. - // Report the earliest invalid op in request order. - if first_invalid.as_ref().is_none_or(|(j, _)| i < *j) { - first_invalid = Some((i, msg)); - } - } Err(TxnOpError::Storage(e)) => { // Infrastructure error, or PostgreSQL aborted the - // transaction: run no further ops. A request-earlier op - // that sorts later is then not validated. + // transaction: run no further ops. failed_op = Some((i, e)); break; } } - ran[i] = true; - // Once every op before the earliest invalid one has run, the - // answer is fixed: stop instead of locking the rest. - if let Some((j, _)) = &first_invalid - && ran[..*j].iter().all(|r| *r) - { - break; - } } - // An invalid request fails validation even if a storage error or a - // conflict abort ended the loop. After an early break, a request-earlier - // op that sorts later was never checked, so the op named is the earliest - // invalid one that ran. - if let Some((_, msg)) = first_invalid { - return Err(StorageError::Validation(msg)); - } if let Some((i, e)) = failed_op { if is_conflict_abort(&e) { // PostgreSQL broke a lock conflict by aborting this transaction. @@ -220,7 +219,15 @@ impl PostgresEngine { } if any_failed { - return Err(StorageError::TransactionCanceled(reasons)); + // Key faults left the members as internal reasons; the shared + // resolver applies the service's cross-member precedence and + // decides between a cancellation and a top-level + // ValidationException. Every member has run, so the first + // wrong-typed key in request order is the one named. + return Err(match CancellationReason::resolve_key_faults(reasons) { + Ok(reasons) => StorageError::TransactionCanceled(reasons), + Err(message) => StorageError::Validation(message), + }); } // Write stream records atomically within the transaction (BLOCKER #1 fix). @@ -430,12 +437,74 @@ pub(super) fn index_key_refs(indexes: &[IndexMeta]) -> Vec, + indexes: &[IndexMeta], +) -> Result<(), CancellationReason> { + match op { + TransactWriteOp::Put { key_info, item, .. } => { + validation::validate_item_keys( + item, + &key_info.key_schema, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; + let idx_refs = index_key_refs(indexes); + validation::validate_index_key_types( + item, + &idx_refs, + super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; + validation::validate_index_key_not_empty( + item, + &idx_refs, + validation::SecondaryIndexEmptyContext::Item, + ) + .map_err(|e| CancellationReason::index_key_empty_abort(e.to_string())) + } + TransactWriteOp::Update { + key_info, + key, + actions, + maps, + .. + } => { + validation::validate_batch_key_only( + key, + &key_info.key_schema, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; + validation::validate_index_key_literal_operands( + actions, + &index_key_refs(indexes), + super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), + maps, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string())) + } + TransactWriteOp::Delete { key_info, key, .. } + | TransactWriteOp::ConditionCheck { key_info, key, .. } => { + validation::validate_batch_key_only( + key, + &key_info.key_schema, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string())) + } + } +} + enum TxnOpError { /// User-driven failure — becomes a per-item cancellation reason. Cancel(CancellationReason), - /// Up-front input validation failure — aborts the whole transaction with a - /// top-level `ValidationException` (not a per-item cancellation reason). - Validation(String), /// Infrastructure failure — bubbles up as `StorageError::Internal`. Storage(StorageError), } @@ -471,28 +540,9 @@ async fn execute_transact_write_op( // Key type validation inside the transaction so mismatches produce // TransactionCanceledException with ValidationError cancellation // reasons, matching real DynamoDB behavior. - validation::validate_item_keys( - item, - &key_info.key_schema, - &key_info.attribute_definitions, - ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; - // Secondary-index key faults split by kind: a type mismatch is a - // per-item cancellation reason; an empty index key is up-front input - // validation (a top-level ValidationException). - let idx_refs = index_key_refs(indexes); - validation::validate_index_key_types( - item, - &idx_refs, - super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), - ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; - validation::validate_index_key_not_empty( - item, - &idx_refs, - validation::SecondaryIndexEmptyContext::Item, - ) - .map_err(|e| TxnOpError::Validation(e.to_string()))?; + // Key checks ran in the pre-pass; repeated so the arm stays + // correct if called on its own. + transact_op_key_checks(op, indexes).map_err(TxnOpError::Cancel)?; let mut existing = fetch_item_for_update(tx, key_info, item) .await .map_err(TxnOpError::Storage)?; @@ -590,7 +640,9 @@ async fn execute_transact_write_op( &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; let mut existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; @@ -635,12 +687,8 @@ async fn execute_transact_write_op( return_values_on_ccf, .. } => { - validation::validate_batch_key_only( - key, - &key_info.key_schema, - &key_info.attribute_definitions, - ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + transact_op_key_checks(op, indexes).map_err(TxnOpError::Cancel)?; + let idx_refs = index_key_refs(indexes); let mut existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; @@ -649,7 +697,6 @@ async fn execute_transact_write_op( // lost and the loser's expression must re-apply on top of the // winner's committed item (the merge semantics measured against // the real service on the non-transactional UpdateItem path). - let idx_refs = index_key_refs(indexes); let compute_item = |base: Option<&Item>| -> Result { let mut item = base.cloned().unwrap_or_else(|| (*key).clone()); expression::apply_update_validated( @@ -662,27 +709,32 @@ async fn execute_transact_write_op( .map_err(|e| { TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())) })?; - // Validate post-update item size - validation::validate_item_size(&item, max_item_size_bytes).map_err(|e| { - TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())) - })?; - // Secondary-index key validation on the post-update item: a type - // mismatch is a cancellation reason; setting an index key to an - // empty value is a top-level ValidationException. - validation::validate_index_key_types( + // A transacted Update is sized on the finished item, with the + // update wording in the cancellation reason. + validation::validate_transact_update_item_size(&item, max_item_size_bytes) + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())) + })?; + // Secondary-index keys on the written item: a value computed from + // the existing item (an attribute reference, the fallback of + // if_not_exists) of the wrong type, reported with the service's + // computed-value wording, and a key left empty. + validation::validate_index_key_types_after_update( &item, &idx_refs, super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), ) .map_err(|e| { - TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())) + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) })?; validation::validate_index_key_not_empty( &item, &idx_refs, validation::SecondaryIndexEmptyContext::UpdateExpression, ) - .map_err(|e| TxnOpError::Validation(e.to_string()))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::index_key_empty_abort(e.to_string())) + })?; Ok(item) }; // Evaluate condition against empty item if non-existent (DynamoDB semantics) @@ -771,7 +823,9 @@ async fn execute_transact_write_op( &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; let mut existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; diff --git a/crates/storage-postgres/src/data/update_item.rs b/crates/storage-postgres/src/data/update_item.rs index 80ab9a515..e48044e83 100755 --- a/crates/storage-postgres/src/data/update_item.rs +++ b/crates/storage-postgres/src/data/update_item.rs @@ -66,6 +66,21 @@ impl PostgresEngine { // write is one the backfill sees. Vector indexes still have the window. let (indexes, vector_metas, stamp) = fetch_write_path_indexes(&key_info.table_id, &self.pool).await?; + // Literal operands assigned to a secondary-index key, checked on the + // statement ahead of the condition as the service checks them. The + // engine ran this check against its cached key info, which does not list + // a GSI until the UpdateTable that adds it returns; this read lists it + // from the moment its CREATING row commits, so a literal of the wrong + // type for an index still being built gets the same per-index wording. + if !indexes.is_empty() { + validation::validate_index_key_literal_operands( + actions, + &super::transactions::index_key_refs(&indexes), + super::index::fresh_attr_defs(&indexes, &key_info.attribute_definitions), + maps, + ) + .map_err(|e| StorageError::Validation(e.to_string()))?; + } // Read whenever anything can propagate, secondary or vector. Gating this on // the secondary set alone made a vector-only table ignore the configured // delay and apply its vector index inline, while a TransactWriteItems on the @@ -187,17 +202,20 @@ impl PostgresEngine { ) .map_err(|e| StorageError::Validation(e.to_string()))?; - // Validate post-update item size (400 KB limit) - validation::validate_item_size(&item, self.max_item_size_bytes) + // Size the update the way the service does: what the statement + // writes plus a fixed cost per clause, against the item limit. + validation::validate_update_item_size(&item, actions, maps, self.max_item_size_bytes) .map_err(|e| StorageError::Validation(e.to_string()))?; - // Secondary-index key validation on the post-update item, as the - // TransactWriteItems update path and SQLite already do: a wrong-typed - // index key or an index key set to an empty value is a - // ValidationException, not an item the index then leaves out. + // Secondary-index keys on the written item, as the transacted Update + // path and the SQLite backend already do. Literal operands were + // checked ahead of the condition, above; a value computed from the + // existing item is only known now and carries the service's + // computed-value wording. An index key left empty is a + // ValidationException, not a malformed index row. if !indexes.is_empty() { let idx_refs = super::transactions::index_key_refs(&indexes); - validation::validate_index_key_types( + validation::validate_index_key_types_after_update( &item, &idx_refs, super::index::fresh_attr_defs(&indexes, &key_info.attribute_definitions), diff --git a/crates/storage-postgres/src/lib.rs b/crates/storage-postgres/src/lib.rs index e9f1ae2e9..acb1aef56 100755 --- a/crates/storage-postgres/src/lib.rs +++ b/crates/storage-postgres/src/lib.rs @@ -745,7 +745,7 @@ fn server_components_factory( // PostgreSQL bootstrap needs operator input (databases, roles, admin // credentials), so `bootstrap_if_uninitialized` is not honored here: // an uninitialized catalog fails with the explicit-`init` guidance. - _options: extenddb_storage::server_components::ServerComponentsOptions, + options: extenddb_storage::server_components::ServerComponentsOptions, ) -> std::pin::Pin< Box> + Send>, > { @@ -758,7 +758,7 @@ fn server_components_factory( let pg_config = PostgresConfig { connection_string: connection_string.clone(), pool_size: max_connections, - max_item_size_bytes: 400_000, + max_item_size_bytes: options.max_item_size_bytes, }; // Create PostgresEngine diff --git a/crates/storage-postgres/tests/gsi_online_build.rs b/crates/storage-postgres/tests/gsi_online_build.rs index 969ede4a8..3f769ab73 100644 --- a/crates/storage-postgres/tests/gsi_online_build.rs +++ b/crates/storage-postgres/tests/gsi_online_build.rs @@ -919,6 +919,86 @@ async fn update_item_rejects_a_wrong_typed_gsi_key() { s.cleanup().await; } +/// The same UpdateItem while UpdateTable is still building the GSI, carrying the +/// key info cached before the UpdateTable, as every write does until it returns. +/// That key info lists no index, so the engine's statement check passes it; the +/// storage layer's catalog read lists the CREATING index, and the refusal is the +/// service's per-index wording for a literal operand, not the wording for a value +/// computed from the item. The build is held open by a client holding one base +/// row: the backfill passes over it and then waits for its lock. +#[tokio::test] +async fn update_item_during_a_gsi_build_names_the_index_for_a_wrong_typed_literal() { + let test = "update_item_during_a_gsi_build_names_the_index_for_a_wrong_typed_literal"; + if base_conn().is_none() { + return skip(test); + } + let s = scratch().await; + let cached = create_table(&s).await; + put(&s, &cached, item(1, Some("held"), None)).await; + put(&s, &cached, item(2, Some("a"), None)).await; + let table_id = data_table_id(&s.db).await; + let mut holder = s.db.begin().await.expect("begin the holder"); + sqlx::query(&format!( + "SELECT 1 FROM \"_ddb_{table_id}\" WHERE pk = 'p1' AND sk_n = 1 FOR UPDATE" + )) + .execute(&mut *holder) + .await + .expect("hold a base row"); + + let mut build = Box::pin(add_gsi(&s, false)); + let early = tokio::time::timeout(Duration::from_secs(3), &mut build).await; + assert!( + early.is_err(), + "UpdateTable returned while its backfill waited on a held row" + ); + assert_eq!(ids(&s.db).await.2, "CREATING"); + + let actions = extenddb_core::expression::parse_update( + &extenddb_core::expression::tokenize("SET g = :v").expect("tokenize"), + ) + .expect("parse"); + let maps = ExpressionMaps::new( + std::collections::HashMap::new(), + std::collections::HashMap::from([("v".to_owned(), AttributeValue::N("9".to_owned()))]), + ); + let result = s + .engine + .update_item( + &cached, + &item(2, None, None), + &actions, + false, + false, + None, + &maps, + None, + ) + .await; + let Err(extenddb_storage::error::StorageError::Validation(msg)) = result else { + panic!("a wrong-typed index key during the build was not a ValidationException"); + }; + assert_eq!( + msg, + "One or more parameter values were invalid: Type mismatch for Index Key g \ + Expected: S Actual: N IndexName: gidx" + ); + + holder.commit().await.expect("release the held row"); + tokio::time::timeout(Duration::from_secs(30), &mut build) + .await + .expect("UpdateTable finished once the row was released"); + drop(build); + assert_eq!(ids(&s.db).await.2, "ACTIVE"); + let g: Option = sqlx::query_scalar(&format!( + "SELECT item_data->'g'->>'S' FROM \"_ddb_{table_id}\" WHERE pk = 'p2' AND sk_n = 2" + )) + .fetch_one(&s.db) + .await + .expect("read the refused item"); + assert_eq!(g.as_deref(), Some("a"), "the refused update wrote nothing"); + s.cleanup().await; +} + /// A base row held open by a client does not hold up the rest of its queue /// partition. The worker claims the lowest due id; with the held item's rows /// first in line, every row behind them used to wait for the whole hold, since diff --git a/crates/storage-sqlite/src/data/transactions.rs b/crates/storage-sqlite/src/data/transactions.rs index e9b5f8f5b..dc37c90c2 100644 --- a/crates/storage-sqlite/src/data/transactions.rs +++ b/crates/storage-sqlite/src/data/transactions.rs @@ -100,6 +100,30 @@ impl SqliteEngine { .await?; } + // Key checks first, for every member, before any row is read or + // locked. They need only the request and the table metadata, and a + // fault among them decides the request under the service's precedence + // (a wrong-typed key on any member, else an empty index key on any + // member), so a decided request never waits on a later member's row. + { + let mut key_reasons: Vec = + vec![CancellationReason::none(); ops.len()]; + let mut any_key_fault = false; + for (i, op) in ops.iter().enumerate() { + let indexes = &table_indexes[op_table_name(op)]; + if let Err(r) = transact_op_key_checks(op, indexes) { + any_key_fault = true; + key_reasons[i] = r; + } + } + if any_key_fault { + return Err(match CancellationReason::resolve_key_faults(key_reasons) { + Ok(reasons) => StorageError::TransactionCanceled(reasons), + Err(message) => StorageError::Validation(message), + }); + } + } + let mut reasons: Vec = Vec::with_capacity(ops.len()); let mut op_items: Vec<(Option, Option)> = Vec::with_capacity(ops.len()); let mut any_failed = false; @@ -124,19 +148,18 @@ impl SqliteEngine { any_failed = true; reasons.push(r); } - Err(TxnOpError::Validation(msg)) => { - // Up-front input validation (e.g. empty secondary-index key): - // abort the whole transaction with a top-level - // ValidationException, not a per-item cancellation reason. - return Err(StorageError::Validation(msg)); - } Err(TxnOpError::Storage(e)) => return Err(e), } } if any_failed { - // Dropping `tx` without commit rolls back all writes. - return Err(StorageError::TransactionCanceled(reasons)); + // Dropping `tx` without commit rolls back all writes. Key faults + // left the members as internal reasons; the shared resolver applies + // the service's cross-member precedence. + return Err(match CancellationReason::resolve_key_faults(reasons) { + Ok(reasons) => StorageError::TransactionCanceled(reasons), + Err(message) => StorageError::Validation(message), + }); } // Capture stream records after all writes are staged. @@ -252,9 +275,6 @@ fn op_key_info<'a>(op: &'a TransactWriteOp<'_>) -> &'a extenddb_core::types::Tab enum TxnOpError { Cancel(CancellationReason), - /// Up-front input validation failure — aborts the whole transaction with a - /// top-level `ValidationException` (not a per-item cancellation reason). - Validation(String), Storage(StorageError), } @@ -270,46 +290,92 @@ pub(crate) fn index_key_refs(indexes: &[IndexMeta]) -> Vec, +/// The key checks a transaction member can fail before any row is read or +/// locked: the table key's shape and types, and the secondary-index keys the +/// member supplies in the request itself (a Put item's attributes, an Update +/// statement's literal operands). Faults carry the internal reasons that +/// `CancellationReason::resolve_key_faults` ranks. Values computed from the +/// existing item are checked later, once the row is read. +fn transact_op_key_checks( op: &TransactWriteOp<'_>, indexes: &[IndexMeta], - max_item_size_bytes: usize, - system_delay: u64, -) -> Result<(Option, Option), TxnOpError> { +) -> Result<(), CancellationReason> { match op { - TransactWriteOp::Put { - key_info, - item, - condition, - maps, - return_values_on_ccf, - .. - } => { + TransactWriteOp::Put { key_info, item, .. } => { validation::validate_item_keys( item, &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; - // Secondary-index key faults split by kind: a type mismatch is a - // per-item cancellation reason; an empty index key is up-front input - // validation (a top-level ValidationException). + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; let idx_refs = index_key_refs(indexes); validation::validate_index_key_types( item, &idx_refs, super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; validation::validate_index_key_not_empty( item, &idx_refs, validation::SecondaryIndexEmptyContext::Item, ) - .map_err(|e| TxnOpError::Validation(e.to_string()))?; + .map_err(|e| CancellationReason::index_key_empty_abort(e.to_string())) + } + TransactWriteOp::Update { + key_info, + key, + actions, + maps, + .. + } => { + validation::validate_batch_key_only( + key, + &key_info.key_schema, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string()))?; + validation::validate_index_key_literal_operands( + actions, + &index_key_refs(indexes), + super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), + maps, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string())) + } + TransactWriteOp::Delete { key_info, key, .. } + | TransactWriteOp::ConditionCheck { key_info, key, .. } => { + validation::validate_batch_key_only( + key, + &key_info.key_schema, + &key_info.attribute_definitions, + ) + .map_err(|e| CancellationReason::key_type_mismatch(e.to_string())) + } + } +} + +/// Execute one transact-write op, returning `(old, new)` images for stream +/// capture, or a cancellation reason on a failed condition / validation. +async fn execute_transact_write_op( + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + op: &TransactWriteOp<'_>, + indexes: &[IndexMeta], + max_item_size_bytes: usize, + system_delay: u64, +) -> Result<(Option, Option), TxnOpError> { + match op { + TransactWriteOp::Put { + key_info, + item, + condition, + maps, + return_values_on_ccf, + .. + } => { + // Key checks ran in the pre-pass; repeated so the arm stays + // correct if called on its own. + transact_op_key_checks(op, indexes).map_err(TxnOpError::Cancel)?; let existing = fetch_item_for_update(tx, key_info, item) .await .map_err(TxnOpError::Storage)?; @@ -352,7 +418,9 @@ async fn execute_transact_write_op( &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; let existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; @@ -391,12 +459,8 @@ async fn execute_transact_write_op( return_values_on_ccf, .. } => { - validation::validate_batch_key_only( - key, - &key_info.key_schema, - &key_info.attribute_definitions, - ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + transact_op_key_checks(op, indexes).map_err(TxnOpError::Cancel)?; + let idx_refs = index_key_refs(indexes); let existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; @@ -417,25 +481,31 @@ async fn execute_transact_write_op( &key_info.attribute_definitions, ) .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; - validation::validate_item_size(&item, max_item_size_bytes).map_err(|e| { - TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())) - })?; - // Secondary-index key validation on the post-update item: a type - // mismatch is a cancellation reason; setting an index key to an - // empty value is a top-level ValidationException. - let idx_refs = index_key_refs(indexes); - validation::validate_index_key_types( + // A transacted Update is sized on the finished item, with the update + // wording in the cancellation reason. + validation::validate_transact_update_item_size(&item, max_item_size_bytes).map_err( + |e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())), + )?; + // Secondary-index keys on the written item: a value computed from + // the existing item (an attribute reference, the fallback of + // if_not_exists) of the wrong type, reported with the service's + // computed-value wording, and a key left empty. + validation::validate_index_key_types_after_update( &item, &idx_refs, super::index::fresh_attr_defs(indexes, &key_info.attribute_definitions), ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; validation::validate_index_key_not_empty( &item, &idx_refs, validation::SecondaryIndexEmptyContext::UpdateExpression, ) - .map_err(|e| TxnOpError::Validation(e.to_string()))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::index_key_empty_abort(e.to_string())) + })?; upsert_item_in_tx(tx, key_info, &item) .await .map_err(TxnOpError::Storage)?; @@ -466,7 +536,9 @@ async fn execute_transact_write_op( &key_info.key_schema, &key_info.attribute_definitions, ) - .map_err(|e| TxnOpError::Cancel(CancellationReason::validation_error(e.to_string())))?; + .map_err(|e| { + TxnOpError::Cancel(CancellationReason::key_type_mismatch(e.to_string())) + })?; let existing = fetch_item_for_update(tx, key_info, key) .await .map_err(TxnOpError::Storage)?; diff --git a/crates/storage-sqlite/src/data/update_item.rs b/crates/storage-sqlite/src/data/update_item.rs index ae18ae1e9..0f9c6db09 100644 --- a/crates/storage-sqlite/src/data/update_item.rs +++ b/crates/storage-sqlite/src/data/update_item.rs @@ -42,6 +42,22 @@ impl SqliteEngine { // Read the index set after acquiring the write lock so a concurrently // added GSI (UpdateTable holds the same lock) is not missed. let indexes = fetch_indexes_for_table(&key_info.table_id, &self.pool).await?; + // Literal operands assigned to a secondary-index key, checked on the + // statement ahead of the condition as the service checks them. The + // engine ran this check against its cached key info, which does not list + // a GSI until the UpdateTable that adds it returns; a write that read the + // cache before then and takes the write lock after the build sees the GSI + // only in this read, and a literal of the wrong type for it gets the same + // per-index wording. + if !indexes.is_empty() { + validation::validate_index_key_literal_operands( + actions, + &index_key_refs(&indexes), + super::index::fresh_attr_defs(&indexes, &key_info.attribute_definitions), + maps, + ) + .map_err(|e| StorageError::Validation(e.to_string()))?; + } let mut tx = self .pool @@ -72,7 +88,9 @@ impl SqliteEngine { &key_info.attribute_definitions, ) .map_err(|e| StorageError::Validation(e.to_string()))?; - validation::validate_item_size(&item, self.max_item_size_bytes) + // Size the update the way the service does: what the statement writes + // plus a fixed cost per clause, against the item limit. + validation::validate_update_item_size(&item, actions, maps, self.max_item_size_bytes) .map_err(|e| StorageError::Validation(e.to_string()))?; // Secondary-index key validation on the post-update item, matching the // TransactWriteItems update path: a wrong-typed index key attribute or @@ -80,7 +98,10 @@ impl SqliteEngine { // rather than silently producing a malformed / unmatchable index row. if !indexes.is_empty() { let idx_refs = index_key_refs(&indexes); - validation::validate_index_key_types( + // Literal operands were checked ahead of the condition, above; a + // value computed from the existing item is only known now and + // carries the service's computed-value wording. + validation::validate_index_key_types_after_update( &item, &idx_refs, super::index::fresh_attr_defs(&indexes, &key_info.attribute_definitions), @@ -157,3 +178,82 @@ impl SqliteEngine { Ok((old_ret, new_ret)) } } + +#[cfg(test)] +mod tests { + use crate::SqliteEngine; + use extenddb_core::expression::{ExpressionMaps, parse_update, tokenize}; + use extenddb_core::types::{AttributeValue, CreateTableInput, Item, UpdateTableInput}; + use extenddb_storage::error::StorageError; + use extenddb_storage::{DataEngine, TableEngine}; + use serde_json::json; + + /// UpdateItem carrying the key info cached before an UpdateTable added a GSI, + /// as a write does that read the cache before the UpdateTable returned and + /// took the write lock after the build. A literal of the wrong type for the + /// new index key gets the service's per-index wording, the answer once the + /// cache lists the index, not the wording for a value computed from the item. + #[tokio::test] + async fn a_wrong_typed_literal_for_a_gsi_the_cached_key_info_lacks_names_the_index() { + let engine = SqliteEngine::new(":memory:", 1, "us-east-1", 409_600) + .await + .expect("engine"); + crate::schema::apply(&engine.pool).await.expect("schema"); + let account = "000000000000"; + sqlx::query("INSERT INTO accounts (account_id, account_name) VALUES (?, 'default')") + .bind(account) + .execute(&engine.pool) + .await + .expect("account"); + let input: CreateTableInput = serde_json::from_value(json!({ + "TableName": "t", + "KeySchema": [{"AttributeName": "pk", "KeyType": "HASH"}], + "AttributeDefinitions": [{"AttributeName": "pk", "AttributeType": "S"}], + "BillingMode": "PAY_PER_REQUEST" + })) + .expect("input"); + engine + .create_table_impl(account, input, false) + .await + .expect("create table"); + sqlx::query("UPDATE tables SET table_status = 'ACTIVE' WHERE table_name = 't'") + .execute(&engine.pool) + .await + .expect("activate the table"); + let cached = engine.table_key_info(account, "t").await.expect("key info"); + let update: UpdateTableInput = serde_json::from_value(json!({ + "TableName": "t", + "AttributeDefinitions": [{"AttributeName": "g", "AttributeType": "S"}], + "GlobalSecondaryIndexUpdates": [{"Create": { + "IndexName": "gidx", + "KeySchema": [{"AttributeName": "g", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"} + }}] + })) + .expect("update input"); + engine + .update_table_impl(account, update) + .await + .expect("add the GSI"); + + let key: Item = serde_json::from_value(json!({"pk": {"S": "a"}})).expect("key"); + let actions = parse_update(&tokenize("SET g = :v").expect("tokenize")).expect("parse"); + let maps = ExpressionMaps::new( + std::collections::HashMap::new(), + std::collections::HashMap::from([("v".to_owned(), AttributeValue::N("9".to_owned()))]), + ); + let result = engine + .update_item(&cached, &key, &actions, false, false, None, &maps, None) + .await; + let Err(StorageError::Validation(msg)) = result else { + panic!("a wrong-typed index key was not a ValidationException: {result:?}"); + }; + assert_eq!( + msg, + "One or more parameter values were invalid: Type mismatch for Index Key g \ + Expected: S Actual: N IndexName: gidx" + ); + let stored = engine.get_item(&cached, &key).await.expect("get the item"); + assert!(stored.is_none(), "the refused update wrote nothing"); + } +} diff --git a/crates/storage-sqlite/src/lib.rs b/crates/storage-sqlite/src/lib.rs index 3953fc350..75c91336f 100644 --- a/crates/storage-sqlite/src/lib.rs +++ b/crates/storage-sqlite/src/lib.rs @@ -193,9 +193,6 @@ use extenddb_storage::server_components::{BackendError, ServerComponents}; use crate::hooks::SqliteRuntimeHooks; -/// Maximum DynamoDB item size in bytes (post-update validation bound). -const MAX_ITEM_SIZE_BYTES: usize = 400_000; - fn sqlite_server_components_factory( config: &(dyn extenddb_storage::config::StorageConfig + 'static), region: &str, @@ -214,12 +211,13 @@ fn sqlite_server_components_factory( tracing::debug!("holding serve lock {}", lock.path().display()); std::sync::Arc::new(lock) }); - let mut engine = SqliteEngine::new(&db_path, pool_size, ®ion, MAX_ITEM_SIZE_BYTES) - .await - .map_err(|e| BackendError::ConnectionFailed { - backend: "sqlite".to_owned(), - details: e.to_string(), - })?; + let mut engine = + SqliteEngine::new(&db_path, pool_size, ®ion, options.max_item_size_bytes) + .await + .map_err(|e| BackendError::ConnectionFailed { + backend: "sqlite".to_owned(), + details: e.to_string(), + })?; // In-memory databases do not persist across the `init` process, so the // catalog must be bootstrapped here, at serve time, on the engine's own diff --git a/crates/storage/src/server_components.rs b/crates/storage/src/server_components.rs index 2ea59c9c9..a2c3cc288 100644 --- a/crates/storage/src/server_components.rs +++ b/crates/storage/src/server_components.rs @@ -86,7 +86,7 @@ impl std::fmt::Display for BackendError { impl std::error::Error for BackendError {} /// Options for server-component creation, beyond the storage config itself. -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Copy)] #[non_exhaustive] pub struct ServerComponentsOptions { /// Bootstrap the catalog at serve time when it is not initialized, @@ -95,6 +95,19 @@ pub struct ServerComponentsOptions { /// Backends whose bootstrap requires operator input ignore this and fail /// as usual; backends with a self-contained bootstrap (SQLite) honor it. pub bootstrap_if_uninitialized: bool, + /// The item size limit the storage layer applies to the finished item of + /// an update, standalone or transacted. Comes from `limits.max_item_size_bytes` + /// so the storage layer and the engine's put path enforce one figure. + pub max_item_size_bytes: usize, +} + +impl Default for ServerComponentsOptions { + fn default() -> Self { + Self { + bootstrap_if_uninitialized: false, + max_item_size_bytes: extenddb_core::limits::LimitsConfig::default().max_item_size_bytes, + } + } } /// Factory function type for creating server components. diff --git a/devtools/run-tests b/devtools/run-tests index 421a97b3a..ae7984896 100755 --- a/devtools/run-tests +++ b/devtools/run-tests @@ -141,9 +141,10 @@ case "$BACKEND" in exit 1 ;; esac -# Tests that depend on the backend (tests/test_ttl.py) read this rather than -# guess from a config file. Only exported when --backend was given: the -# SQLite CI leg runs without it. +# Tests that depend on the backend (tests/test_ttl.py, and the update-size +# classes in tests/test_item_size_at_limit.py that apply only to the PostgreSQL +# and SQLite storage layers) read this rather than guess from a config file. +# Only exported when --backend was given: the SQLite CI leg runs without it. if [[ -n "${BACKEND_GIVEN:-}" ]]; then export EXTENDDB_TEST_BACKEND="$BACKEND" fi diff --git a/docs/differences-from-dynamodb.md b/docs/differences-from-dynamodb.md index 2d6469b7e..3aed5bb67 100755 --- a/docs/differences-from-dynamodb.md +++ b/docs/differences-from-dynamodb.md @@ -92,6 +92,7 @@ adaptation when switching between ExtendDB and the real service. | Multi-part base table keys | Not supported | Preview extension (opt-in via `enable_multipart_keys` setting). Standard single/composite keys work identically. | | GSI throughput on a `PROVISIONED` table | Every GSI needs `ProvisionedThroughput` of at least 1 read and 1 write unit | A GSI `ProvisionedThroughput` below 1 (including 0/0) is `ValidationException` on `CreateTable` of a `PROVISIONED` table and on `UpdateTable`'s GSI `Create` ("ReadCapacityUnits and WriteCapacityUnits must both be greater than or equal to 1 for index: "; the service's text was not captured). A GSI with no `ProvisionedThroughput` is still accepted on a `PROVISIONED` table, and `UpdateTable` to `PROVISIONED` leaves an on-demand table's GSIs without one; see the `BillingModeOverride` row under Backup and Restore for how a backup of such a table restores | | Projected attributes across all indexes | At most 100 | Enforced on `CreateTable` (and so on `RestoreTableFromBackup`, which validates its target the same way) and on `UpdateTable`'s GSI `Create`: every `NonKeyAttributes` entry of every GSI and LSI is counted, so an attribute projected into two indexes counts twice. Over 100 is `ValidationException` ("Number of projected attributes in all indexes exceeds limit of 100, projected: "; the service's text was not captured) | +| Index key validation on `UpdateItem` and transaction writes | A secondary-index key attribute must carry its declared scalar type and may not be empty. On `UpdateItem` a literal operand of the wrong type (`SET k = :v`, `ADD`, `DELETE`, `list_append` of literals) is refused on the statement, ahead of the `ConditionExpression`, naming the attribute and the index; a value computed from the item (an attribute reference, the fallback of `if_not_exists`) is refused after the condition passes with the message "The update expression attempted to update the secondary index key to unsupported type"; a key left empty is refused after the condition passes with the update-expression wording. In `TransactWriteItems` a wrong-typed key (table or index) on any member cancels the transaction with the first such member's `ValidationError` in position and `None` elsewhere; otherwise an empty index key on any member is a top-level `ValidationException`; otherwise members report their own conditions. On a `Put` member both checks come ahead of the member's condition. (Measured on the service.) | Matches on every backend. Two known shared differences: an empty index key supplied through the legacy `AttributeUpdates` parameter is reported with the update-expression wording where the service uses the `PutItem` wording naming the index; and a request whose own value exceeds 400 KB is refused for size by the service before any index-key check, where ExtendDB reports the index-key fault first. | ## Backup and Restore @@ -139,6 +140,37 @@ The same on all four backends. | `OnDemandThroughputOverride` | Applied on an on-demand restore, as on the service (measured on DynamoDB): equal to the backup's, changed, with one member, or on a `PROVISIONED` backup restored with `BillingModeOverride` `PAY_PER_REQUEST`. It replaces the backup's on-demand throughput as a whole: a member left out means no maximum, as on `CreateTable`, so `{}` restores a table with no maxima. On a `PROVISIONED` restore (from a `PROVISIONED` backup or by override) it is refused with `ValidationException`, as is a member other than `MaxReadRequestUnits` and `MaxWriteRequestUnits`. Measured on SQLite | | `VectorIndexOverride` | Refused with `ValidationException` | +## Item Size + +ExtendDB enforces the 400 KB (409,600-byte) item-size limit. This section is the +single reference for how an item and an update are sized; `docs/dynamodb-limits.md` +records the limit and points here. Every figure was measured against the service +in us-east-1 on 2026-10-19. + +How an item is sized: its attribute names plus its values. A string or binary +value costs its byte length. A number costs `1 + ceil(integer significant digits +/ 2) + ceil(fraction significant digits / 2)`, plus one for a sign, the two +halves rounded up separately, so `1.5` is 3 bytes and `3.14159` is 5. A list +costs 3 bytes plus one per element; a map costs 3 bytes plus, per entry, the +entry name, its value, and one more. + +How a standalone `UpdateItem` is sized: by the statement, not the finished item. +A base of 3; 18 per `SET` or `ADD` clause; a skeleton of the written paths (each +map level 3 bytes plus one per named entry, each list level 3 bytes plus one per +element, untouched siblings left out) whose `SET` or `ADD` leaves carry their +value-placeholder bytes; one more per clause written through a list index; 26 +plus the name length per function call; 34 per `+` and 37 per `-`; and 15 plus +the name per document path operand (14 for a nested attribute, 15 for an index). +`REMOVE` and `DELETE` cost their path skeleton only. The legacy `AttributeUpdates` +form costs 3 plus the name and finished value of each attribute it names that +remains. The key and untouched attributes are left out. Both the statement figure +and the finished item are checked against the limit, and a breach refuses with +`Item size to update has exceeded the maximum allowed size`. + +| Area | DynamoDB | ExtendDB | +|------|----------|------| +| `UpdateItem` item-size accounting | In 31 of 33 commercial regions a standalone `UpdateItem` is sized by the statement (the rule above). At a one-byte key the largest finished item it can produce is 409,581 bytes. Two regions (`eu-south-2`, `me-central-1`) size an `UpdateItem` on the finished item, like a `PutItem`. | PostgreSQL and SQLite follow the 31-region statement rule. A transacted `Update` is sized on the finished item on both, as on the service. Cassandra sizes every update on the finished item against a fixed 409,600 bytes; MongoDB applies no item-size limit on updates. | + ## Capacity and Throttling | Area | DynamoDB | ExtendDB | diff --git a/docs/dynamodb-limits.md b/docs/dynamodb-limits.md index 503836958..55d5de06b 100755 --- a/docs/dynamodb-limits.md +++ b/docs/dynamodb-limits.md @@ -1,6 +1,6 @@ # DynamoDB Limits Enforcement Status -Last updated: 2026-04-23 (P42) +Last updated: 2026-10-19 Source: [AWS DynamoDB Service Quotas](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/ServiceQuotas.html) @@ -38,7 +38,7 @@ Source: [AWS DynamoDB Service Quotas](https://docs.aws.amazon.com/amazondynamodb | Limit | DynamoDB Value | Status | Notes | |-------|---------------|--------|-------------------------------------------------------------------------------| -| Maximum item size | 400 KB (409,600 bytes) | Enforced | `LimitsConfig::max_item_size_bytes`, validated on PutItem and post-UpdateItem | +| Maximum item size | 400 KB (409,600 bytes) | Enforced | `LimitsConfig::max_item_size_bytes`. The engine enforces it on PutItem, BatchWriteItem, the transaction Put path and ImportTable; the PostgreSQL and SQLite storage layers enforce it on UpdateItem and the transaction Update path. Cassandra applies a fixed 409,600 to the finished item of an update with the put wording; MongoDB applies no limit on updates. The sizing calculation (item bytes, number cost, and the statement-based UpdateItem rule) lives in [differences-from-dynamodb.md](differences-from-dynamodb.md#item-size). | | Partition key size | 1–2,048 bytes | Enforced | `validate_key_sizes`, `LimitsConfig::max_partition_key_size_bytes` | | Sort key size | 1–1,024 bytes | Enforced | `validate_key_sizes`, `LimitsConfig::max_sort_key_size_bytes` | | Attribute name size | 1–64 KB (65,535 bytes) | Partial | `validate_attribute_name_sizes`, `LimitsConfig::max_attribute_name_bytes`; top-level names and nested map keys at every depth, on PutItem, BatchWriteItem.PutRequest, TransactWriteItems.Put, and ImportTable. Not enforced on UpdateItem or TransactWriteItems.Update at any level. | diff --git a/tests/test_index_key_validation_on_update.py b/tests/test_index_key_validation_on_update.py new file mode 100644 index 000000000..5271c290b --- /dev/null +++ b/tests/test_index_key_validation_on_update.py @@ -0,0 +1,856 @@ +"""Secondary-index key validation on UpdateItem and TransactWriteItems. + +Every expected code and message here was measured against live DynamoDB. The +rules: + +- An UpdateItem whose statement leaves a secondary-index key empty (empty + string or empty binary) or wrong-typed is refused with ValidationException. + The empty case carries the update-expression wording; the type case carries + the Type mismatch wording naming the index. The type case is reported ahead + of a ConditionExpression; the empty case after the condition passes. +- In TransactWriteItems a wrong-typed index key on a Put item or on the + post-update item of an Update cancels the transaction with a positional + ValidationError reason. An index key left empty aborts the whole transaction + with a top-level ValidationException and no CancellationReasons. On a Put + member both are reported ahead of the member's condition; on an Update + member the type case is reported ahead of the condition and the empty case + after it. +- Across members: a wrong-typed index key on any member wins (the transaction + is cancelled, None in the other positions); otherwise an empty index key on + any member wins over failed conditions and unapplicable statements on the + others. +- if_not_exists(k, :empty) is judged on what the statement writes: accepted + when k is present, refused when k is absent. +- Nothing is written in any refused case. + +Run with EXTENDDB_TEST_ENDPOINT unset to assert the same against the service. +""" + +from __future__ import annotations + +import pytest +from botocore.exceptions import ClientError + +from conftest import scoped_table + +EMPTY_UPDATE_WORDING = ( + "One or more parameter values are not valid. The update expression attempted to " + "update a secondary index key to a value that is not supported. The AttributeValue " + "for a key attribute cannot contain an empty {kind} value." +) +EMPTY_ITEM_WORDING = ( + "One or more parameter values are not valid. A value specified for a secondary index " + "key is not supported. The AttributeValue for a key attribute cannot contain an empty " + "{kind} value. IndexName: {index}, IndexKey: {attr}" +) +TYPE_WORDING = ( + "One or more parameter values were invalid: Type mismatch for Index Key {attr} " + "Expected: {expected} Actual: {actual} IndexName: {index}" +) +CANCELLED = "Transaction cancelled, please refer cancellation reasons for specific reasons" + + +@pytest.fixture(scope="module") +def indexed_table(dynamodb_client): + """A composite-key table with a binary-keyed GSI, a string-keyed GSI and an LSI.""" + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "sk", "AttributeType": "S"}, + {"AttributeName": "bidx", "AttributeType": "B"}, + {"AttributeName": "sidx", "AttributeType": "S"}, + {"AttributeName": "lk", "AttributeType": "S"}, + ], + key_schema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "bidx-index", + "KeySchema": [{"AttributeName": "bidx", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + }, + { + "IndexName": "sidx-index", + "KeySchema": [{"AttributeName": "sidx", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + }, + ], + LocalSecondaryIndexes=[ + { + "IndexName": "lk-index", + "KeySchema": [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "lk", "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + } + ], + ) as name: + yield name + + +def _key(sk: str) -> dict: + return {"pk": {"S": "ik"}, "sk": {"S": sk}} + + +def _err(exc_info) -> dict: + return exc_info.value.response["Error"] + + +def _seed(client, table: str, sk: str) -> dict: + key = _key(sk) + client.put_item(TableName=table, Item={**key, "v": {"N": "1"}}) + return key + + +def _item(client, table: str, key: dict) -> dict | None: + return client.get_item(TableName=table, Key=key, ConsistentRead=True).get("Item") + + +def _update(client, table: str, key: dict, attr: str, value: dict): + return client.update_item( + TableName=table, + Key=key, + UpdateExpression=f"SET {attr} = :v", + ExpressionAttributeValues={":v": value}, + ) + + +def _txn_update(client, table: str, key: dict, attr: str, value: dict, *, lead_put: dict | None = None): + members = [] + if lead_put is not None: + members.append({"Put": {"TableName": table, "Item": lead_put}}) + members.append( + { + "Update": { + "TableName": table, + "Key": key, + "UpdateExpression": f"SET {attr} = :v", + "ExpressionAttributeValues": {":v": value}, + } + } + ) + return client.transact_write_items(TransactItems=members) + + +def _txn_put(client, table: str, item: dict, *, lead_put: dict | None = None): + members = [] + if lead_put is not None: + members.append({"Put": {"TableName": table, "Item": lead_put}}) + members.append({"Put": {"TableName": table, "Item": item}}) + return client.transact_write_items(TransactItems=members) + + +class TestUpdateItemEmptyIndexKey: + """UpdateItem leaving a secondary-index key empty is refused up front.""" + + @pytest.mark.parametrize( + "attr, value, kind", + [ + ("bidx", {"B": b""}, "binary"), + ("sidx", {"S": ""}, "string"), + ("lk", {"S": ""}, "string"), + ], + ids=["gsi-binary", "gsi-string", "lsi-string"], + ) + def test_existing_item(self, dynamodb_client, indexed_table, attr, value, kind): + key = _seed(dynamodb_client, indexed_table, f"e-{attr}") + with pytest.raises(ClientError) as exc_info: + _update(dynamodb_client, indexed_table, key, attr, value) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind=kind) + assert attr not in _item(dynamodb_client, indexed_table, key) + + def test_new_item_is_not_created(self, dynamodb_client, indexed_table): + key = _key("e-new") + with pytest.raises(ClientError) as exc_info: + _update(dynamodb_client, indexed_table, key, "bidx", {"B": b""}) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind="binary") + assert _item(dynamodb_client, indexed_table, key) is None + + +class TestUpdateItemWrongTypedIndexKey: + """UpdateItem setting a secondary-index key to another type is refused, naming the index.""" + + @pytest.mark.parametrize( + "attr, value, expected, actual, index", + [ + ("bidx", {"N": "1"}, "B", "N", "bidx-index"), + ("sidx", {"N": "1"}, "S", "N", "sidx-index"), + ("sidx", {"L": []}, "S", "L", "sidx-index"), + ("lk", {"N": "1"}, "S", "N", "lk-index"), + ], + ids=["gsi-binary-gets-number", "gsi-string-gets-number", "gsi-string-gets-list", "lsi-gets-number"], + ) + def test_existing_item(self, dynamodb_client, indexed_table, attr, value, expected, actual, index): + key = _seed(dynamodb_client, indexed_table, f"t-{attr}-{actual}") + with pytest.raises(ClientError) as exc_info: + _update(dynamodb_client, indexed_table, key, attr, value) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format( + attr=attr, expected=expected, actual=actual, index=index + ) + assert attr not in _item(dynamodb_client, indexed_table, key) + + def test_new_item_is_not_created(self, dynamodb_client, indexed_table): + key = _key("t-new") + with pytest.raises(ClientError) as exc_info: + _update(dynamodb_client, indexed_table, key, "bidx", {"N": "1"}) + assert _err(exc_info)["Code"] == "ValidationException" + assert _item(dynamodb_client, indexed_table, key) is None + + def test_a_valid_value_still_writes(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "t-ok") + _update(dynamodb_client, indexed_table, key, "sidx", {"S": "x"}) + assert _item(dynamodb_client, indexed_table, key)["sidx"] == {"S": "x"} + + +class TestTransactUpdateEmptyIndexKey: + """A transacted Update leaving an index key empty aborts the whole transaction up front.""" + + @pytest.mark.parametrize( + "attr, value, kind", + [ + ("bidx", {"B": b""}, "binary"), + ("sidx", {"S": ""}, "string"), + ("lk", {"S": ""}, "string"), + ], + ids=["gsi-binary", "gsi-string", "lsi-string"], + ) + def test_single_member(self, dynamodb_client, indexed_table, attr, value, kind): + key = _seed(dynamodb_client, indexed_table, f"xe-{attr}") + with pytest.raises(ClientError) as exc_info: + _txn_update(dynamodb_client, indexed_table, key, attr, value) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind=kind) + assert attr not in _item(dynamodb_client, indexed_table, key) + + def test_a_passing_member_does_not_make_it_a_cancellation(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "xe-two") + lead = {"pk": {"S": "lead"}, "sk": {"S": "xe-two"}} + with pytest.raises(ClientError) as exc_info: + _txn_update(dynamodb_client, indexed_table, key, "bidx", {"B": b""}, lead_put=lead) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind="binary") + assert "CancellationReasons" not in exc_info.value.response + assert _item(dynamodb_client, indexed_table, lead) is None + + +class TestTransactUpdateWrongTypedIndexKey: + """A transacted Update setting an index key to another type is a positional cancellation.""" + + @pytest.mark.parametrize( + "attr, value, expected, actual, index", + [ + ("bidx", {"N": "1"}, "B", "N", "bidx-index"), + ("sidx", {"L": []}, "S", "L", "sidx-index"), + ("lk", {"N": "1"}, "S", "N", "lk-index"), + ], + ids=["gsi-binary-gets-number", "gsi-string-gets-list", "lsi-gets-number"], + ) + def test_single_member(self, dynamodb_client, indexed_table, attr, value, expected, actual, index): + key = _seed(dynamodb_client, indexed_table, f"xt-{attr}-{actual}") + with pytest.raises(ClientError) as exc_info: + _txn_update(dynamodb_client, indexed_table, key, attr, value) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert resp["Error"]["Message"] == f"{CANCELLED} [ValidationError]" + reasons = resp["CancellationReasons"] + assert [r["Code"] for r in reasons] == ["ValidationError"] + assert reasons[0]["Message"] == TYPE_WORDING.format( + attr=attr, expected=expected, actual=actual, index=index + ) + assert attr not in _item(dynamodb_client, indexed_table, key) + + def test_reason_is_positional_after_a_passing_member(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "xt-two") + lead = {"pk": {"S": "lead"}, "sk": {"S": "xt-two"}} + with pytest.raises(ClientError) as exc_info: + _txn_update(dynamodb_client, indexed_table, key, "bidx", {"N": "1"}, lead_put=lead) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert resp["Error"]["Message"] == f"{CANCELLED} [None, ValidationError]" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["None", "ValidationError"] + assert _item(dynamodb_client, indexed_table, lead) is None + + +class TestTransactPutIndexKey: + """A transacted Put splits the same way: empty aborts up front, wrong type cancels positionally.""" + + @pytest.mark.parametrize( + "attr, value, kind, index", + [ + ("bidx", {"B": b""}, "binary", "bidx-index"), + ("sidx", {"S": ""}, "string", "sidx-index"), + ("lk", {"S": ""}, "string", "lk-index"), + ], + ids=["gsi-binary", "gsi-string", "lsi-string"], + ) + def test_empty_key_aborts_the_transaction(self, dynamodb_client, indexed_table, attr, value, kind, index): + item = {**_key(f"pe-{attr}"), attr: value} + with pytest.raises(ClientError) as exc_info: + _txn_put(dynamodb_client, indexed_table, item) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_ITEM_WORDING.format(kind=kind, index=index, attr=attr) + assert _item(dynamodb_client, indexed_table, _key(f"pe-{attr}")) is None + + @pytest.mark.parametrize( + "attr, value, expected, actual, index", + [ + ("bidx", {"N": "1"}, "B", "N", "bidx-index"), + ("lk", {"N": "1"}, "S", "N", "lk-index"), + ], + ids=["gsi-binary-gets-number", "lsi-gets-number"], + ) + def test_wrong_type_cancels(self, dynamodb_client, indexed_table, attr, value, expected, actual, index): + item = {**_key(f"pt-{attr}"), attr: value} + with pytest.raises(ClientError) as exc_info: + _txn_put(dynamodb_client, indexed_table, item) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + reasons = resp["CancellationReasons"] + assert [r["Code"] for r in reasons] == ["ValidationError"] + assert reasons[0]["Message"] == TYPE_WORDING.format( + attr=attr, expected=expected, actual=actual, index=index + ) + assert _item(dynamodb_client, indexed_table, _key(f"pt-{attr}")) is None + + def test_wrong_type_reason_is_positional(self, dynamodb_client, indexed_table): + lead = {"pk": {"S": "lead"}, "sk": {"S": "pt-two"}} + item = {**_key("pt-two"), "bidx": {"N": "1"}} + with pytest.raises(ClientError) as exc_info: + _txn_put(dynamodb_client, indexed_table, item, lead_put=lead) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert resp["Error"]["Message"] == f"{CANCELLED} [None, ValidationError]" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["None", "ValidationError"] + assert _item(dynamodb_client, indexed_table, lead) is None + + +class TestOrderAgainstTheCondition: + """Where the index-key checks sit relative to a ConditionExpression, as measured. + + A wrong-typed index key is reported ahead of the condition. A failed + condition is reported ahead of an empty index key and ahead of a statement + that cannot be applied (arithmetic on a missing attribute). + """ + + def test_update_item_type_mismatch_beats_a_failing_condition(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "o-type") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, + Key=key, + UpdateExpression="SET bidx = :v", + ConditionExpression="attribute_not_exists(pk)", + ExpressionAttributeValues={":v": {"N": "1"}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format( + attr="bidx", expected="B", actual="N", index="bidx-index" + ) + + def test_update_item_failing_condition_beats_an_empty_key(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "o-empty") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, + Key=key, + UpdateExpression="SET sidx = :v", + ConditionExpression="attribute_not_exists(pk)", + ExpressionAttributeValues={":v": {"S": ""}}, + ) + assert _err(exc_info)["Code"] == "ConditionalCheckFailedException" + + def test_update_item_failing_condition_beats_a_statement_that_cannot_apply(self, dynamodb_client, indexed_table): + key = _key("o-apply-missing") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, + Key=key, + UpdateExpression="SET v = v + :one", + ConditionExpression="attribute_exists(pk)", + ExpressionAttributeValues={":one": {"N": "1"}}, + ) + assert _err(exc_info)["Code"] == "ConditionalCheckFailedException" + + def test_transact_update_type_mismatch_beats_a_failing_condition(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "xo-type") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Update": { + "TableName": indexed_table, + "Key": key, + "UpdateExpression": "SET bidx = :v", + "ConditionExpression": "attribute_not_exists(pk)", + "ExpressionAttributeValues": {":v": {"N": "1"}}, + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError"] + assert resp["CancellationReasons"][0]["Message"] == TYPE_WORDING.format( + attr="bidx", expected="B", actual="N", index="bidx-index" + ) + + def test_transact_update_failing_condition_beats_an_empty_key(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "xo-empty") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Update": { + "TableName": indexed_table, + "Key": key, + "UpdateExpression": "SET sidx = :v", + "ConditionExpression": "attribute_not_exists(pk)", + "ExpressionAttributeValues": {":v": {"S": ""}}, + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ConditionalCheckFailed"] + + def test_transact_update_failing_condition_beats_a_statement_that_cannot_apply(self, dynamodb_client, indexed_table): + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Update": { + "TableName": indexed_table, + "Key": _key("xo-apply-missing"), + "UpdateExpression": "SET v = v + :one", + "ConditionExpression": "attribute_exists(pk)", + "ExpressionAttributeValues": {":one": {"N": "1"}}, + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ConditionalCheckFailed"] + + def test_transact_update_type_mismatch_wins_over_an_empty_key_in_the_same_statement(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "xo-both") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Update": { + "TableName": indexed_table, + "Key": key, + "UpdateExpression": "SET sidx = :e, bidx = :bad", + "ExpressionAttributeValues": {":e": {"S": ""}, ":bad": {"N": "1"}}, + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError"] + assert resp["CancellationReasons"][0]["Message"] == TYPE_WORDING.format( + attr="bidx", expected="B", actual="N", index="bidx-index" + ) + + +def _upd(table: str, pk: str, expr: str, vals: dict, cond: str | None = None) -> dict: + member = { + "TableName": table, + "Key": _key(pk), + "UpdateExpression": expr, + "ExpressionAttributeValues": vals, + } + if cond: + member["ConditionExpression"] = cond + return {"Update": member} + + +class TestTransactPutOrderAgainstTheCondition: + """On a Put member both index-key checks come ahead of the condition, as measured.""" + + def test_wrong_type_beats_a_failing_condition(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "po-type") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Put": { + "TableName": indexed_table, + "Item": {**_key("po-type"), "bidx": {"N": "1"}}, + "ConditionExpression": "attribute_not_exists(pk)", + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError"] + + def test_empty_key_beats_a_failing_condition(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "po-empty") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Put": { + "TableName": indexed_table, + "Item": {**_key("po-empty"), "sidx": {"S": ""}}, + "ConditionExpression": "attribute_not_exists(pk)", + } + } + ] + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_ITEM_WORDING.format( + kind="string", index="sidx-index", attr="sidx" + ) + assert "CancellationReasons" not in exc_info.value.response + + def test_a_plain_put_still_reports_its_failed_condition(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "po-plain") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + { + "Put": { + "TableName": indexed_table, + "Item": {**_key("po-plain"), "v": {"S": "2"}}, + "ConditionExpression": "attribute_not_exists(pk)", + } + } + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ConditionalCheckFailed"] + + +class TestCrossMemberPrecedence: + """Which fault a transaction reports when different members carry different faults.""" + + @pytest.mark.parametrize("type_first", [True, False], ids=["type-then-empty", "empty-then-type"]) + def test_a_wrong_type_on_one_member_wins_over_an_empty_key_on_another( + self, dynamodb_client, indexed_table, type_first + ): + a = _seed(dynamodb_client, indexed_table, "cm-a") + b = _seed(dynamodb_client, indexed_table, "cm-b") + typed = _upd(indexed_table, "cm-a", "SET bidx = :v", {":v": {"N": "1"}}) + empty = _upd(indexed_table, "cm-b", "SET sidx = :v", {":v": {"S": ""}}) + members = [typed, empty] if type_first else [empty, typed] + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=members) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + codes = [r["Code"] for r in resp["CancellationReasons"]] + assert codes == (["ValidationError", "None"] if type_first else ["None", "ValidationError"]) + typed_reason = resp["CancellationReasons"][0 if type_first else 1] + assert typed_reason["Message"] == TYPE_WORDING.format( + attr="bidx", expected="B", actual="N", index="bidx-index" + ) + assert "bidx" not in _item(dynamodb_client, indexed_table, a) + assert "sidx" not in _item(dynamodb_client, indexed_table, b) + + @pytest.mark.parametrize("empty_first", [True, False], ids=["empty-then-cond", "cond-then-empty"]) + def test_an_empty_key_on_one_member_wins_over_a_failed_condition_on_another( + self, dynamodb_client, indexed_table, empty_first + ): + _seed(dynamodb_client, indexed_table, "ce-a") + _seed(dynamodb_client, indexed_table, "ce-b") + empty = _upd(indexed_table, "ce-a", "SET sidx = :v", {":v": {"S": ""}}) + failing = _upd(indexed_table, "ce-b", "SET v = :v", {":v": {"N": "2"}}, "attribute_not_exists(pk)") + members = [empty, failing] if empty_first else [failing, empty] + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=members) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind="string") + assert "CancellationReasons" not in exc_info.value.response + + def test_an_empty_key_wins_over_a_statement_that_cannot_apply_elsewhere(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "ca-b") + cannot_apply = _upd(indexed_table, "ca-missing", "SET v = v + :one", {":one": {"N": "1"}}) + empty = _upd(indexed_table, "ca-b", "SET sidx = :v", {":v": {"S": ""}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=[cannot_apply, empty]) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind="string") + assert _item(dynamodb_client, indexed_table, _key("ca-missing")) is None + + def test_a_wrong_type_masks_a_failed_condition_on_another_member(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "ct-a") + _seed(dynamodb_client, indexed_table, "ct-b") + failing = _upd(indexed_table, "ct-a", "SET v = :v", {":v": {"N": "2"}}, "attribute_not_exists(pk)") + typed = _upd(indexed_table, "ct-b", "SET bidx = :v", {":v": {"N": "1"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=[failing, typed]) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["None", "ValidationError"] + + def test_a_wrong_type_masks_a_statement_that_cannot_apply_on_another_member(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "cx-b") + cannot_apply = _upd(indexed_table, "cx-missing", "SET v = v + :one", {":one": {"N": "1"}}) + typed = _upd(indexed_table, "cx-b", "SET bidx = :v", {":v": {"N": "1"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=[cannot_apply, typed]) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["None", "ValidationError"] + assert _item(dynamodb_client, indexed_table, _key("cx-missing")) is None + + @pytest.mark.parametrize("bad_key_first", [True, False], ids=["table-key-then-cond", "cond-then-table-key"]) + def test_a_wrong_typed_table_key_on_a_put_masks_a_failed_condition_elsewhere( + self, dynamodb_client, indexed_table, bad_key_first + ): + _seed(dynamodb_client, indexed_table, "tk-a") + bad_put = {"Put": {"TableName": indexed_table, "Item": {"pk": {"N": "1"}, "sk": {"S": "s"}}}} + failing = _upd(indexed_table, "tk-a", "SET v = :v", {":v": {"N": "2"}}, "attribute_not_exists(pk)") + members = [bad_put, failing] if bad_key_first else [failing, bad_put] + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=members) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + codes = [r["Code"] for r in resp["CancellationReasons"]] + assert codes == (["ValidationError", "None"] if bad_key_first else ["None", "ValidationError"]) + + def test_a_wrong_typed_table_key_on_a_put_masks_an_empty_index_key_elsewhere(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "tk-b") + bad_put = {"Put": {"TableName": indexed_table, "Item": {"pk": {"N": "1"}, "sk": {"S": "s"}}}} + empty = _upd(indexed_table, "tk-b", "SET sidx = :v", {":v": {"S": ""}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=[empty, bad_put]) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["None", "ValidationError"] + assert "sidx" not in _item(dynamodb_client, indexed_table, _key("tk-b")) + + def test_two_wrong_types_report_only_the_first(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "cd-a") + _seed(dynamodb_client, indexed_table, "cd-b") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[ + _upd(indexed_table, "cd-a", "SET bidx = :v", {":v": {"N": "1"}}), + _upd(indexed_table, "cd-b", "SET sidx = :v", {":v": {"N": "1"}}), + ] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError", "None"] + assert resp["CancellationReasons"][0]["Message"] == TYPE_WORDING.format( + attr="bidx", expected="B", actual="N", index="bidx-index" + ) + + def test_an_empty_key_whose_own_condition_fails_is_a_plain_cancellation(self, dynamodb_client, indexed_table): + _seed(dynamodb_client, indexed_table, "cc-a") + _seed(dynamodb_client, indexed_table, "cc-b") + empty_failing = _upd(indexed_table, "cc-a", "SET sidx = :v", {":v": {"S": ""}}, "attribute_not_exists(pk)") + plain = _upd(indexed_table, "cc-b", "SET v = :v", {":v": {"N": "2"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items(TransactItems=[empty_failing, plain]) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ConditionalCheckFailed", "None"] + assert _item(dynamodb_client, indexed_table, _key("cc-b"))["v"] == {"N": "1"} + + +class TestIfNotExistsIsJudgedOnWhatIsWritten: + """if_not_exists(k, :empty) writes nothing bad when k is present and is refused when k is absent.""" + + def test_present_key_is_kept_and_the_statement_is_accepted(self, dynamodb_client, indexed_table): + key = _key("ine-present") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}}) + dynamodb_client.update_item( + TableName=indexed_table, + Key=key, + UpdateExpression="SET sidx = if_not_exists(sidx, :e)", + ExpressionAttributeValues={":e": {"S": ""}}, + ) + assert _item(dynamodb_client, indexed_table, key)["sidx"] == {"S": "x"} + + def test_absent_key_makes_the_statement_a_refused_empty_key(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "ine-absent") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, + Key=key, + UpdateExpression="SET sidx = if_not_exists(sidx, :e)", + ExpressionAttributeValues={":e": {"S": ""}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == EMPTY_UPDATE_WORDING.format(kind="string") + assert "sidx" not in _item(dynamodb_client, indexed_table, key) + + def test_transacted_present_key_is_accepted(self, dynamodb_client, indexed_table): + key = _key("ine-txn-present") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}}) + dynamodb_client.transact_write_items( + TransactItems=[_upd(indexed_table, "ine-txn-present", "SET sidx = if_not_exists(sidx, :e)", {":e": {"S": ""}})] + ) + assert _item(dynamodb_client, indexed_table, key)["sidx"] == {"S": "x"} + + +COMPUTED_WORDING = "The update expression attempted to update the secondary index key to unsupported type" + + +class TestLiteralVersusComputedIndexKeyValues: + """Two type checks, as measured. A literal operand on an index key is checked on the + statement before the condition and before any action is applied, with the per-index + wording. A value computed from the item (an attribute reference, the fallback of + if_not_exists) is checked after the condition passes and the statement is applied, + with a wording that names neither attribute nor index.""" + + def test_attribute_reference_of_the_wrong_type_uses_the_computed_wording(self, dynamodb_client, indexed_table): + key = _key("lc-ref") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}, "n": {"N": "5"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item(TableName=indexed_table, Key=key, UpdateExpression="SET sidx = n") + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == COMPUTED_WORDING + assert _item(dynamodb_client, indexed_table, key)["sidx"] == {"S": "x"} + + def test_attribute_reference_is_checked_after_the_condition(self, dynamodb_client, indexed_table): + key = _key("lc-ref-cond") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}, "n": {"N": "5"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, UpdateExpression="SET sidx = n", + ConditionExpression="attribute_not_exists(pk)", + ) + assert _err(exc_info)["Code"] == "ConditionalCheckFailedException" + + def test_if_not_exists_fallback_of_the_wrong_type_uses_the_computed_wording(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "lc-ine") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, + UpdateExpression="SET sidx = if_not_exists(nope, :num)", + ExpressionAttributeValues={":num": {"N": "9"}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == COMPUTED_WORDING + + def test_if_not_exists_fallback_is_checked_after_the_condition(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "lc-ine-cond") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, + UpdateExpression="SET sidx = if_not_exists(nope, :num)", + ConditionExpression="attribute_not_exists(pk)", + ExpressionAttributeValues={":num": {"N": "9"}}, + ) + assert _err(exc_info)["Code"] == "ConditionalCheckFailedException" + + def test_list_append_of_literals_is_a_literal(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "lc-la") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, + UpdateExpression="SET sidx = list_append(:l, :l)", + ConditionExpression="attribute_not_exists(pk)", + ExpressionAttributeValues={":l": {"L": []}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format(attr="sidx", expected="S", actual="L", index="sidx-index") + + @pytest.mark.parametrize( + "expr, vals, actual", + [ + ("ADD sidx :one", {":one": {"N": "1"}}, "N"), + ("ADD sidx :ss", {":ss": {"SS": ["a"]}}, "SS"), + ("DELETE sidx :ss", {":ss": {"SS": ["a"]}}, "SS"), + ], + ids=["add-number-to-string-key", "add-set-to-string-key", "delete-set-from-string-key"], + ) + def test_add_and_delete_on_an_index_key_are_literal_type_mismatches(self, dynamodb_client, indexed_table, expr, vals, actual): + key = _seed(dynamodb_client, indexed_table, f"lc-{expr.split()[0].lower()}-{actual}") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, UpdateExpression=expr, + ConditionExpression="attribute_not_exists(pk)", ExpressionAttributeValues=vals, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format(attr="sidx", expected="S", actual=actual, index="sidx-index") + + def test_delete_on_an_absent_item_is_still_a_type_mismatch(self, dynamodb_client, indexed_table): + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=_key("lc-del-absent"), UpdateExpression="DELETE sidx :ss", + ExpressionAttributeValues={":ss": {"SS": ["a"]}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format(attr="sidx", expected="S", actual="SS", index="sidx-index") + assert _item(dynamodb_client, indexed_table, _key("lc-del-absent")) is None + + def test_a_literal_fault_beats_an_action_that_cannot_apply(self, dynamodb_client, indexed_table): + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=_key("lc-apply"), + UpdateExpression="SET v = v + :one, bidx = :num", + ConditionExpression="attribute_exists(pk)", + ExpressionAttributeValues={":one": {"N": "1"}, ":num": {"N": "9"}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format(attr="bidx", expected="B", actual="N", index="bidx-index") + + def test_a_literal_fault_beats_an_invalid_document_path(self, dynamodb_client, indexed_table): + key = _seed(dynamodb_client, indexed_table, "lc-path") + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=key, + UpdateExpression="SET plain.x = :num, bidx = :num", + ExpressionAttributeValues={":num": {"N": "9"}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == TYPE_WORDING.format(attr="bidx", expected="B", actual="N", index="bidx-index") + + def test_an_empty_literal_does_not_beat_an_action_that_cannot_apply(self, dynamodb_client, indexed_table): + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=indexed_table, Key=_key("lc-empty-apply"), + UpdateExpression="SET v = v + :one, sidx = :e", + ExpressionAttributeValues={":one": {"N": "1"}, ":e": {"S": ""}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == "The provided expression refers to an attribute that does not exist in the item" + + def test_transacted_computed_value_is_a_cancellation_with_the_computed_wording(self, dynamodb_client, indexed_table): + key = _key("lc-txn-ref") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}, "n": {"N": "5"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[{"Update": {"TableName": indexed_table, "Key": key, "UpdateExpression": "SET sidx = n"}}] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError"] + assert resp["CancellationReasons"][0]["Message"] == COMPUTED_WORDING + + def test_transacted_computed_value_is_checked_after_the_condition(self, dynamodb_client, indexed_table): + key = _key("lc-txn-ref-cond") + dynamodb_client.put_item(TableName=indexed_table, Item={**key, "sidx": {"S": "x"}, "n": {"N": "5"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[{"Update": {"TableName": indexed_table, "Key": key, "UpdateExpression": "SET sidx = n", "ConditionExpression": "attribute_not_exists(pk)"}}] + ) + resp = exc_info.value.response + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ConditionalCheckFailed"] + + def test_transacted_literal_fault_beats_an_action_that_cannot_apply(self, dynamodb_client, indexed_table): + with pytest.raises(ClientError) as exc_info: + dynamodb_client.transact_write_items( + TransactItems=[_upd(indexed_table, "lc-txn-apply", "SET v = v + :one, bidx = :num", {":one": {"N": "1"}, ":num": {"N": "9"}}, "attribute_exists(pk)")] + ) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + assert [r["Code"] for r in resp["CancellationReasons"]] == ["ValidationError"] + assert resp["CancellationReasons"][0]["Message"] == TYPE_WORDING.format(attr="bidx", expected="B", actual="N", index="bidx-index") diff --git a/tests/test_item_size_at_limit.py b/tests/test_item_size_at_limit.py new file mode 100644 index 000000000..ec71c2838 --- /dev/null +++ b/tests/test_item_size_at_limit.py @@ -0,0 +1,431 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Item size at the 409,600-byte limit: number byte costs, the UpdateItem rule, +the transacted Update rule, and the empty-binary index key refusal on UpdateItem. + +Every figure here was measured against Amazon DynamoDB (us-east-1, 2026-10-19) +by an accept/refuse pair either side of the limit: an item of exactly N bytes is +accepted and one of N+1 is refused, so the pair measures a single attribute's +cost to the byte with two requests. + +Numbers: ``1 + ceil(integer significant digits / 2) + ceil(fraction significant +digits / 2)``, plus one for a sign. The two halves round up separately, so +``1.5`` costs 3 where ``15`` costs 2 and ``3.14159`` costs 5 where ``123456`` +costs 4. + +UpdateItem is sized by the statement, not the finished item: a base of 3; 18 +per SET or ADD clause and nothing per REMOVE or DELETE; a skeleton item holding +only the written paths (each map entry its name plus one, each map or list 3, +each list element one more, untouched siblings left out), where a SET or ADD +leaf costs the clause's value placeholders and a REMOVE or DELETE leaf costs +nothing; one more for a clause through a list index; and inside a SET operand +26 plus the name length per function call, 34 per "+", 37 per "-", and for a +document path operand 15 plus the name for its first element, 14 plus the name +for a later attribute, 15 for an index, with the attribute's stored value not +charged. The legacy AttributeUpdates form costs 3 plus the name and finished +value of each attribute it names that is present afterwards. The key and +untouched attributes are left out, so at a one-byte key the largest finished +item a plain SET can produce is 409,581 bytes, and a longer key or an +untouched attribute buys the headroom back, capped at 409,600, which the +finished item never exceeds. The service applies this rule in 31 of 33 +commercial regions, us-east-1 among them. A transacted Update is sized on the +finished item alone: exactly 409,600 is stored, one byte over is a +cancellation. Both update paths word the refusal "Item size to update has +exceeded the maximum allowed size". + +Dual-target against Amazon DynamoDB and extenddb. +""" + +from __future__ import annotations + +import os + +import pytest +from botocore.exceptions import ClientError + +from conftest import scoped_table + +# The statement-sized UpdateItem rule and the empty-binary index-key refusal are +# enforced by the PostgreSQL and SQLite storage layers. The Cassandra layer +# sizes every update on the finished item against a fixed 409,600 bytes and the +# MongoDB layer applies no item-size limit on updates, both documented in +# docs/differences-from-dynamodb.md, so the update-specific classes below are +# skipped on those two backends. Number sizing lives in shared core and is +# asserted on every backend. When the backend is unset (a run against live +# DynamoDB through conf-run/live), every class runs. +on_default_backends_only = pytest.mark.skipif( + os.environ.get("EXTENDDB_TEST_BACKEND", "") in {"mongodb", "cassandra"}, + reason="UpdateItem is sized on the finished item (Cassandra) or unbounded (MongoDB); " + "see docs/differences-from-dynamodb.md", +) + +MAX = 409_600 +# A one-byte partition key "k": pk(2) + value(1) bytes the statement never writes. +KEY_K_BYTES = 3 +PUT_WORDING = "Item size has exceeded the maximum allowed size" +UPDATE_WORDING = "Item size to update has exceeded the maximum allowed size" +UPDATE_BASE = 3 +SET_COST = 19 # 18 for the clause plus the written attribute's map entry byte +REMOVE_COST = 2 # nothing for the clause plus the attribute's name and entry byte + + +@pytest.fixture(scope="class") +def table(dynamodb_client): + with scoped_table(dynamodb_client) as name: + yield name + + +@pytest.fixture(scope="class") +def gsi_table(dynamodb_client): + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "bidx", "AttributeType": "B"}, + ], + GlobalSecondaryIndexes=[ + { + "IndexName": "by-bidx", + "KeySchema": [{"AttributeName": "bidx", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "KEYS_ONLY"}, + } + ], + ) as name: + yield name + + +def _err(exc_info) -> dict: + return exc_info.value.response["Error"] + + +class TestNumberByteCost: + """A number's byte cost, measured by the accept/refuse pair at the limit.""" + + # (literal, bytes) measured on the service. + CASES = [ + ("0", 1), ("1", 2), ("15", 2), ("100", 2), ("1E125", 2), ("0.15", 2), + ("1.5", 3), ("1.2", 3), ("1.200", 3), ("1010", 3), ("-42", 3), + ("1.234", 4), ("100.5", 4), ("123456", 4), ("3.14159", 5), + ] + + @pytest.mark.parametrize(("literal", "cost"), CASES) + def test_number_costs_measured_bytes(self, dynamodb_client, table, literal, cost): + pk = "n-" + literal.replace(".", "_").replace("-", "m") + # pk(2) + pk value + n(1) + cost + p(1) + pad == MAX + pad = MAX - (2 + len(pk) + 1 + cost + 1) + dynamodb_client.put_item( + TableName=table, + Item={"pk": {"S": pk}, "n": {"N": literal}, "p": {"S": "x" * pad}}, + ) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.put_item( + TableName=table, + Item={"pk": {"S": pk}, "n": {"N": literal}, "p": {"S": "x" * (pad + 1)}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == PUT_WORDING + + +@on_default_backends_only +class TestUpdateItemSizeRule: + """UpdateItem is sized by the statement, not the finished item.""" + + def _set(self, client, table, pk, finished_bytes, extra_expr="", extra_values=None): + pad = finished_bytes - (2 + len(pk) + 1) # pk name, pk value, "p" name + values = {":v": {"S": "x" * pad}} + values.update(extra_values or {}) + client.update_item( + TableName=table, + Key={"pk": {"S": pk}}, + UpdateExpression="SET p = :v" + extra_expr, + ExpressionAttributeValues=values, + ) + + def test_one_set_at_a_one_byte_key_caps_below_the_limit(self, dynamodb_client, table): + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - SET_COST # 409,581 + self._set(dynamodb_client, table, "k", ceiling) + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": "k"}}) + with pytest.raises(ClientError) as exc_info: + self._set(dynamodb_client, table, "k", ceiling + 1) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_a_second_set_costs_nineteen_bytes(self, dynamodb_client, table): + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - 2 * SET_COST # 409,562 + extra = {":w": {"S": "y"}} + # finished = pk(2)+1 + p(1)+pad + q(1)+1; the SET q costs 19, writes 2. + pad = ceiling - (2 + 1 + 1 + 2) + dynamodb_client.update_item( + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET p = :v, q = :w", + ExpressionAttributeValues={":v": {"S": "x" * pad}, **extra}, + ) + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": "k"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET p = :v, q = :w", + ExpressionAttributeValues={":v": {"S": "x" * (pad + 1)}, **extra}, + ) + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_a_remove_costs_two_bytes(self, dynamodb_client, table): + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - SET_COST - REMOVE_COST # 409,579 + pad = ceiling - (2 + 1 + 1) + for size, ok in ((pad, True), (pad + 1, False)): + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "k"}, "r": {"S": "z"}}) + call = lambda: dynamodb_client.update_item( # noqa: E731 + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET p = :v REMOVE r", + ExpressionAttributeValues={":v": {"S": "x" * size}}, + ) + if ok: + call() + else: + with pytest.raises(ClientError) as exc_info: + call() + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_an_untouched_attribute_buys_headroom_back_to_the_limit(self, dynamodb_client, table): + # 100-byte attribute u is left alone; finished item may reach 409,600 exactly. + for finished, ok in ((MAX, True), (MAX + 1, False)): + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "k"}, "u": {"S": "u" * 100}}) + pad = finished - (2 + 1 + 1 + 101) + call = lambda: dynamodb_client.update_item( # noqa: E731 + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET p = :v", + ExpressionAttributeValues={":v": {"S": "x" * pad}}, + ) + if ok: + call() + else: + with pytest.raises(ClientError) as exc_info: + call() + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_a_long_key_buys_headroom_back_to_the_limit(self, dynamodb_client, table): + pk = "K" * 100 + self._set(dynamodb_client, table, pk, MAX) + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": pk}}) + with pytest.raises(ClientError) as exc_info: + self._set(dynamodb_client, table, pk, MAX + 1) + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_an_alias_is_charged_as_the_attribute_it_names(self, dynamodb_client, table): + # SET #twelvechars = :v writes attribute "p"; the alias costs nothing extra. + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - SET_COST # 409,581 + for finished, ok in ((ceiling, True), (ceiling + 1, False)): + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": "k"}}) + pad = finished - (2 + 1 + 1) + call = lambda: dynamodb_client.update_item( # noqa: E731 + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET #twelvechars = :v", + ExpressionAttributeNames={"#twelvechars": "p"}, + ExpressionAttributeValues={":v": {"S": "x" * pad}}, + ) + if ok: + call() + else: + with pytest.raises(ClientError) as exc_info: + call() + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_a_set_through_a_list_index_costs_one_byte_more(self, dynamodb_client, table): + # Measured: SET d[0] = :v is charged 20 where SET d = :whole is charged 19. + # finished = pk(2)+1 + d(1) + 3 + (pad + 1) + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - SET_COST - 1 # 409,580 + for finished, ok in ((ceiling, True), (ceiling + 1, False)): + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "k"}, "d": {"L": [{"S": ""}]}}) + pad = finished - (2 + 1 + 1 + 3 + 1) + call = lambda: dynamodb_client.update_item( # noqa: E731 + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET d[0] = :v", + ExpressionAttributeValues={":v": {"S": "x" * pad}}, + ) + if ok: + call() + else: + with pytest.raises(ClientError) as exc_info: + call() + assert _err(exc_info)["Message"] == UPDATE_WORDING + + def test_a_nested_path_set_is_charged_like_a_plain_set(self, dynamodb_client, table): + ceiling = MAX + KEY_K_BYTES - UPDATE_BASE - SET_COST # 409,581 + for finished, ok in ((ceiling, True), (ceiling + 1, False)): + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "k"}, "m": {"M": {"a": {"S": ""}}}}) + # finished = pk(2)+1 + m(1) + 3 + (a(1) + pad + 1) + pad = finished - (2 + 1 + 1 + 3 + 1 + 1) + call = lambda: dynamodb_client.update_item( # noqa: E731 + TableName=table, Key={"pk": {"S": "k"}}, + UpdateExpression="SET m.a = :v", + ExpressionAttributeValues={":v": {"S": "x" * pad}}, + ) + if ok: + call() + else: + with pytest.raises(ClientError) as exc_info: + call() + assert _err(exc_info)["Message"] == UPDATE_WORDING + + +def _s(n: int) -> dict: + return {"S": "x" * n} + + +@on_default_backends_only +class TestUpdateStatementShapes: + """The largest accepted padding at a one-byte key, per statement shape. + + Each row is (name, seed item or None, builder) where the builder returns the + UpdateItem arguments for a given padding, and the figure is the largest + padding the service accepted on 2026-10-19; the padding one larger is refused + with the update wording. The seed is written with PutItem before each + attempt. Shapes whose figure is the finished-item cap rather than the + statement figure are marked so in the name. + """ + + SHAPES = [ + ("plain_set", None, 409_577, + lambda n: {"UpdateExpression": "SET p = :v", "ExpressionAttributeValues": {":v": _s(n)}}), + ("two_sets_different_attributes", None, 409_556, + lambda n: {"UpdateExpression": "SET p = :v, q = :w", "ExpressionAttributeValues": {":v": _s(n), ":w": {"S": "y"}}}), + ("two_clauses_same_map_share_the_container", {"m": {"M": {}}}, 409_551, + lambda n: {"UpdateExpression": "SET m.a = :v, m.b = :w", "ExpressionAttributeValues": {":v": _s(n), ":w": {"S": "y"}}}), + ("three_clauses_same_map", {"m": {"M": {}}}, 409_530, + lambda n: {"UpdateExpression": "SET m.a = :v, m.b = :w, m.c = :x", "ExpressionAttributeValues": {":v": _s(n), ":w": {"S": "y"}, ":x": {"S": "y"}}}), + ("nested_two_deep", {"d": {"M": {"a": {"M": {}}}}}, 409_567, + lambda n: {"UpdateExpression": "SET d.a.b = :v", "ExpressionAttributeValues": {":v": _s(n)}}), + ("map_operand", None, 409_572, + lambda n: {"UpdateExpression": "SET p = :v", "ExpressionAttributeValues": {":v": {"M": {"a": _s(n)}}}}), + ("list_index_siblings_not_charged_cap_binds", {"p": {"L": [{"S": "s" * 50}, {"S": "seed"}]}}, 409_541, + lambda n: {"UpdateExpression": "SET p[1] = :v", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_charges_the_operand", None, 409_522, + lambda n: {"UpdateExpression": "SET p = if_not_exists(p, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_charges_the_operand_when_present", {"p": {"S": "q"}}, 409_522, + lambda n: {"UpdateExpression": "SET p = if_not_exists(p, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_ten_byte_names", None, 409_504, + lambda n: {"UpdateExpression": "SET pppppppppp = if_not_exists(pppppppppp, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_other_inner_path", None, 409_513, + lambda n: {"UpdateExpression": "SET p = if_not_exists(qqqqqqqqqq, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_nested_inner_path", {"d": {"M": {}}}, 409_504, + lambda n: {"UpdateExpression": "SET p = if_not_exists(d.leaf, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_nested_target", {"d": {"M": {}}}, 409_496, + lambda n: {"UpdateExpression": "SET d.leaf = if_not_exists(d.leaf, :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("if_not_exists_indexed_inner_path", {"d": {"L": [{"S": "q"}]}}, 409_507, + lambda n: {"UpdateExpression": "SET p = if_not_exists(d[0], :v)", "ExpressionAttributeValues": {":v": _s(n)}}), + ("list_append_two_values", None, 409_531, + lambda n: {"UpdateExpression": "SET p = list_append(:v, :w)", "ExpressionAttributeValues": {":v": {"L": [_s(n)]}, ":w": {"L": [{"S": "y"}]}}}), + ("list_append_path_value_not_charged", {"p": {"L": [{"S": "q"}]}}, 409_520, + lambda n: {"UpdateExpression": "SET p = list_append(p, :v)", "ExpressionAttributeValues": {":v": {"L": [_s(n)]}}}), + ("list_append_to_big_list_cap_binds", "big_list", 409_590, + lambda n: {"UpdateExpression": "SET p = list_append(p, :v)", "ExpressionAttributeValues": {":v": {"L": [{"S": "y"}]}}}), + ("nested_functions", None, 409_478, + lambda n: {"UpdateExpression": "SET p = list_append(if_not_exists(p, :e), :v)", "ExpressionAttributeValues": {":e": {"L": []}, ":v": {"L": [_s(n)]}}}), + ("addition_with_path_operand", {"n": {"N": "1"}}, 409_505, + lambda n: {"UpdateExpression": "SET p = :v, n = n + :w", "ExpressionAttributeValues": {":v": _s(n), ":w": {"N": "1"}}}), + ("addition_with_two_values", None, 409_519, + lambda n: {"UpdateExpression": "SET p = :v, n = :w + :x", "ExpressionAttributeValues": {":v": _s(n), ":w": {"N": "1"}, ":x": {"N": "1"}}}), + ("subtraction_with_two_values", None, 409_516, + lambda n: {"UpdateExpression": "SET p = :v, n = :w - :x", "ExpressionAttributeValues": {":v": _s(n), ":w": {"N": "1"}, ":x": {"N": "1"}}}), + ("add_number", None, 409_555, + lambda n: {"UpdateExpression": "SET p = :v ADD n :w", "ExpressionAttributeValues": {":v": _s(n), ":w": {"N": "1"}}}), + ("add_to_big_set_cap_binds", "big_set", 409_595, + lambda n: {"UpdateExpression": "ADD s :v", "ExpressionAttributeValues": {":v": {"SS": ["y"]}}}), + ("remove_nested_path", {"d": {"M": {"leaf": {"S": "y"}}}}, 409_567, + lambda n: {"UpdateExpression": "SET p = :v REMOVE d.leaf", "ExpressionAttributeValues": {":v": _s(n)}}), + ("remove_through_list_index", {"d": {"L": [{"S": "y"}]}}, 409_570, + lambda n: {"UpdateExpression": "SET p = :v REMOVE d[0]", "ExpressionAttributeValues": {":v": _s(n)}}), + ("delete_operand_not_charged", {"s": {"SS": ["a", "b"]}}, 409_575, + lambda n: {"UpdateExpression": "SET p = :v DELETE s :w", "ExpressionAttributeValues": {":v": _s(n), ":w": {"SS": ["a"]}}}), + ("legacy_put", None, 409_596, + lambda n: {"AttributeUpdates": {"p": {"Value": _s(n), "Action": "PUT"}}}), + ("legacy_two_puts", None, 409_594, + lambda n: {"AttributeUpdates": {"p": {"Value": _s(n), "Action": "PUT"}, "q": {"Value": {"S": "y"}, "Action": "PUT"}}}), + ("legacy_put_ten_byte_name", None, 409_587, + lambda n: {"AttributeUpdates": {"pppppppppp": {"Value": _s(n), "Action": "PUT"}}}), + ("legacy_put_map", None, 409_591, + lambda n: {"AttributeUpdates": {"p": {"Value": {"M": {"a": _s(n)}}, "Action": "PUT"}}}), + ("legacy_put_and_delete_charges_the_finished_set", {"s": {"SS": ["a", "b"]}}, 409_594, + lambda n: {"AttributeUpdates": {"p": {"Value": _s(n), "Action": "PUT"}, "s": {"Value": {"SS": ["a"]}, "Action": "DELETE"}}}), + ("legacy_put_and_remove_charges_nothing_for_the_removed", {"s": {"SS": ["a", "b"]}}, 409_596, + lambda n: {"AttributeUpdates": {"p": {"Value": _s(n), "Action": "PUT"}, "s": {"Action": "DELETE"}}}), + ] + + @staticmethod + def _seed(seed, n): + if seed == "big_list": + return {"p": {"L": [_s(n)]}} + if seed == "big_set": + return {"s": {"SS": ["x" * n]}} + return seed + + @pytest.mark.parametrize("name,seed,ceiling,build", SHAPES, ids=[r[0] for r in SHAPES]) + def test_largest_accepted_padding(self, dynamodb_client, table, name, seed, ceiling, build): + key = {"pk": {"S": "k"}} + for n, ok in ((ceiling, True), (ceiling + 1, False)): + dynamodb_client.delete_item(TableName=table, Key=key) + seeded = self._seed(seed, n) + try: + if seeded: + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "k"}, **seeded}) + dynamodb_client.update_item(TableName=table, Key=key, **build(n)) + accepted = True + except ClientError as exc: + assert exc.response["Error"]["Code"] == "ValidationException" + assert "size" in exc.response["Error"]["Message"], exc.response["Error"]["Message"] + accepted = False + assert accepted is ok, f"{name}: padding {n} {'accepted' if accepted else 'refused'}" + + +@on_default_backends_only +class TestTransactedUpdateSizeRule: + """A transacted Update is sized on the finished item and cancels with the update wording.""" + + def _txn(self, client, table, finished): + pad = finished - (2 + 1 + 1) + client.transact_write_items(TransactItems=[{ + "Update": { + "TableName": table, "Key": {"pk": {"S": "t"}}, + "UpdateExpression": "SET p = :v", + "ExpressionAttributeValues": {":v": {"S": "x" * pad}}, + } + }]) + + def test_exactly_the_limit_is_stored(self, dynamodb_client, table): + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": "t"}}) + self._txn(dynamodb_client, table, MAX) + got = dynamodb_client.get_item(TableName=table, Key={"pk": {"S": "t"}}, ConsistentRead=True) + assert len(got["Item"]["p"]["S"]) == MAX - 4 + + def test_one_byte_over_cancels_with_the_update_wording(self, dynamodb_client, table): + dynamodb_client.delete_item(TableName=table, Key={"pk": {"S": "t"}}) + with pytest.raises(ClientError) as exc_info: + self._txn(dynamodb_client, table, MAX + 1) + resp = exc_info.value.response + assert resp["Error"]["Code"] == "TransactionCanceledException" + reasons = resp["CancellationReasons"] + assert reasons[0]["Code"] == "ValidationError" + assert reasons[0]["Message"] == UPDATE_WORDING + + +@on_default_backends_only +class TestEmptyBinaryIndexKeyOnUpdate: + """UpdateItem setting a GSI key to an empty binary is refused with the update-form message.""" + + def test_update_refuses_empty_binary_index_key(self, dynamodb_client, gsi_table): + dynamodb_client.put_item(TableName=gsi_table, Item={"pk": {"S": "eb"}, "v": {"S": "1"}}) + with pytest.raises(ClientError) as exc_info: + dynamodb_client.update_item( + TableName=gsi_table, Key={"pk": {"S": "eb"}}, + UpdateExpression="SET bidx = :b", + ExpressionAttributeValues={":b": {"B": b""}}, + ) + assert _err(exc_info)["Code"] == "ValidationException" + assert _err(exc_info)["Message"] == ( + "One or more parameter values are not valid. The update expression attempted to " + "update a secondary index key to a value that is not supported. The AttributeValue " + "for a key attribute cannot contain an empty binary value." + ) + got = dynamodb_client.get_item(TableName=gsi_table, Key={"pk": {"S": "eb"}}, ConsistentRead=True) + assert "bidx" not in got["Item"]