diff --git a/crates/storage-mongodb/src/pushdown.rs b/crates/storage-mongodb/src/pushdown.rs index 616ef7508..88341cb75 100644 --- a/crates/storage-mongodb/src/pushdown.rs +++ b/crates/storage-mongodb/src/pushdown.rs @@ -41,6 +41,9 @@ //! - `path :v` where `:v` is `B` (base64 string ordering //! ≠ bytewise byte ordering across mismatched lengths) //! - Any operand type the analyzer doesn't yet classify +//! - A condition whose `AND`, `OR` and `NOT` nest more than +//! 64 levels deep (`MAX_LOGICAL_DEPTH`; the compiled filter would pass +//! MongoDB's BSON depth limit) use extenddb_core::expression::{CompareOp, Expr, ExpressionMaps, PathElement}; use extenddb_core::types::AttributeValue; @@ -62,6 +65,19 @@ impl Pushable { } } +/// Deepest nesting of `AND`, `OR` and `NOT` compiled to a MongoDB filter. +/// +/// The compiler emits one `$and`, `$or` or `$nor` level per operator, and +/// MongoDB refuses a command nested past its BSON depth limit. Measured on +/// mongo:7.0.45 through the calls the pushdown makes (`find_one` for +/// UpdateItem, `find_one_and_delete` for DeleteItem), with the key filter +/// merged in: a condition nested 98 levels runs and 99 answers error 15, +/// "BSONObj exceeds maximum nested object depth"; with `contains` leaves, the +/// deepest leaf the compiler emits, 97 runs and 98 fails. A deeper condition +/// is evaluated in Rust instead; the margin below 97 leaves room for deeper +/// leaves the compiler may emit later. +const MAX_LOGICAL_DEPTH: usize = 64; + /// Decide whether a compiled MongoDB filter for `expr` will agree with /// `evaluate_condition(expr, item, maps)` for every item. /// @@ -70,9 +86,24 @@ pub fn is_pushable(expr: &Expr, maps: &ExpressionMaps) -> Pushable { if let Some(reason) = unsafe_attribute_reason(expr, maps) { return Pushable::No(reason); } + if logical_depth(expr) > MAX_LOGICAL_DEPTH { + return Pushable::No("AND, OR and NOT nest deeper than a MongoDB filter allows"); + } walk(expr, maps) } +/// Nesting depth of `AND`, `OR` and `NOT` in `expr`; a leaf is 0. A chain +/// `a OR b OR c` parses left-nested, so `n` terms have depth `n - 1`. +fn logical_depth(expr: &Expr) -> usize { + match expr { + Expr::And(left, right) | Expr::Or(left, right) => { + 1 + logical_depth(left).max(logical_depth(right)) + } + Expr::Not(inner) => 1 + logical_depth(inner), + _ => 0, + } +} + /// Return why an expression contains a literal DynamoDB attribute name that /// cannot safely be emitted as a MongoDB field path. A dot would mean nested /// document traversal in MongoDB, while a leading `$` would be interpreted as @@ -318,318 +349,5 @@ fn arg_resolves_to_scalar_type(expr: &Expr, maps: &ExpressionMaps, expected: Att } #[cfg(test)] -mod tests { - use super::*; - use extenddb_core::expression::PathElement; - use std::collections::HashMap; - - fn maps_with(values: &[(&str, AttributeValue)]) -> ExpressionMaps { - let mut m = HashMap::new(); - for (k, v) in values { - m.insert((*k).to_string(), v.clone()); - } - ExpressionMaps::new(HashMap::new(), m) - } - - fn path(name: &str) -> Expr { - Expr::Path(vec![PathElement::Attribute(name.to_string())]) - } - - fn aliased_maps(resolved_name: &str, value: AttributeValue) -> ExpressionMaps { - let mut names = HashMap::new(); - names.insert("name".to_owned(), resolved_name.to_owned()); - let mut values = HashMap::new(); - values.insert(":value".to_owned(), value); - ExpressionMaps::new(names, values) - } - - #[test] - fn attribute_exists_is_pushable() { - let expr = Expr::Function { - name: "attribute_exists".into(), - args: vec![path("a")], - }; - assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); - } - - #[test] - fn dotted_attribute_alias_is_not_pushable() { - let expr = Expr::Compare { - left: Box::new(path("#name")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":value".into())), - }; - let maps = aliased_maps("a.b", AttributeValue::S("value".into())); - - assert_eq!( - is_pushable(&expr, &maps), - Pushable::No("literal attribute name contains '.'") - ); - } - - #[test] - fn dotted_attribute_alias_in_function_is_not_pushable() { - let expr = Expr::Function { - name: "attribute_exists".into(), - args: vec![path("#name")], - }; - let maps = aliased_maps("a.b", AttributeValue::S("unused".into())); - - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn dollar_prefixed_attribute_in_condition_is_not_pushable() { - let expr = Expr::Compare { - left: Box::new(path("$foo")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":value".into())), - }; - let maps = maps_with(&[(":value", AttributeValue::S("value".into()))]); - - assert_eq!( - is_pushable(&expr, &maps), - Pushable::No("literal attribute name begins with '$'") - ); - } - - #[test] - fn nested_document_path_without_literal_dot_remains_pushable() { - let expr = Expr::Compare { - left: Box::new(Expr::Path(vec![ - PathElement::Attribute("a".into()), - PathElement::Attribute("b".into()), - ])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":value".into())), - }; - let maps = maps_with(&[(":value", AttributeValue::S("value".into()))]); - - assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); - } - - #[test] - fn size_is_not_pushable() { - let expr = Expr::Function { - name: "size".into(), - args: vec![path("a")], - }; - assert!(!is_pushable(&expr, &maps_with(&[])).is_yes()); - } - - #[test] - fn number_operand_is_not_pushable() { - let expr = Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":n".into())), - }; - let maps = maps_with(&[(":n", AttributeValue::N("42".into()))]); - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn field_vs_field_is_not_pushable() { - // Both operands are plain fields whose runtime types are unknown at - // compile time. Pushing $expr would compare tagged subdocuments - // (lexical for Numbers), so this must fall back to the in-Rust - // evaluator — for every comparator, not just ordering ones. - for op in [ - CompareOp::Eq, - CompareOp::Ne, - CompareOp::Lt, - CompareOp::Le, - CompareOp::Gt, - CompareOp::Ge, - ] { - let expr = Expr::Compare { - left: Box::new(path("counter_a")), - op, - right: Box::new(path("counter_b")), - }; - assert!( - !is_pushable(&expr, &maps_with(&[])).is_yes(), - "Field vs Field must not be pushable for {op:?}" - ); - } - } - - #[test] - fn string_equality_is_pushable() { - let expr = Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":s".into())), - }; - let maps = maps_with(&[(":s", AttributeValue::S("x".into()))]); - assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); - } - - #[test] - fn binary_equality_is_pushable() { - let expr = Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":b".into())), - }; - let maps = maps_with(&[(":b", AttributeValue::B(vec![0, 1, 2]))]); - assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); - } - - #[test] - fn binary_ordering_is_not_pushable() { - let expr = Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Lt, - right: Box::new(Expr::Placeholder(":b".into())), - }; - let maps = maps_with(&[(":b", AttributeValue::B(vec![0]))]); - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn not_attribute_exists_is_pushable() { - let expr = Expr::Not(Box::new(Expr::Function { - name: "attribute_exists".into(), - args: vec![path("a")], - })); - assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); - } - - #[test] - fn not_around_comparison_is_not_pushable() { - let expr = Expr::Not(Box::new(Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":s".into())), - })); - let maps = maps_with(&[(":s", AttributeValue::S("x".into()))]); - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn and_of_two_pushable_is_pushable() { - let expr = Expr::And( - Box::new(Expr::Function { - name: "attribute_exists".into(), - args: vec![path("a")], - }), - Box::new(Expr::Function { - name: "attribute_exists".into(), - args: vec![path("b")], - }), - ); - assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); - } - - #[test] - fn and_taints_on_either_side() { - let expr = Expr::And( - Box::new(Expr::Function { - name: "attribute_exists".into(), - args: vec![path("a")], - }), - Box::new(Expr::Function { - name: "size".into(), - args: vec![path("b")], - }), - ); - assert!(!is_pushable(&expr, &maps_with(&[])).is_yes()); - } - - #[test] - fn between_is_not_pushable() { - let expr = Expr::Between { - operand: Box::new(path("a")), - low: Box::new(Expr::Placeholder(":lo".into())), - high: Box::new(Expr::Placeholder(":hi".into())), - }; - let maps = maps_with(&[ - (":lo", AttributeValue::S("a".into())), - (":hi", AttributeValue::S("z".into())), - ]); - // BETWEEN is currently non-pushable pending analyzer coverage; - // this test locks in that decision. - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - // ── D-M6 exclusion tests ──────────────────────────────────────── - // - // The condition compiler in `condition.rs` has latent correctness - // bugs on numeric operands, set/list/map equality, mixed-type IN - // lists, and unvalidated attribute_type tags. The A5 analyzer is - // supposed to keep every one of those input shapes out of the - // pushdown path. These tests lock in that boundary so a future - // widening of the compiler doesn't accidentally admit a buggy - // shape without the analyzer being updated. - - #[test] - fn numeric_compare_is_not_pushable() { - // Numeric operands would compile to BSON string comparisons — - // the analyzer must reject. - let maps = maps_with(&[(":n", AttributeValue::N("42".into()))]); - let expr = Expr::Compare { - left: Box::new(path("a")), - op: CompareOp::Lt, - right: Box::new(Expr::Placeholder(":n".into())), - }; - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn in_is_not_pushable() { - // IN with mixed-type list uses the first literal's type for - // all entries — analyzer must reject entirely. - let maps = maps_with(&[ - (":a", AttributeValue::S("x".into())), - (":b", AttributeValue::N("1".into())), - ]); - let expr = Expr::In { - operand: Box::new(path("a")), - list: vec![ - Expr::Placeholder(":a".into()), - Expr::Placeholder(":b".into()), - ], - }; - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn set_equality_is_not_pushable() { - // Eq/Ne on SS/NS/BS compiles to Bson::Null in the compiler. - // Analyzer classifies as `un-inferrable operand kind` and - // must reject. - let ss: std::collections::BTreeSet = - ["a".to_owned(), "b".to_owned()].into_iter().collect(); - let maps = maps_with(&[(":s", AttributeValue::SS(ss))]); - let expr = Expr::Compare { - left: Box::new(path("tags")), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":s".into())), - }; - assert!(!is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn attribute_type_valid_tag_is_pushable() { - // "S" is a valid DDB type tag — analyzer admits. - let expr = Expr::Function { - name: "attribute_type".to_owned(), - args: vec![path("a"), Expr::Placeholder(":t".into())], - }; - let maps = maps_with(&[(":t", AttributeValue::S("S".into()))]); - assert!(is_pushable(&expr, &maps).is_yes()); - } - - #[test] - fn attribute_type_invalid_tag_is_not_pushable() { - // An arbitrary string as the tag is refused so the compiler - // never assembles `field.$evil` mongo path fragments. - let expr = Expr::Function { - name: "attribute_type".to_owned(), - args: vec![path("a"), Expr::Placeholder(":t".into())], - }; - let maps = maps_with(&[(":t", AttributeValue::S("$ne".into()))]); - assert!(!is_pushable(&expr, &maps).is_yes()); - } -} +#[path = "pushdown_tests.rs"] +mod tests; diff --git a/crates/storage-mongodb/src/pushdown_tests.rs b/crates/storage-mongodb/src/pushdown_tests.rs new file mode 100644 index 000000000..cd1b1d03b --- /dev/null +++ b/crates/storage-mongodb/src/pushdown_tests.rs @@ -0,0 +1,359 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +use super::*; +use extenddb_core::expression::PathElement; +use std::collections::HashMap; + +fn maps_with(values: &[(&str, AttributeValue)]) -> ExpressionMaps { + let mut m = HashMap::new(); + for (k, v) in values { + m.insert((*k).to_string(), v.clone()); + } + ExpressionMaps::new(HashMap::new(), m) +} + +fn path(name: &str) -> Expr { + Expr::Path(vec![PathElement::Attribute(name.to_string())]) +} + +fn aliased_maps(resolved_name: &str, value: AttributeValue) -> ExpressionMaps { + let mut names = HashMap::new(); + names.insert("name".to_owned(), resolved_name.to_owned()); + let mut values = HashMap::new(); + values.insert(":value".to_owned(), value); + ExpressionMaps::new(names, values) +} + +#[test] +fn attribute_exists_is_pushable() { + let expr = Expr::Function { + name: "attribute_exists".into(), + args: vec![path("a")], + }; + assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); +} + +#[test] +fn dotted_attribute_alias_is_not_pushable() { + let expr = Expr::Compare { + left: Box::new(path("#name")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":value".into())), + }; + let maps = aliased_maps("a.b", AttributeValue::S("value".into())); + + assert_eq!( + is_pushable(&expr, &maps), + Pushable::No("literal attribute name contains '.'") + ); +} + +#[test] +fn dotted_attribute_alias_in_function_is_not_pushable() { + let expr = Expr::Function { + name: "attribute_exists".into(), + args: vec![path("#name")], + }; + let maps = aliased_maps("a.b", AttributeValue::S("unused".into())); + + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn dollar_prefixed_attribute_in_condition_is_not_pushable() { + let expr = Expr::Compare { + left: Box::new(path("$foo")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":value".into())), + }; + let maps = maps_with(&[(":value", AttributeValue::S("value".into()))]); + + assert_eq!( + is_pushable(&expr, &maps), + Pushable::No("literal attribute name begins with '$'") + ); +} + +#[test] +fn nested_document_path_without_literal_dot_remains_pushable() { + let expr = Expr::Compare { + left: Box::new(Expr::Path(vec![ + PathElement::Attribute("a".into()), + PathElement::Attribute("b".into()), + ])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":value".into())), + }; + let maps = maps_with(&[(":value", AttributeValue::S("value".into()))]); + + assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); +} + +#[test] +fn size_is_not_pushable() { + let expr = Expr::Function { + name: "size".into(), + args: vec![path("a")], + }; + assert!(!is_pushable(&expr, &maps_with(&[])).is_yes()); +} + +#[test] +fn number_operand_is_not_pushable() { + let expr = Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":n".into())), + }; + let maps = maps_with(&[(":n", AttributeValue::N("42".into()))]); + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn field_vs_field_is_not_pushable() { + // Both operands are plain fields whose runtime types are unknown at + // compile time. Pushing $expr would compare tagged subdocuments + // (lexical for Numbers), so this must fall back to the in-Rust + // evaluator — for every comparator, not just ordering ones. + for op in [ + CompareOp::Eq, + CompareOp::Ne, + CompareOp::Lt, + CompareOp::Le, + CompareOp::Gt, + CompareOp::Ge, + ] { + let expr = Expr::Compare { + left: Box::new(path("counter_a")), + op, + right: Box::new(path("counter_b")), + }; + assert!( + !is_pushable(&expr, &maps_with(&[])).is_yes(), + "Field vs Field must not be pushable for {op:?}" + ); + } +} + +#[test] +fn string_equality_is_pushable() { + let expr = Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":s".into())), + }; + let maps = maps_with(&[(":s", AttributeValue::S("x".into()))]); + assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); +} + +#[test] +fn binary_equality_is_pushable() { + let expr = Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":b".into())), + }; + let maps = maps_with(&[(":b", AttributeValue::B(vec![0, 1, 2]))]); + assert_eq!(is_pushable(&expr, &maps), Pushable::Yes); +} + +#[test] +fn binary_ordering_is_not_pushable() { + let expr = Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Lt, + right: Box::new(Expr::Placeholder(":b".into())), + }; + let maps = maps_with(&[(":b", AttributeValue::B(vec![0]))]); + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn not_attribute_exists_is_pushable() { + let expr = Expr::Not(Box::new(Expr::Function { + name: "attribute_exists".into(), + args: vec![path("a")], + })); + assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); +} + +#[test] +fn not_around_comparison_is_not_pushable() { + let expr = Expr::Not(Box::new(Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":s".into())), + })); + let maps = maps_with(&[(":s", AttributeValue::S("x".into()))]); + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn and_of_two_pushable_is_pushable() { + let expr = Expr::And( + Box::new(Expr::Function { + name: "attribute_exists".into(), + args: vec![path("a")], + }), + Box::new(Expr::Function { + name: "attribute_exists".into(), + args: vec![path("b")], + }), + ); + assert_eq!(is_pushable(&expr, &maps_with(&[])), Pushable::Yes); +} + +#[test] +fn and_taints_on_either_side() { + let expr = Expr::And( + Box::new(Expr::Function { + name: "attribute_exists".into(), + args: vec![path("a")], + }), + Box::new(Expr::Function { + name: "size".into(), + args: vec![path("b")], + }), + ); + assert!(!is_pushable(&expr, &maps_with(&[])).is_yes()); +} + +/// `#v = :s OR #v = :s OR ...` with `terms` terms, left-nested as parsed. +fn or_chain(terms: usize) -> Expr { + let term = || Expr::Compare { + left: Box::new(path("v")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":s".into())), + }; + (1..terms).fold(term(), |acc, _| Expr::Or(Box::new(acc), Box::new(term()))) +} + +#[test] +fn logical_nesting_past_the_limit_is_not_pushable() { + let maps = maps_with(&[(":s", AttributeValue::S("x".into()))]); + assert_eq!( + is_pushable(&or_chain(MAX_LOGICAL_DEPTH + 1), &maps), + Pushable::Yes + ); + assert_eq!( + is_pushable(&or_chain(MAX_LOGICAL_DEPTH + 2), &maps), + Pushable::No("AND, OR and NOT nest deeper than a MongoDB filter allows") + ); + // A 150-term chain, which nested 149 `$or` levels in one filter. + assert!(!is_pushable(&or_chain(150), &maps).is_yes()); + // Depth 97, the first depth at which a chain of `contains` leaves fails + // on MongoDB, is refused whatever the limit is set to. + assert!(!is_pushable(&or_chain(98), &maps).is_yes()); + // Depth is the deeper side's: the NOT on the left adds one level to a + // depth-1 side, the chain on the right sets the depth. + let not_wrapped = Expr::And( + Box::new(Expr::Not(Box::new(Expr::Function { + name: "attribute_exists".into(), + args: vec![path("a")], + }))), + Box::new(or_chain(MAX_LOGICAL_DEPTH)), + ); + assert_eq!(logical_depth(¬_wrapped), MAX_LOGICAL_DEPTH); + // NOT counts as a level. + assert_eq!( + logical_depth(&Expr::Not(Box::new(or_chain(MAX_LOGICAL_DEPTH + 1)))), + MAX_LOGICAL_DEPTH + 1 + ); +} + +#[test] +fn between_is_not_pushable() { + let expr = Expr::Between { + operand: Box::new(path("a")), + low: Box::new(Expr::Placeholder(":lo".into())), + high: Box::new(Expr::Placeholder(":hi".into())), + }; + let maps = maps_with(&[ + (":lo", AttributeValue::S("a".into())), + (":hi", AttributeValue::S("z".into())), + ]); + // BETWEEN is currently non-pushable pending analyzer coverage; + // this test locks in that decision. + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +// ── D-M6 exclusion tests ──────────────────────────────────────── +// +// The condition compiler in `condition.rs` has latent correctness +// bugs on numeric operands, set/list/map equality, mixed-type IN +// lists, and unvalidated attribute_type tags. The A5 analyzer is +// supposed to keep every one of those input shapes out of the +// pushdown path. These tests lock in that boundary so a future +// widening of the compiler doesn't accidentally admit a buggy +// shape without the analyzer being updated. + +#[test] +fn numeric_compare_is_not_pushable() { + // Numeric operands would compile to BSON string comparisons — + // the analyzer must reject. + let maps = maps_with(&[(":n", AttributeValue::N("42".into()))]); + let expr = Expr::Compare { + left: Box::new(path("a")), + op: CompareOp::Lt, + right: Box::new(Expr::Placeholder(":n".into())), + }; + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn in_is_not_pushable() { + // IN with mixed-type list uses the first literal's type for + // all entries — analyzer must reject entirely. + let maps = maps_with(&[ + (":a", AttributeValue::S("x".into())), + (":b", AttributeValue::N("1".into())), + ]); + let expr = Expr::In { + operand: Box::new(path("a")), + list: vec![ + Expr::Placeholder(":a".into()), + Expr::Placeholder(":b".into()), + ], + }; + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn set_equality_is_not_pushable() { + // Eq/Ne on SS/NS/BS compiles to Bson::Null in the compiler. + // Analyzer classifies as `un-inferrable operand kind` and + // must reject. + let ss: std::collections::BTreeSet = + ["a".to_owned(), "b".to_owned()].into_iter().collect(); + let maps = maps_with(&[(":s", AttributeValue::SS(ss))]); + let expr = Expr::Compare { + left: Box::new(path("tags")), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":s".into())), + }; + assert!(!is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn attribute_type_valid_tag_is_pushable() { + // "S" is a valid DDB type tag — analyzer admits. + let expr = Expr::Function { + name: "attribute_type".to_owned(), + args: vec![path("a"), Expr::Placeholder(":t".into())], + }; + let maps = maps_with(&[(":t", AttributeValue::S("S".into()))]); + assert!(is_pushable(&expr, &maps).is_yes()); +} + +#[test] +fn attribute_type_invalid_tag_is_not_pushable() { + // An arbitrary string as the tag is refused so the compiler + // never assembles `field.$evil` mongo path fragments. + let expr = Expr::Function { + name: "attribute_type".to_owned(), + args: vec![path("a"), Expr::Placeholder(":t".into())], + }; + let maps = maps_with(&[(":t", AttributeValue::S("$ne".into()))]); + assert!(!is_pushable(&expr, &maps).is_yes()); +} diff --git a/docs/design/13-storage-mongodb.md b/docs/design/13-storage-mongodb.md index 9612c00b9..57ffff89b 100644 --- a/docs/design/13-storage-mongodb.md +++ b/docs/design/13-storage-mongodb.md @@ -531,7 +531,12 @@ provably in agreement with `evaluate_condition`: Not pushable: any operand of type `N` (numbers stored as strings; `"10" > "9"` is false lex-wise), `size` (MongoDB has no UTF-16 code-unit count), `BETWEEN` / `IN` (pending proptest coverage; the compiler emits -them, the analyzer refuses them), and `NOT` around anything else. +them, the analyzer refuses them), `NOT` around anything else, and a +condition whose `AND`, `OR` and `NOT` nest more than 64 levels deep. The +compiler emits one `$and`, `$or` or `$nor` level per operator, and MongoDB +refuses a filter nested past its BSON depth limit: measured on mongo:7, the +merged filter runs with 98 nested levels and fails with 99 (97 and 98 with +`contains` leaves). Property tests (`tests/pushdown_parity.rs`) generate random items and expressions, compile the filter, and check that a pure-Rust BSON diff --git a/tests/test_condition_nesting_depth.py b/tests/test_condition_nesting_depth.py new file mode 100644 index 000000000..73a6ca2ba --- /dev/null +++ b/tests/test_condition_nesting_depth.py @@ -0,0 +1,214 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Long and deeply nested condition expressions on UpdateItem and DeleteItem. + +Amazon DynamoDB evaluates a condition whose AND, OR and NOT chains run 150 +terms long or nest 140 levels deep, and a legacy ``Expected`` map of 120 +entries joined by ``ConditionalOperator``: it answers +ConditionalCheckFailedException when the condition fails and applies the +write when it holds. Measured on 2026-10-25 at 18:34 and 19:53 UTC +(us-east-1), each failing case twice. + +On MongoDB such a condition on a table with no index and no stream used to be +compiled to one filter nested one level per operator, which MongoDB refused, +and the request answered InternalServerError. The server takes that path for +60 seconds after a write records that the table has no index, so each test +gets a table of its own and writes an item to it before the conditional +write. + +Dual-target against Amazon DynamoDB and extenddb. +""" + +from __future__ import annotations + +import uuid + +import pytest +from botocore.exceptions import ClientError + +from conftest import scoped_table + +NAMES = {"#v": "v", "#u": "u"} +VALUES = {":v": {"S": "x"}, ":w": {"S": "y"}, ":x": {"S": "z"}} +UPDATE = "SET #u = :x" + + +def or_chain(n: int, term: str = "#v = :w") -> str: + return " OR ".join([term] * n) + + +def and_chain(n: int, term: str = "attribute_exists(pk)") -> str: + return " AND ".join([term] * n) + + +def alternating(depth: int) -> str: + """``#v = :w OR (#v = :w AND (#v = :w OR (...)))``, ``depth`` operators deep.""" + expr = "#v = :w" + for i in range(depth): + expr = f"#v = :w {'OR' if i % 2 == 0 else 'AND'} ({expr})" + return expr + + +def _maps(*exprs: str) -> dict: + text = " ".join(exprs) + out = {} + names = {k: v for k, v in NAMES.items() if k in text} + values = {k: v for k, v in VALUES.items() if k in text} + if names: + out["ExpressionAttributeNames"] = names + if values: + out["ExpressionAttributeValues"] = values + return out + + +def update(client, table, key, condition): + return client.update_item( + TableName=table, + Key=key, + UpdateExpression=UPDATE, + ConditionExpression=condition, + **_maps(UPDATE, condition), + ) + + +def delete(client, table, key, condition): + return client.delete_item( + TableName=table, Key=key, ConditionExpression=condition, **_maps(condition) + ) + + +@pytest.fixture +def table(dynamodb_client): + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "sk", "AttributeType": "S"}, + ], + key_schema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + ) as name: + yield name + + +@pytest.fixture +def warm(dynamodb_client, table): + dynamodb_client.put_item(TableName=table, Item={"pk": {"S": "warm"}, "sk": {"S": "warm"}}) + + +@pytest.fixture +def live_key(dynamodb_client, table, warm): + key = {"pk": {"S": f"live-{uuid.uuid4().hex[:8]}"}, "sk": {"S": "live"}} + dynamodb_client.put_item(TableName=table, Item={**key, "v": {"S": "x"}}) + return key + + +def _get(client, table, key): + return client.get_item(TableName=table, Key=key, ConsistentRead=True).get("Item") + + +@pytest.mark.parametrize( + "write, condition", + [ + (update, or_chain(150)), + (delete, or_chain(150)), + (update, and_chain(150)), + (delete, and_chain(150)), + (update, alternating(100)), + (delete, alternating(140)), + (update, f"attribute_exists(pk) AND ({or_chain(120)})"), + ], + ids=[ + "update-or-150", + "delete-or-150", + "update-and-150", + "delete-and-150", + "update-alternating-100", + "delete-alternating-140", + "update-or-120-inside-and", + ], +) +def test_failing_condition_is_evaluated(dynamodb_client, table, warm, write, condition): + key = {"pk": {"S": f"absent-{uuid.uuid4().hex[:8]}"}, "sk": {"S": "absent"}} + with pytest.raises(ClientError) as exc_info: + write(dynamodb_client, table, key, condition) + err = exc_info.value.response["Error"] + assert err["Code"] == "ConditionalCheckFailedException", err + assert _get(dynamodb_client, table, key) is None + + +@pytest.mark.parametrize( + "condition", + [ + or_chain(149) + " OR #v = :v", + and_chain(150), + "#v = :v OR (" + alternating(99) + ")", + ], + ids=["or-150-last-holds", "and-150", "alternating-100-outermost-holds"], +) +def test_update_with_holding_condition_applies(dynamodb_client, table, live_key, condition): + update(dynamodb_client, table, live_key, condition) + assert _get(dynamodb_client, table, live_key)["u"] == {"S": "z"} + + +@pytest.mark.parametrize( + "condition", + [ + or_chain(149) + " OR #v = :v", + and_chain(150), + "#v = :v OR (" + alternating(139) + ")", + ], + ids=["or-150-last-holds", "and-150", "alternating-140-outermost-holds"], +) +def test_delete_with_holding_condition_applies(dynamodb_client, table, live_key, condition): + delete(dynamodb_client, table, live_key, condition) + assert _get(dynamodb_client, table, live_key) is None + + +def expected(n: int, last_holds: bool = False) -> dict: + """``n`` legacy ``Expected`` entries ``a EQ "no"``; the last one ``EQ "x"``.""" + entries = { + f"a{i}": {"ComparisonOperator": "EQ", "AttributeValueList": [{"S": "no"}]} + for i in range(n) + } + if last_holds: + entries[f"a{n - 1}"]["AttributeValueList"] = [{"S": "x"}] + return entries + + +ATTRIBUTE_UPDATES = {"u": {"Action": "PUT", "Value": {"S": "z"}}} + + +@pytest.mark.parametrize("operation", ["update_item", "delete_item"]) +def test_legacy_expected_failing_is_evaluated(dynamodb_client, table, warm, operation): + key = {"pk": {"S": f"absent-{uuid.uuid4().hex[:8]}"}, "sk": {"S": "absent"}} + extra = {"AttributeUpdates": ATTRIBUTE_UPDATES} if operation == "update_item" else {} + with pytest.raises(ClientError) as exc_info: + getattr(dynamodb_client, operation)( + TableName=table, Key=key, Expected=expected(120), ConditionalOperator="OR", **extra + ) + err = exc_info.value.response["Error"] + assert err["Code"] == "ConditionalCheckFailedException", err + assert _get(dynamodb_client, table, key) is None + + +@pytest.mark.parametrize("operation", ["update_item", "delete_item"]) +def test_legacy_expected_holding_applies(dynamodb_client, table, warm, operation): + key = {"pk": {"S": f"live-{uuid.uuid4().hex[:8]}"}, "sk": {"S": "live"}} + dynamodb_client.put_item(TableName=table, Item={**key, "a119": {"S": "x"}}) + extra = {"AttributeUpdates": ATTRIBUTE_UPDATES} if operation == "update_item" else {} + getattr(dynamodb_client, operation)( + TableName=table, + Key=key, + Expected=expected(120, last_holds=True), + ConditionalOperator="OR", + **extra, + ) + item = _get(dynamodb_client, table, key) + if operation == "update_item": + assert item["u"] == {"S": "z"} + else: + assert item is None diff --git a/tests/test_expression_operator_limits.py b/tests/test_expression_operator_limits.py index 211f39a8d..4d52da9b6 100644 --- a/tests/test_expression_operator_limits.py +++ b/tests/test_expression_operator_limits.py @@ -29,8 +29,6 @@ from __future__ import annotations -import os - import pytest from botocore.exceptions import ClientError @@ -40,19 +38,6 @@ TOO_MANY = "The expression contains too many operators; operator count: " -# On MongoDB an UpdateItem condition is compiled to a filter whose OR and AND -# chains nest one level per operator, and a chain of 150 terms fails inside -# MongoDB with InternalServerError. This change does not touch that compiler. -# The MongoDB test runner sets EXTENDDB_TEST_MONGODB_CONTAINER. -# TODO: remove this marker once the MongoDB filter compiler flattens chains. -XFAIL_ON_MONGODB_DEEP_OR = pytest.mark.xfail( - bool(os.environ.get("EXTENDDB_TEST_MONGODB_CONTAINER", "").strip()), - reason="MongoDB answers InternalServerError for a condition chain of 150 terms", - raises=AssertionError, - strict=True, -) - - def _error(func) -> tuple[str, str]: with pytest.raises(ClientError) as exc_info: func() @@ -200,7 +185,6 @@ def test_update_item_condition_count(self, dynamodb_client, table): ) assert _after_prefix(msg) == f"Invalid ConditionExpression: {TOO_MANY}599" - @XFAIL_ON_MONGODB_DEEP_OR def test_update_item_condition_at_the_cap_is_evaluated(self, dynamodb_client, table): # [u03] 300 accepted (condition evaluated). code, _ = _error(