diff --git a/crates/storage-mongodb/src/condition.rs b/crates/storage-mongodb/src/condition.rs index c2301b941..d3bab9a81 100644 --- a/crates/storage-mongodb/src/condition.rs +++ b/crates/storage-mongodb/src/condition.rs @@ -22,14 +22,17 @@ pub fn condition_to_filter(expr: &Expr, maps: &ExpressionMaps) -> Result Result { let mut parts = vec!["item_data".to_string()]; - for elem in elements { + for (position, elem) in elements.iter().enumerate() { match elem { PathElement::Attribute(name) => { let resolved = if let Some(stripped) = name.strip_prefix('#') { @@ -39,6 +42,11 @@ fn resolve_path_to_field( } else { name.clone() }; + // A member name after the top-level attribute is a key of + // the enclosing map. + if position > 0 { + parts.push("M".to_string()); + } parts.push(resolved); } PathElement::Index(idx) => { @@ -493,267 +501,5 @@ fn regex_escape(s: &str) -> String { // ============================================================================ #[cfg(test)] -mod tests { - use super::*; - use extenddb_core::expression::ExpressionMaps; - use extenddb_core::types::AttributeValue; - use std::collections::HashMap; - - fn make_maps(names: Vec<(&str, &str)>, values: Vec<(&str, AttributeValue)>) -> ExpressionMaps { - let names_map: HashMap = names - .into_iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect(); - let values_map: HashMap = values - .into_iter() - .map(|(k, v)| (k.to_string(), v)) - .collect(); - ExpressionMaps::new(names_map, values_map) - } - - #[test] - fn test_attribute_exists() { - let maps = make_maps(vec![], vec![]); - let expr = Expr::Function { - name: "attribute_exists".to_string(), - args: vec![Expr::Path(vec![PathElement::Attribute("foo".to_string())])], - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.foo": { "$exists": true } }); - } - - #[test] - fn test_attribute_not_exists() { - let maps = make_maps(vec![], vec![]); - let expr = Expr::Function { - name: "attribute_not_exists".to_string(), - args: vec![Expr::Path(vec![PathElement::Attribute("bar".to_string())])], - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.bar": { "$exists": false } }); - } - - #[test] - fn test_equality_comparison_string() { - let maps = make_maps( - vec![], - vec![(":val", AttributeValue::S("hello".to_string()))], - ); - let expr = Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute("name".to_string())])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":val".to_string())), - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.name.S": "hello" }); - } - - #[test] - fn test_less_than_comparison_number() { - let maps = make_maps(vec![], vec![(":min", AttributeValue::N("100".to_string()))]); - let expr = Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute( - "price".to_string(), - )])), - op: CompareOp::Lt, - right: Box::new(Expr::Placeholder(":min".to_string())), - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.price.N": { "$lt": "100" } }); - } - - #[test] - fn test_and_condition() { - let maps = make_maps( - vec![], - vec![ - (":v1", AttributeValue::S("active".to_string())), - (":v2", AttributeValue::N("5".to_string())), - ], - ); - let expr = Expr::And( - Box::new(Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute( - "status".to_string(), - )])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":v1".to_string())), - }), - Box::new(Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute( - "count".to_string(), - )])), - op: CompareOp::Gt, - right: Box::new(Expr::Placeholder(":v2".to_string())), - }), - ); - let filter = condition_to_filter(&expr, &maps).unwrap(); - let expected = doc! { - "$and": [ - { "item_data.status.S": "active" }, - { "item_data.count.N": { "$gt": "5" } } - ] - }; - assert_eq!(filter, expected); - } - - #[test] - fn test_or_condition() { - let maps = make_maps( - vec![], - vec![ - (":a", AttributeValue::S("x".to_string())), - (":b", AttributeValue::S("y".to_string())), - ], - ); - let expr = Expr::Or( - Box::new(Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute("f".to_string())])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":a".to_string())), - }), - Box::new(Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute("f".to_string())])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":b".to_string())), - }), - ); - let filter = condition_to_filter(&expr, &maps).unwrap(); - let expected = doc! { - "$or": [ - { "item_data.f.S": "x" }, - { "item_data.f.S": "y" } - ] - }; - assert_eq!(filter, expected); - } - - #[test] - fn test_not_condition() { - let maps = make_maps(vec![], vec![]); - let expr = Expr::Not(Box::new(Expr::Function { - name: "attribute_exists".to_string(), - args: vec![Expr::Path(vec![PathElement::Attribute( - "deleted".to_string(), - )])], - })); - let filter = condition_to_filter(&expr, &maps).unwrap(); - let expected = doc! { - "$nor": [{ "item_data.deleted": { "$exists": true } }] - }; - assert_eq!(filter, expected); - } - - #[test] - fn test_begins_with() { - let maps = make_maps( - vec![], - vec![(":prefix", AttributeValue::S("user#".to_string()))], - ); - let expr = Expr::Function { - name: "begins_with".to_string(), - args: vec![ - Expr::Path(vec![PathElement::Attribute("sk".to_string())]), - Expr::Placeholder(":prefix".to_string()), - ], - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.sk.S": { "$regex": "^user#" } }); - } - - #[test] - fn test_between() { - let maps = make_maps( - vec![], - vec![ - (":lo", AttributeValue::N("10".to_string())), - (":hi", AttributeValue::N("20".to_string())), - ], - ); - let expr = Expr::Between { - operand: Box::new(Expr::Path(vec![PathElement::Attribute("age".to_string())])), - low: Box::new(Expr::Placeholder(":lo".to_string())), - high: Box::new(Expr::Placeholder(":hi".to_string())), - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!( - filter, - doc! { "item_data.age.N": { "$gte": "10", "$lte": "20" } } - ); - } - - #[test] - fn test_in_condition() { - let maps = make_maps( - vec![], - vec![ - (":v1", AttributeValue::S("a".to_string())), - (":v2", AttributeValue::S("b".to_string())), - (":v3", AttributeValue::S("c".to_string())), - ], - ); - let expr = Expr::In { - operand: Box::new(Expr::Path(vec![PathElement::Attribute("x".to_string())])), - list: vec![ - Expr::Placeholder(":v1".to_string()), - Expr::Placeholder(":v2".to_string()), - Expr::Placeholder(":v3".to_string()), - ], - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.x.S": { "$in": ["a", "b", "c"] } }); - } - - #[test] - fn test_name_ref_resolution() { - let maps = make_maps( - vec![("n", "status")], - vec![(":v", AttributeValue::S("active".to_string()))], - ); - let expr = Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute("#n".to_string())])), - op: CompareOp::Eq, - right: Box::new(Expr::Placeholder(":v".to_string())), - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.status.S": "active" }); - } - - #[test] - fn test_ne_comparison() { - let maps = make_maps( - vec![], - vec![(":v", AttributeValue::S("deleted".to_string()))], - ); - let expr = Expr::Compare { - left: Box::new(Expr::Path(vec![PathElement::Attribute( - "status".to_string(), - )])), - op: CompareOp::Ne, - right: Box::new(Expr::Placeholder(":v".to_string())), - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.status.S": { "$ne": "deleted" } }); - } - - #[test] - fn test_regex_escape() { - assert_eq!(regex_escape("user.name"), "user\\.name"); - assert_eq!(regex_escape("a+b"), "a\\+b"); - assert_eq!(regex_escape("normal"), "normal"); - } - - #[test] - fn test_attribute_type() { - let maps = make_maps(vec![], vec![(":t", AttributeValue::S("S".to_string()))]); - let expr = Expr::Function { - name: "attribute_type".to_string(), - args: vec![ - Expr::Path(vec![PathElement::Attribute("field".to_string())]), - Expr::Placeholder(":t".to_string()), - ], - }; - let filter = condition_to_filter(&expr, &maps).unwrap(); - assert_eq!(filter, doc! { "item_data.field.S": { "$exists": true } }); - } -} +#[path = "condition_tests.rs"] +mod tests; diff --git a/crates/storage-mongodb/src/condition_tests.rs b/crates/storage-mongodb/src/condition_tests.rs new file mode 100644 index 000000000..ba381e50c --- /dev/null +++ b/crates/storage-mongodb/src/condition_tests.rs @@ -0,0 +1,365 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +use super::*; +use extenddb_core::expression::ExpressionMaps; +use extenddb_core::types::AttributeValue; +use std::collections::HashMap; + +fn make_maps(names: Vec<(&str, &str)>, values: Vec<(&str, AttributeValue)>) -> ExpressionMaps { + let names_map: HashMap = names + .into_iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + let values_map: HashMap = values + .into_iter() + .map(|(k, v)| (k.to_string(), v)) + .collect(); + ExpressionMaps::new(names_map, values_map) +} + +#[test] +fn test_attribute_exists() { + let maps = make_maps(vec![], vec![]); + let expr = Expr::Function { + name: "attribute_exists".to_string(), + args: vec![Expr::Path(vec![PathElement::Attribute("foo".to_string())])], + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.foo": { "$exists": true } }); +} + +#[test] +fn test_attribute_not_exists() { + let maps = make_maps(vec![], vec![]); + let expr = Expr::Function { + name: "attribute_not_exists".to_string(), + args: vec![Expr::Path(vec![PathElement::Attribute("bar".to_string())])], + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.bar": { "$exists": false } }); +} + +#[test] +fn test_equality_comparison_string() { + let maps = make_maps( + vec![], + vec![(":val", AttributeValue::S("hello".to_string()))], + ); + let expr = Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute("name".to_string())])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":val".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.name.S": "hello" }); +} + +#[test] +fn test_less_than_comparison_number() { + let maps = make_maps(vec![], vec![(":min", AttributeValue::N("100".to_string()))]); + let expr = Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute( + "price".to_string(), + )])), + op: CompareOp::Lt, + right: Box::new(Expr::Placeholder(":min".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.price.N": { "$lt": "100" } }); +} + +#[test] +fn test_and_condition() { + let maps = make_maps( + vec![], + vec![ + (":v1", AttributeValue::S("active".to_string())), + (":v2", AttributeValue::N("5".to_string())), + ], + ); + let expr = Expr::And( + Box::new(Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute( + "status".to_string(), + )])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":v1".to_string())), + }), + Box::new(Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute( + "count".to_string(), + )])), + op: CompareOp::Gt, + right: Box::new(Expr::Placeholder(":v2".to_string())), + }), + ); + let filter = condition_to_filter(&expr, &maps).unwrap(); + let expected = doc! { + "$and": [ + { "item_data.status.S": "active" }, + { "item_data.count.N": { "$gt": "5" } } + ] + }; + assert_eq!(filter, expected); +} + +#[test] +fn test_or_condition() { + let maps = make_maps( + vec![], + vec![ + (":a", AttributeValue::S("x".to_string())), + (":b", AttributeValue::S("y".to_string())), + ], + ); + let expr = Expr::Or( + Box::new(Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute("f".to_string())])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":a".to_string())), + }), + Box::new(Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute("f".to_string())])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":b".to_string())), + }), + ); + let filter = condition_to_filter(&expr, &maps).unwrap(); + let expected = doc! { + "$or": [ + { "item_data.f.S": "x" }, + { "item_data.f.S": "y" } + ] + }; + assert_eq!(filter, expected); +} + +#[test] +fn test_not_condition() { + let maps = make_maps(vec![], vec![]); + let expr = Expr::Not(Box::new(Expr::Function { + name: "attribute_exists".to_string(), + args: vec![Expr::Path(vec![PathElement::Attribute( + "deleted".to_string(), + )])], + })); + let filter = condition_to_filter(&expr, &maps).unwrap(); + let expected = doc! { + "$nor": [{ "item_data.deleted": { "$exists": true } }] + }; + assert_eq!(filter, expected); +} + +#[test] +fn test_begins_with() { + let maps = make_maps( + vec![], + vec![(":prefix", AttributeValue::S("user#".to_string()))], + ); + let expr = Expr::Function { + name: "begins_with".to_string(), + args: vec![ + Expr::Path(vec![PathElement::Attribute("sk".to_string())]), + Expr::Placeholder(":prefix".to_string()), + ], + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.sk.S": { "$regex": "^user#" } }); +} + +#[test] +fn test_between() { + let maps = make_maps( + vec![], + vec![ + (":lo", AttributeValue::N("10".to_string())), + (":hi", AttributeValue::N("20".to_string())), + ], + ); + let expr = Expr::Between { + operand: Box::new(Expr::Path(vec![PathElement::Attribute("age".to_string())])), + low: Box::new(Expr::Placeholder(":lo".to_string())), + high: Box::new(Expr::Placeholder(":hi".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!( + filter, + doc! { "item_data.age.N": { "$gte": "10", "$lte": "20" } } + ); +} + +#[test] +fn test_in_condition() { + let maps = make_maps( + vec![], + vec![ + (":v1", AttributeValue::S("a".to_string())), + (":v2", AttributeValue::S("b".to_string())), + (":v3", AttributeValue::S("c".to_string())), + ], + ); + let expr = Expr::In { + operand: Box::new(Expr::Path(vec![PathElement::Attribute("x".to_string())])), + list: vec![ + Expr::Placeholder(":v1".to_string()), + Expr::Placeholder(":v2".to_string()), + Expr::Placeholder(":v3".to_string()), + ], + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.x.S": { "$in": ["a", "b", "c"] } }); +} + +#[test] +fn test_name_ref_resolution() { + let maps = make_maps( + vec![("n", "status")], + vec![(":v", AttributeValue::S("active".to_string()))], + ); + let expr = Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute("#n".to_string())])), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":v".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.status.S": "active" }); +} + +#[test] +fn test_ne_comparison() { + let maps = make_maps( + vec![], + vec![(":v", AttributeValue::S("deleted".to_string()))], + ); + let expr = Expr::Compare { + left: Box::new(Expr::Path(vec![PathElement::Attribute( + "status".to_string(), + )])), + op: CompareOp::Ne, + right: Box::new(Expr::Placeholder(":v".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.status.S": { "$ne": "deleted" } }); +} + +#[test] +fn test_regex_escape() { + assert_eq!(regex_escape("user.name"), "user\\.name"); + assert_eq!(regex_escape("a+b"), "a\\+b"); + assert_eq!(regex_escape("normal"), "normal"); +} + +#[test] +fn test_attribute_type() { + let maps = make_maps(vec![], vec![(":t", AttributeValue::S("S".to_string()))]); + let expr = Expr::Function { + name: "attribute_type".to_string(), + args: vec![ + Expr::Path(vec![PathElement::Attribute("field".to_string())]), + Expr::Placeholder(":t".to_string()), + ], + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.field.S": { "$exists": true } }); +} + +/// Path elements written as strings: an all-digit string is a list index and +/// anything else is a name. A member whose name is all digits is written as +/// `PathElement::Attribute` and passed to `path` or `exists_at` directly. +fn parse(elements: &[&str]) -> Vec { + elements + .iter() + .map(|e| match e.parse::() { + Ok(i) => PathElement::Index(i), + Err(_) => PathElement::Attribute((*e).to_string()), + }) + .collect() +} + +fn path(elements: Vec) -> Expr { + Expr::Path(elements) +} + +fn exists_at(elements: Vec) -> Document { + let maps = make_maps(vec![("m", "m"), ("k", "k")], vec![]); + let expr = Expr::Function { + name: "attribute_exists".to_string(), + args: vec![path(elements)], + }; + condition_to_filter(&expr, &maps).unwrap() +} + +fn exists(elements: &[&str]) -> Document { + exists_at(parse(elements)) +} + +#[test] +fn map_members_and_list_elements_use_the_typed_fields() { + // A map member sits under the map's `M` field and a list element under + // the list's `L` field, as `item_to_document` stores them. + assert_eq!( + exists(&["#m", "#k"]), + doc! { "item_data.m.M.k": { "$exists": true } } + ); + assert_eq!( + exists(&["m", "mm", "k"]), + doc! { "item_data.m.M.mm.M.k": { "$exists": true } } + ); + assert_eq!( + exists(&["l", "0", "k"]), + doc! { "item_data.l.L.0.M.k": { "$exists": true } } + ); + assert_eq!( + exists(&["m", "n", "1"]), + doc! { "item_data.m.M.n.L.1": { "$exists": true } } + ); + assert_eq!( + exists(&["l", "0"]), + doc! { "item_data.l.L.0": { "$exists": true } } + ); + // A list inside a list: each index gets its own `L` field. + assert_eq!( + exists(&["l", "2", "1"]), + doc! { "item_data.l.L.2.L.1": { "$exists": true } } + ); + // The typed segment is added at every depth, not only the first three. + assert_eq!( + exists(&["m", "mm", "n", "k"]), + doc! { "item_data.m.M.mm.M.n.M.k": { "$exists": true } } + ); + assert_eq!(exists(&["m"]), doc! { "item_data.m": { "$exists": true } }); +} + +#[test] +fn members_named_like_a_type_tag_or_an_index_are_keys_of_the_map() { + // The fields of a stored value are named by its type tag (`M`, `L`, + // `S`, ...) and a digit segment of a MongoDB path can address an array + // element, so a member with one of these names is still reached through + // the enclosing map's `M` field. + let member = |name: &str| { + exists_at(vec![ + PathElement::Attribute("m".to_string()), + PathElement::Attribute(name.to_string()), + ]) + }; + assert_eq!(member("M"), doc! { "item_data.m.M.M": { "$exists": true } }); + assert_eq!(member("L"), doc! { "item_data.m.M.L": { "$exists": true } }); + assert_eq!(member("S"), doc! { "item_data.m.M.S": { "$exists": true } }); + assert_eq!(member("0"), doc! { "item_data.m.M.0": { "$exists": true } }); +} + +#[test] +fn comparison_on_a_map_member_uses_the_typed_field() { + let maps = make_maps( + vec![("m", "m"), ("k", "k")], + vec![(":v", AttributeValue::S("inner".to_string()))], + ); + let expr = Expr::Compare { + left: Box::new(path(parse(&["#m", "#k"]))), + op: CompareOp::Eq, + right: Box::new(Expr::Placeholder(":v".to_string())), + }; + let filter = condition_to_filter(&expr, &maps).unwrap(); + assert_eq!(filter, doc! { "item_data.m.M.k.S": "inner" }); +} diff --git a/crates/storage-mongodb/tests/pushdown_parity.rs b/crates/storage-mongodb/tests/pushdown_parity.rs index 6e3cba54e..9dbce4c4b 100644 --- a/crates/storage-mongodb/tests/pushdown_parity.rs +++ b/crates/storage-mongodb/tests/pushdown_parity.rs @@ -46,6 +46,19 @@ use common::eval_filter; /// expressions is what surfaces path-hit / path-miss interactions. const NAMES: &[&str] = &["a", "b", "c", "x", "y"]; +/// Names of map members, in items and in path elements after the first. +/// `M`, `L` and `S` are also type tags, the names of the fields a stored +/// value sits under, and `0` reads as an array index in a MongoDB path, so a +/// compiled path that drops or misplaces a typed field reaches a different +/// field for these names. +const MEMBER_NAMES: &[&str] = &["a", "b", "M", "L", "S", "0"]; + +/// The longest path a generated condition names. +const MAX_PATH_LEN: usize = 3; + +/// A generated item, keyed by attribute name. +type Item = BTreeMap; + /// Constrained string alphabet — short, printable, small alphabet. /// Keeps proptest shrinking behavior tractable and matches the kinds of /// values real DDB workloads use for status flags, tags, ETags, etc. @@ -57,12 +70,10 @@ fn arb_short_bytes() -> impl Strategy> { proptest::collection::vec(any::(), 0..4) } -/// Attribute values in the "safe subset" — everything except N/NS. -/// The pushable expression grammar never references numeric operands, so -/// items don't need to contain them either. Including L/M would require a -/// recursive strategy; we keep depth flat for now (paths in the grammar -/// are single-name only, so nested M/L values wouldn't be reachable -/// anyway). +/// Flat attribute values in the "safe subset": everything except N/NS, and +/// lists of strings only. The pushable expression grammar never references +/// numeric operands, so items don't need to contain them either. +/// `arb_value` nests these values in maps and lists. fn arb_safe_value() -> impl Strategy { prop_oneof![ arb_short_str().prop_map(AttributeValue::S), @@ -71,19 +82,49 @@ fn arb_safe_value() -> impl Strategy { Just(AttributeValue::Null), proptest::collection::btree_set(arb_short_str(), 0..3).prop_map(AttributeValue::SS), proptest::collection::btree_set(arb_short_bytes(), 0..3).prop_map(AttributeValue::BS), - // Lists of strings — the compiler's contains-on-list path takes a - // scalar and matches any element. Mixed-type lists aren't in the - // pushable grammar so we don't generate them. + // Lists of strings: the compiler's contains-on-list path takes a + // scalar and matches any element. The lists `arb_value` builds may + // mix types. proptest::collection::vec(arb_short_str().prop_map(AttributeValue::S), 0..3) .prop_map(AttributeValue::L), ] } +/// A map of up to three members named from `MEMBER_NAMES`, or a list of up +/// to three elements, holding values drawn from `inner`. +fn arb_map_or_list(inner: BoxedStrategy) -> impl Strategy { + prop_oneof![ + proptest::collection::btree_map( + select(MEMBER_NAMES).prop_map(String::from), + inner.clone(), + 0..4, + ) + .prop_map(AttributeValue::M), + proptest::collection::vec(inner, 0..4).prop_map(AttributeValue::L), + ] +} + +/// An attribute value: half of the time a flat value, otherwise a map or +/// list whose members and elements are flat values or maps and lists of +/// flat values. The lists in these values may mix types. +fn arb_value() -> impl Strategy { + let flat = arb_safe_value().boxed(); + let member = prop_oneof![ + 1 => flat.clone(), + 1 => arb_map_or_list(flat.clone()), + ] + .boxed(); + prop_oneof![ + 1 => flat, + 1 => arb_map_or_list(member), + ] +} + /// A random DDB Item. Not every name is present in every item — that's /// how the "path missing" edge cases get exercised. fn arb_item() -> impl Strategy> { proptest::collection::vec( - (select(NAMES).prop_map(String::from), arb_safe_value()), + (select(NAMES).prop_map(String::from), arb_value()), 0..NAMES.len(), ) .prop_map(|pairs| { @@ -99,9 +140,24 @@ fn arb_item() -> impl Strategy> { // Expression strategies // --------------------------------------------------------------------------- -/// A random attribute name from the vocabulary. +/// A random path: a name from `NAMES`, then up to two elements, each a +/// member name from `MEMBER_NAMES` or a list index below 3. Most of these +/// paths are absent from a generated item; `arb_item_and_expr` swaps three +/// in four of them for paths the item has. fn arb_name_expr() -> impl Strategy { - select(NAMES).prop_map(|n| Expr::Path(vec![PathElement::Attribute(n.to_string())])) + let element = prop_oneof![ + select(MEMBER_NAMES).prop_map(|n| PathElement::Attribute(n.to_string())), + (0..3usize).prop_map(PathElement::Index), + ]; + ( + select(NAMES), + proptest::collection::vec(element, 0..MAX_PATH_LEN), + ) + .prop_map(|(first, rest)| { + let mut path = vec![PathElement::Attribute(first.to_string())]; + path.extend(rest); + Expr::Path(path) + }) } /// A random placeholder reference (`:v0`, `:v1`, ...). @@ -321,6 +377,80 @@ fn arb_expr() -> impl Strategy)> { ] } +/// An item and a condition on it. Three in four paths of the condition, by +/// their picks, are replaced by paths the item has, so a nested member or +/// element the condition names is present often enough for a wrong compiled +/// path to give a different answer; the other paths stay random. +fn arb_item_and_expr() -> impl Strategy))> { + (arb_item(), arb_expr(), any::<[u8; 2]>()).prop_map(|(item, (mut expr, values), picks)| { + steer_paths(&mut expr, &item_paths(&item), &mut picks.into_iter()); + (item, (expr, values)) + }) +} + +/// Every path into `item` of at most `MAX_PATH_LEN` elements: each +/// top-level name, and below it each member of a map and each element of a +/// list. A path is listed once per element, so a deeper path, where a +/// misplaced typed segment shows, is picked more often. +fn item_paths(item: &Item) -> Vec> { + fn walk(value: &AttributeValue, path: &mut Vec, out: &mut Vec>) { + out.extend(std::iter::repeat_n(path.clone(), path.len())); + if path.len() == MAX_PATH_LEN { + return; + } + let children: Vec<(PathElement, &AttributeValue)> = match value { + AttributeValue::M(members) => members + .iter() + .map(|(name, v)| (PathElement::Attribute(name.clone()), v)) + .collect(), + AttributeValue::L(elements) => elements + .iter() + .enumerate() + .map(|(idx, v)| (PathElement::Index(idx), v)) + .collect(), + _ => Vec::new(), + }; + for (element, child) in children { + path.push(element); + walk(child, path, out); + path.pop(); + } + } + let mut out = Vec::new(); + for (name, value) in item { + let mut path = vec![PathElement::Attribute(name.clone())]; + walk(value, &mut path, &mut out); + } + out +} + +/// Replace the paths of `expr`, in order: for each pick that is not a +/// multiple of 4, by `paths[pick / 4]` (modulo its length). A path left +/// without a pick is kept. +fn steer_paths(expr: &mut Expr, paths: &[Vec], picks: &mut impl Iterator) { + match expr { + Expr::Path(path) => { + if let Some(pick) = picks.next() + && pick % 4 != 0 + && !paths.is_empty() + { + *path = paths[usize::from(pick / 4) % paths.len()].clone(); + } + } + Expr::Compare { left, right, .. } | Expr::And(left, right) | Expr::Or(left, right) => { + steer_paths(left, paths, picks); + steer_paths(right, paths, picks); + } + Expr::Not(inner) => steer_paths(inner, paths, picks), + Expr::Function { args, .. } => { + for arg in args { + steer_paths(arg, paths, picks); + } + } + _ => {} + } +} + // --------------------------------------------------------------------------- // Item → BSON conversion for the interpreter side // --------------------------------------------------------------------------- @@ -351,8 +481,7 @@ proptest! { /// in-Rust DDB evaluator's pass/fail result. #[test] fn compiled_filter_matches_ddb_evaluator( - item in arb_item(), - expr_pair in arb_expr(), + (item, expr_pair) in arb_item_and_expr(), ) { let (expr, values_vec) = expr_pair; @@ -414,8 +543,7 @@ proptest! { /// short-circuits without a comparison. #[test] fn analyzer_yes_implies_filter_parity( - item in arb_item(), - expr_pair in arb_expr(), + (item, expr_pair) in arb_item_and_expr(), ) { let (expr, values_vec) = expr_pair; let mut values = HashMap::new(); diff --git a/docs/design/13-storage-mongodb.md b/docs/design/13-storage-mongodb.md index 9612c00b9..5498e6cc0 100644 --- a/docs/design/13-storage-mongodb.md +++ b/docs/design/13-storage-mongodb.md @@ -533,11 +533,15 @@ Not pushable: any operand of type `N` (numbers stored as strings; `"10" count), `BETWEEN` / `IN` (pending proptest coverage; the compiler emits them, the analyzer refuses them), and `NOT` around anything else. -Property tests (`tests/pushdown_parity.rs`) generate random items and -expressions, compile the filter, and check that a pure-Rust BSON -interpreter and `evaluate_condition` agree on match/no-match — the -regression harness that lets the analyzer's certification be extended -safely. +Property tests (`tests/pushdown_parity.rs`) generate random items, whose +attributes can hold maps and lists nested two deep with members named +from `a`, `b`, `M`, `L`, `S` and `0`, and conditions on paths of up to +three elements; when the item has an attribute, about three in four of +the paths are replaced by paths the item has (about three in five of all +generated paths, since one item in five is empty). They +compile each condition and check that a pure-Rust BSON interpreter and +`evaluate_condition` agree on match or no match. This is the regression +harness that lets the analyzer's certification be extended safely. ### 5.3 Query and Scan diff --git a/tests/test_condition_nested_paths.py b/tests/test_condition_nested_paths.py new file mode 100644 index 000000000..4cdab9670 --- /dev/null +++ b/tests/test_condition_nested_paths.py @@ -0,0 +1,209 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Conditions on nested map members and list elements, on UpdateItem and DeleteItem. + +The item holds ``m = {k: "inner", n: [10, "b"], mm: {k: "deep"}}`` and +``l = [{k: "first"}, "s"]``. Every expected answer in HOLDS, FAILS and the +DeleteItem tests was measured against Amazon DynamoDB on 2026-10-25 at 20:51 +and 20:52 UTC (us-east-1), each request twice: a holding condition applies the +write and a failing one answers ConditionalCheckFailedException. + +The item also holds ``t``, a map whose members are named ``M``, ``L``, ``S`` +and ``0``, and ``g = ["g0", "g1", ["g20", "g21"]]``, a list inside a list. +The first three member names of ``t`` are type tags, the names of the fields +a stored value sits under, and the last reads as a list index in a MongoDB +path. The rows in DERIVED_HOLDS were first encoded from ExtendDB's in-Rust +condition evaluator and then measured on the service on 2026-10-28 (see the +note under the list); the list keeps its name. + +On MongoDB, a condition on a table with no index and no stream is compiled to +a MongoDB filter while the server's cache records that the table has no index. +That entry is written by a session-path write and lasts 60 seconds; each test +puts its own item immediately before the conditional write, and that put +renews the entry when it has expired. + +Dual-target against Amazon DynamoDB and extenddb. +""" + +from __future__ import annotations + +import re +import uuid + +import pytest +from botocore.exceptions import ClientError + +from conftest import scoped_table + +NAMES = { + "#m": "m", + "#k": "k", + "#n": "n", + "#mm": "mm", + "#l": "l", + "#z": "z", + "#u": "u", + "#t": "t", + "#M": "M", + "#L": "L", + "#S": "S", + "#zero": "0", + "#g": "g", +} +VALUES = { + ":x": {"S": "z"}, + ":inner": {"S": "inner"}, + ":deep": {"S": "deep"}, + ":first": {"S": "first"}, + ":b": {"S": "b"}, + ":in": {"S": "in"}, + ":ner": {"S": "ner"}, + ":S": {"S": "S"}, + ":M": {"S": "M"}, + ":other": {"S": "other"}, + ":s": {"S": "s"}, + ":tm": {"S": "tm"}, + ":tl": {"S": "tl"}, + ":ts": {"S": "ts"}, + ":t0": {"S": "t0"}, + ":g21": {"S": "g21"}, +} + +ITEM = { + "v": {"S": "x"}, + "m": { + "M": { + "k": {"S": "inner"}, + "n": {"L": [{"N": "10"}, {"S": "b"}]}, + "mm": {"M": {"k": {"S": "deep"}}}, + } + }, + "l": {"L": [{"M": {"k": {"S": "first"}}}, {"S": "s"}]}, + "t": { + "M": { + "M": {"S": "tm"}, + "L": {"S": "tl"}, + "S": {"S": "ts"}, + "0": {"S": "t0"}, + } + }, + "g": {"L": [{"S": "g0"}, {"S": "g1"}, {"L": [{"S": "g20"}, {"S": "g21"}]}]}, +} + +HOLDS = [ + "attribute_exists(#m.#k)", + "attribute_not_exists(#m.#z)", + "#m.#k = :inner", + "begins_with(#m.#k, :in)", + "contains(#m.#k, :ner)", + "attribute_type(#m.#k, :S)", + "attribute_type(#m.#mm, :M)", + "#m.#mm.#k = :deep", + "#l[0].#k = :first", + "#m.#n[1] = :b", + "#l[1] = :s", + "attribute_exists(#m.#k) AND #m.#mm.#k = :deep AND #l[0].#k = :first", + "NOT attribute_exists(#m.#mm.#z)", +] + +FAILS = [ + "attribute_exists(#m.#z)", + "attribute_not_exists(#m.#k)", + "#m.#k <> :inner", + "attribute_exists(#l.#k)", + "attribute_exists(#m[0])", + "#m.#k = :other OR #m.#mm.#k = :other OR #l[0].#k = :other", +] + +DERIVED_HOLDS = [ + "#t.#M = :tm", + "#t.#L = :tl", + "#t.#S = :ts", + "#t.#zero = :t0", + "#g[2][1] = :g21", +] +# Measured on Amazon DynamoDB on 2026-10-28 with this item, these names and +# values, two samples each: every row applied the update; with :other in place +# of the value, #t.#M and #g[2][1] answered ConditionalCheckFailedException and +# left the item unchanged. The list keeps its name for the parametrize ids. + + +def _maps(*exprs: str) -> dict: + tokens = set(re.findall(r"[#:][A-Za-z0-9_]+", " ".join(exprs))) + out = {"ExpressionAttributeNames": {k: v for k, v in NAMES.items() if k in tokens}} + values = {k: v for k, v in VALUES.items() if k in tokens} + if values: + out["ExpressionAttributeValues"] = values + return out + + +@pytest.fixture(scope="module") +def table(dynamodb_client): + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", "AttributeType": "S"}, + {"AttributeName": "sk", "AttributeType": "S"}, + ], + key_schema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + ) as name: + yield name + + +@pytest.fixture +def key(dynamodb_client, table): + key = {"pk": {"S": f"nested-{uuid.uuid4().hex[:8]}"}, "sk": {"S": "item"}} + dynamodb_client.put_item(TableName=table, Item={**key, **ITEM}) + return key + + +def _get(client, table, key): + return client.get_item(TableName=table, Key=key, ConsistentRead=True).get("Item") + + +def _update(client, table, key, condition): + return client.update_item( + TableName=table, + Key=key, + UpdateExpression="SET #u = :x", + ConditionExpression=condition, + **_maps("SET #u = :x", condition), + ) + + +@pytest.mark.parametrize("condition", HOLDS + DERIVED_HOLDS) +def test_update_with_holding_nested_condition_applies(dynamodb_client, table, key, condition): + _update(dynamodb_client, table, key, condition) + assert _get(dynamodb_client, table, key)["u"] == {"S": "z"} + + +@pytest.mark.parametrize("condition", FAILS) +def test_update_with_failing_nested_condition_is_refused(dynamodb_client, table, key, condition): + with pytest.raises(ClientError) as exc_info: + _update(dynamodb_client, table, key, condition) + err = exc_info.value.response["Error"] + assert err["Code"] == "ConditionalCheckFailedException", err + assert "u" not in _get(dynamodb_client, table, key) + + +def test_delete_with_holding_nested_condition_applies(dynamodb_client, table, key): + condition = "#m.#mm.#k = :deep AND #l[0].#k = :first" + dynamodb_client.delete_item( + TableName=table, Key=key, ConditionExpression=condition, **_maps(condition) + ) + assert _get(dynamodb_client, table, key) is None + + +def test_delete_with_failing_nested_condition_is_refused(dynamodb_client, table, key): + condition = "#m.#mm.#k = :other" + with pytest.raises(ClientError) as exc_info: + dynamodb_client.delete_item( + TableName=table, Key=key, ConditionExpression=condition, **_maps(condition) + ) + err = exc_info.value.response["Error"] + assert err["Code"] == "ConditionalCheckFailedException", err + assert _get(dynamodb_client, table, key) is not None