diff --git a/crates/core/src/expression/key_condition.rs b/crates/core/src/expression/key_condition.rs index edaa1f8e5..a7c833ff4 100755 --- a/crates/core/src/expression/key_condition.rs +++ b/crates/core/src/expression/key_condition.rs @@ -21,6 +21,9 @@ use crate::types::{ KeySchemaElement, KeyType, hash_key_elements, is_multipart_key_schema, range_key_elements, }; +mod no_hash_equality; +mod references; + /// Parsed key condition: partition key value expression(s) and optional sort key condition. /// /// For standard (single-attribute) keys, `pk_path`/`pk_value` hold the single PK condition @@ -30,7 +33,10 @@ use crate::types::{ /// `extra_pk_conditions` holds additional PK equality conditions. #[derive(Debug, Clone)] pub struct KeyCondition { - /// First partition key attribute path (resolved name). + /// First partition key attribute path (resolved name). Empty when the + /// expression held one or two clauses and none is an equality; they are + /// then `sk_condition` and `extra_sk_conditions`, and + /// [`KeyCondition::refuse_without_hash_equality`] refuses them. pub pk_path: Vec, /// First partition key value expression (placeholder or literal). pub pk_value: Expr, @@ -681,9 +687,9 @@ fn classify_single(clause: RawClause) -> Result { sk_condition: None, extra_sk_conditions: Vec::new(), }), - _ => Err(validation_err( - "Invalid KeyConditionExpression: partition key condition must use equality (=)", - )), + // The wording of the refusal depends on the key schema, which is not + // known here. See `KeyCondition::refuse_without_hash_equality`. + other => Ok(KeyCondition::without_equality(raw_to_sk(other), None)), } } @@ -691,7 +697,7 @@ fn classify_pair(a: RawClause, b: RawClause) -> Result { Ok(KeyCondition { @@ -702,8 +708,11 @@ fn classify_pair(a: RawClause, b: RawClause) -> Result Err(validation_err( - "Invalid KeyConditionExpression: one condition must be an equality on the partition key", + // Neither clause is an equality. As for a single clause, the key + // schema decides the wording. See `KeyCondition::refuse_without_hash_equality`. + (a, b) => Ok(KeyCondition::without_equality( + raw_to_sk(a), + Some(raw_to_sk(b)), )), } } diff --git a/crates/core/src/expression/key_condition/no_hash_equality.rs b/crates/core/src/expression/key_condition/no_hash_equality.rs new file mode 100644 index 000000000..161284614 --- /dev/null +++ b/crates/core/src/expression/key_condition/no_hash_equality.rs @@ -0,0 +1,377 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! A `KeyConditionExpression` with no equality clause on the HASH attribute. +//! +//! Which message the service returns for `pk > :x` depends on the key schema +//! of the table or index being queried, and the parser does not know it. The +//! parser therefore accepts the clauses and records them in a +//! [`KeyCondition`] without a partition key condition (`pk_path` is empty). +//! The engine calls [`KeyCondition::refuse_without_hash_equality`] once the +//! key schema is known; that call is the only place the wording is chosen. +//! Every such condition is refused, so no storage backend receives one. +//! +//! Two shapes reach the refusal: +//! +//! - one or two clauses, none an equality (`pk > :x`, `sk > :x AND zz > :y`). +//! `sk_condition` is the first clause, or the second when only the second +//! is a `begins_with` (`sk > :x AND begins_with(pk, :n)`), so that the +//! engine sees the `begins_with`. The other clause is kept as its operands +//! in `extra_sk_conditions`, one `(path, operand)` entry per operand (two +//! for `BETWEEN`), so that the references of both clauses are counted as +//! used. +//! - an equality on another attribute plus a non-equality clause on the HASH +//! attribute of a single-HASH schema (`sk = :x AND pk > :y`). The parser +//! classifies the pair as it does any other, equality first, and +//! [`KeyCondition::lacks_hash_equality`] spots it once the schema is known. +//! +//! The wordings, measured on the service on 2026-10-21 (table with HASH `pk` +//! and RANGE `sk`, index with HASH `g` and RANGE `s`, multi-attribute index +//! with HASH `h` and RANGE `a`, `b`, `c`, `d`) and on 2026-10-23 (every other +//! row): +//! +//! | clauses | key schema | message | +//! |---|---|---| +//! | a non-equality clause names the HASH attribute (`pk > :x`, `begins_with(pk, :x)`, `pk BETWEEN :x AND :y`, `sk = :x AND pk > :y`, `pk > :x AND zz > :y`) | one HASH attribute | `Query key condition not supported` | +//! | no clause names the HASH attribute (`sk > :x`, `z > :x`, `sk > :x AND zz > :y`) | one HASH attribute | `Query condition missed key schema element: pk` | +//! | any clauses (`h > :h`, `a > :x`, `z > :x`, `h1 > :x`, `r > :x`) | several key attributes | `HASH key attributes h must have equality conditions specified in the query` (every HASH attribute, comma separated) | +//! | two non-equality clauses on one attribute (`pk > :x AND pk < :y`) | any | `Invalid KeyConditionExpression: KeyConditionExpressions must only contain one condition per key` | +//! +//! The service reports an undefined `#name` or `:value` first, with the +//! `Invalid KeyConditionExpression: ` prefix; see +//! [`KeyCondition::validate_references`]. It reports an unused value, an +//! invalid `FilterExpression` or `ProjectionExpression` and an invalid +//! `Select` ahead of the wording, and an `ExclusiveStartKey` or a key value +//! size after it, so the engine calls the refusal between those. On a schema +//! with one HASH attribute the engine also type-checks the clause on the HASH +//! attribute, after [`KeyCondition::refuse_two_conditions_on_one_attribute`] +//! and before the wording; see [`KeyCondition::operands_naming`]. + +use std::collections::HashMap; + +use super::{CompareOp, Expr, KeyCondition, PathElement, SortKeyCondition}; +use crate::error::DynamoDbError; +use crate::types::{KeySchemaElement, hash_key_elements, is_multipart_key_schema}; + +#[cfg(test)] +mod tests; + +const NOT_SUPPORTED: &str = "Query key condition not supported"; +const ONE_CONDITION_PER_KEY: &str = "Invalid KeyConditionExpression: KeyConditionExpressions \ + must only contain one condition per key"; + +impl KeyCondition { + /// The condition for one or two clauses, none of them an equality. + /// + /// `pk_path` is empty because no partition key condition was given. + /// `pk_value` repeats the first clause's first operand so the field holds + /// a well-formed expression; it carries no meaning while `pk_path` is + /// empty. + /// + /// `sk_condition` keeps one clause whole: the first, or the second when + /// only the second is a `begins_with`, because the engine reads the + /// operator of a `begins_with` there. The other clause is kept as its + /// operands in `extra_sk_conditions`. + pub(super) fn without_equality( + first: SortKeyCondition, + second: Option, + ) -> Self { + let pk_value = first_operand(&first).clone(); + let is_begins_with = + |clause: &SortKeyCondition| matches!(clause, SortKeyCondition::BeginsWith { .. }); + let (whole, other) = match second { + Some(second) if is_begins_with(&second) && !is_begins_with(&first) => { + (second, Some(first)) + } + second => (first, second), + }; + let extra_sk_conditions = other.map_or_else(Vec::new, |clause| operands(&clause)); + Self { + pk_path: Vec::new(), + pk_value, + extra_pk_conditions: Vec::new(), + sk_condition: Some(whole), + extra_sk_conditions, + } + } + + /// Whether the key schema has no equality clause on its HASH attribute + /// in this condition, in a shape [`Self::refuse_without_hash_equality`] + /// refuses. + /// + /// True for a condition with no equality clause at all. On a schema with + /// one HASH attribute it is also true for two clauses where the equality + /// names another attribute and the other clause names the HASH attribute + /// with a non-equality (`sk = :x AND pk > :y`). An equality on the HASH + /// attribute makes it false, so `pk = :x AND pk > :y` keeps its existing + /// handling. + #[must_use] + pub fn lacks_hash_equality( + &self, + key_schema: &[KeySchemaElement], + names: &HashMap, + ) -> bool { + if self.pk_path.is_empty() { + return self.sk_condition.is_some(); + } + if is_multipart_key_schema(key_schema) + || !self.extra_pk_conditions.is_empty() + || !self.extra_sk_conditions.is_empty() + { + return false; + } + let Some(hash_attr) = single_hash_attr(key_schema) else { + return false; + }; + let names_hash = |path: &[PathElement]| attr_of(path, names).as_deref() == Some(hash_attr); + let Some(sk) = &self.sk_condition else { + return false; + }; + let sk_is_equality = matches!( + sk, + SortKeyCondition::Compare { + op: CompareOp::Eq, + .. + } + ); + // `pk_path` holds an equality whenever it is set. A second equality + // on the HASH attribute (`sk = :x AND pk = :y`) also satisfies it. + if names_hash(&self.pk_path) { + return false; + } + !sk_is_equality && names_hash(clause_path(sk)) + } + + /// Refuse a key condition that has no equality clause on the HASH + /// attribute, choosing the message by the key schema being queried. + /// + /// Returns `Ok(())` for every other key condition. Call this after the + /// expression validations (unused names and values, `FilterExpression`, + /// `ProjectionExpression`, `Select`) and the references check, and before + /// the storage read. + /// + /// - Two clauses on one attribute: `Invalid KeyConditionExpression: + /// KeyConditionExpressions must only contain one condition per key`. + /// - A schema with several HASH or several RANGE attributes: the service + /// names every HASH attribute, because none has an equality condition. + /// Measured on 2026-10-23 on an index with HASH `h` and RANGE `a`, `b`, + /// `c`, `d` (`h > :x`, `a > :x`, `d > :x`, `z > :x`, `begins_with(a, :x)`, + /// `a BETWEEN :x AND :y`, and `h > :n` with an N value) and on an index + /// with HASH `h1`, `h2` and RANGE `r` (`h1`, `h2`, `r`, `z` and a + /// `#name` for `h1`). + /// - A schema with one HASH attribute and at most one RANGE attribute: if + /// a clause names the HASH attribute (`<`, `<=`, `>`, `>=`, + /// `begins_with`, `BETWEEN`), `Query key condition not supported`; + /// otherwise `Query condition missed key schema element: `. Measured on a table, a hash-only table, a GSI and an + /// LSI. + /// + /// # Errors + /// + /// Returns `ValidationException` for the conditions above, and when a + /// `#name` on a clause is not defined in `names`. + pub fn refuse_without_hash_equality( + &self, + key_schema: &[KeySchemaElement], + names: &HashMap, + ) -> Result<(), DynamoDbError> { + if !self.lacks_hash_equality(key_schema, names) { + return Ok(()); + } + self.refuse_two_conditions_on_one_attribute(names)?; + let attrs = self.clause_attrs(names)?; + + let hash_attrs: Vec<&str> = hash_key_elements(key_schema) + .iter() + .map(|ks| ks.attribute_name.as_str()) + .collect(); + + if is_multipart_key_schema(key_schema) { + return Err(DynamoDbError::ValidationException(format!( + "HASH key attributes {} must have equality conditions specified in the query", + hash_attrs.join(", ") + ))); + } + + let pk_attr = hash_attrs.first().copied().unwrap_or_default(); + let message = if attrs.iter().any(|attr| attr == pk_attr) { + NOT_SUPPORTED.to_owned() + } else { + format!("Query condition missed key schema element: {pk_attr}") + }; + Err(DynamoDbError::ValidationException(message)) + } + + /// Refuse two clauses on one attribute in a condition with no equality + /// clause (`pk > :x AND pk < :y`), with `Invalid KeyConditionExpression: + /// KeyConditionExpressions must only contain one condition per key`. + /// + /// The service reports this ahead of a value whose type differs from the + /// attribute definition (measured 2026-11-01: `pk > :x AND pk < :n` and + /// `pk > :n AND pk < :m` with N values on an S key), so the engine calls + /// it before its type check. Returns `Ok(())` for a condition with an + /// equality clause. + /// + /// # Errors + /// + /// Returns `ValidationException` for two clauses on one attribute, and + /// when a `#name` on a clause is not defined in `names`. + pub fn refuse_two_conditions_on_one_attribute( + &self, + names: &HashMap, + ) -> Result<(), DynamoDbError> { + if !self.pk_path.is_empty() { + return Ok(()); + } + let attrs = self.clause_attrs(names)?; + if attrs.len() == 2 && attrs[0] == attrs[1] { + return Err(DynamoDbError::ValidationException( + ONE_CONDITION_PER_KEY.to_owned(), + )); + } + Ok(()) + } + + /// The value operands of every clause that names `attribute`, with + /// `#name` resolved through `names`: one for a comparison or + /// `begins_with`, two for `BETWEEN`. A clause whose `#name` is not + /// defined names no attribute. + /// + /// The engine type-checks the operands of the clause on the HASH + /// attribute with this, and no other operand (measured 2026-11-01: + /// `pk > :x AND sk > :n` with an N value on an S sort key answers `Query + /// key condition not supported`). + #[must_use] + pub fn operands_naming<'a>( + &'a self, + attribute: &str, + names: &HashMap, + ) -> Vec<(&'a [PathElement], &'a Expr)> { + let names_it = |path: &[PathElement]| attr_of(path, names).as_deref() == Some(attribute); + let mut found: Vec<(&[PathElement], &Expr)> = Vec::new(); + if names_it(&self.pk_path) { + found.push((&self.pk_path, &self.pk_value)); + } + for (path, value) in self + .extra_pk_conditions + .iter() + .chain(&self.extra_sk_conditions) + { + if names_it(path) { + found.push((path, value)); + } + } + match &self.sk_condition { + Some(SortKeyCondition::Compare { path, value, .. }) if names_it(path) => { + found.push((path, value)); + } + Some(SortKeyCondition::Between { path, low, high }) if names_it(path) => { + found.push((path, low)); + found.push((path, high)); + } + Some(SortKeyCondition::BeginsWith { path, prefix }) if names_it(path) => { + found.push((path, prefix)); + } + _ => {} + } + found + } + + /// The top-level attribute of each clause, in the order of + /// [`Self::clause_paths`], with `#name` resolved. + fn clause_attrs(&self, names: &HashMap) -> Result, DynamoDbError> { + self.clause_paths() + .into_iter() + .map(|path| resolve_clause_attr(path, names)) + .collect() + } + + /// The path of each clause, in classification order: the equality, the + /// `sk_condition` and, for two clauses with no equality, the other one. + fn clause_paths(&self) -> Vec<&[PathElement]> { + let mut paths: Vec<&[PathElement]> = Vec::new(); + if !self.pk_path.is_empty() { + paths.push(&self.pk_path); + } + if let Some(sk) = &self.sk_condition { + paths.push(clause_path(sk)); + } + if let Some((path, _)) = self.extra_sk_conditions.first() { + paths.push(path); + } + paths + } +} + +/// The only HASH attribute of a schema, or `None` for zero or several. +fn single_hash_attr(key_schema: &[KeySchemaElement]) -> Option<&str> { + match hash_key_elements(key_schema).as_slice() { + [only] => Some(only.attribute_name.as_str()), + _ => None, + } +} + +/// The top-level attribute a path names, with `#name` resolved; `None` when +/// the name is not defined or the path is not an attribute. +fn attr_of(path: &[PathElement], names: &HashMap) -> Option { + match path.first()? { + PathElement::Attribute(name) => match name.strip_prefix('#') { + Some(ref_name) => names.get(ref_name).cloned(), + None => Some(name.clone()), + }, + PathElement::Index(_) => None, + } +} + +/// The top-level attribute a clause names, with `#name` resolved. +/// +/// An undefined `#name` is refused with the `KeyConditionExpression` form of +/// the service's sentence. +fn resolve_clause_attr( + path: &[PathElement], + names: &HashMap, +) -> Result { + match path.first() { + Some(PathElement::Attribute(name)) => match name.strip_prefix('#') { + Some(ref_name) => names.get(ref_name).cloned().ok_or_else(|| { + DynamoDbError::ValidationException(format!( + "Invalid KeyConditionExpression: An expression attribute name used in \ + the document path is not defined; attribute name: #{ref_name}" + )) + }), + None => Ok(name.clone()), + }, + _ => Err(DynamoDbError::ValidationException( + "Invalid KeyConditionExpression: Invalid key condition path".to_owned(), + )), + } +} + +/// The attribute path a clause applies to. +fn clause_path(clause: &SortKeyCondition) -> &[PathElement] { + match clause { + SortKeyCondition::Compare { path, .. } + | SortKeyCondition::Between { path, .. } + | SortKeyCondition::BeginsWith { path, .. } => path, + } +} + +/// The first value operand of a clause. +fn first_operand(clause: &SortKeyCondition) -> &Expr { + match clause { + SortKeyCondition::Compare { value, .. } => value, + SortKeyCondition::Between { low, .. } => low, + SortKeyCondition::BeginsWith { prefix, .. } => prefix, + } +} + +/// Every value operand of a clause as a `(path, operand)` entry. +fn operands(clause: &SortKeyCondition) -> Vec<(Vec, Expr)> { + match clause { + SortKeyCondition::Compare { path, value, .. } => vec![(path.clone(), value.clone())], + SortKeyCondition::Between { path, low, high } => { + vec![(path.clone(), low.clone()), (path.clone(), high.clone())] + } + SortKeyCondition::BeginsWith { path, prefix } => vec![(path.clone(), prefix.clone())], + } +} diff --git a/crates/core/src/expression/key_condition/no_hash_equality/tests.rs b/crates/core/src/expression/key_condition/no_hash_equality/tests.rs new file mode 100644 index 000000000..7434dea37 --- /dev/null +++ b/crates/core/src/expression/key_condition/no_hash_equality/tests.rs @@ -0,0 +1,463 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Key conditions with no equality clause on the HASH attribute. Every +//! expected message is a row measured on the service: the issue table on +//! 2026-10-21, everything else on 2026-10-23. + +use std::collections::HashMap; + +use super::*; +use crate::expression::{collect_key_condition_refs, parse_key_condition, tokenize}; +use crate::types::KeyType; + +const WANT_ONE_PER_KEY: &str = "Invalid KeyConditionExpression: KeyConditionExpressions must \ + only contain one condition per key"; +const MISSED_PK: &str = "Query condition missed key schema element: pk"; +const MISSED_G: &str = "Query condition missed key schema element: g"; + +fn element(name: &str, key_type: KeyType) -> KeySchemaElement { + KeySchemaElement { + attribute_name: name.to_owned(), + key_type, + } +} + +/// HASH `pk`, RANGE `sk`: the base table of the issue. +fn table_schema() -> Vec { + vec![element("pk", KeyType::Hash), element("sk", KeyType::Range)] +} + +/// HASH `pk`, RANGE `ls`: a local secondary index on the table above. +fn lsi_schema() -> Vec { + vec![element("pk", KeyType::Hash), element("ls", KeyType::Range)] +} + +/// HASH `g`, RANGE `s`: the single-attribute index of the issue. +fn gsi_schema() -> Vec { + vec![element("g", KeyType::Hash), element("s", KeyType::Range)] +} + +/// HASH `h`, RANGE `a`, `b`, `c`, `d`: the multi-attribute index of the issue. +fn multi_range_schema() -> Vec { + vec![ + element("h", KeyType::Hash), + element("a", KeyType::Range), + element("b", KeyType::Range), + element("c", KeyType::Range), + element("d", KeyType::Range), + ] +} + +/// HASH `h1`, `h2`, RANGE `r`. +fn multi_hash_schema() -> Vec { + vec![ + element("h1", KeyType::Hash), + element("h2", KeyType::Hash), + element("r", KeyType::Range), + ] +} + +fn parse(expr: &str) -> KeyCondition { + let tokens = tokenize(expr).unwrap(); + parse_key_condition(&tokens).unwrap_or_else(|e| panic!("{expr}: {e:?}")) +} + +fn refusal(expr: &str, schema: &[KeySchemaElement]) -> String { + refusal_with_names(expr, schema, &HashMap::new()) +} + +fn refusal_with_names( + expr: &str, + schema: &[KeySchemaElement], + names: &HashMap, +) -> String { + match parse(expr).refuse_without_hash_equality(schema, names) { + Err(DynamoDbError::ValidationException(msg)) => msg, + other => panic!("{expr}: expected a refusal, got {other:?}"), + } +} + +fn assert_all(rows: &[(&str, &str)], schema: &[KeySchemaElement]) { + for (expr, want) in rows { + assert_eq!(refusal(expr, schema), *want, "{expr}"); + } +} + +#[test] +fn clauses_without_an_equality_parse_without_a_partition_key_condition() { + for expr in [ + "pk > :x", + "pk >= :x", + "pk < :x", + "pk <= :x", + ":x < pk", + "begins_with(pk, :x)", + "pk BETWEEN :a AND :b", + "(pk > :x)", + "pk > :x AND sk > :y", + "begins_with(pk, :x) AND sk BETWEEN :a AND :b", + ] { + let kc = parse(expr); + assert!(kc.pk_path.is_empty(), "{expr}"); + assert!(kc.extra_pk_conditions.is_empty(), "{expr}"); + assert!(kc.sk_condition.is_some(), "{expr}"); + } +} + +#[test] +fn the_operands_of_both_clauses_count_as_used_references() { + let kc = parse("#a > :x AND zz BETWEEN :lo AND :hi"); + let (names, values) = collect_key_condition_refs(&kc); + assert!(names.contains("a")); + for value in ["x", "lo", "hi"] { + assert!(values.contains(value), "{value}"); + } +} + +#[test] +fn key_conditions_with_an_equality_on_the_hash_attribute_are_not_refused() { + let schema = table_schema(); + for expr in [ + "pk = :p", + "pk = :p AND sk > :s", + "sk > :s AND pk = :p", + "sk = :s AND pk = :p", + "pk = :p AND begins_with(sk, :s)", + "pk = :p AND sk BETWEEN :a AND :b", + // Two conditions on one key: the service refuses these; ExtendDB + // does not yet and this change leaves them alone. + "pk = :p AND pk > :x", + "pk > :x AND pk = :p", + // Three clauses are outside this rule. + "a = :a AND b = :b AND pk > :x", + ] { + let kc = parse(expr); + assert!( + !kc.lacks_hash_equality(&schema, &HashMap::new()), + "lacks: {expr}" + ); + assert!( + kc.refuse_without_hash_equality(&schema, &HashMap::new()) + .is_ok(), + "{expr}" + ); + } +} + +#[test] +fn a_single_equality_on_another_attribute_is_left_to_the_partition_key_check() { + // `sk = :x` and `zz = :x` carry an equality, so they keep the engine's + // existing `missed key schema element` check. + let schema = table_schema(); + for expr in [ + "sk = :x", + "zz = :x", + "zz = :x AND sk > :y", + "zz = :x AND yy = :y", + ] { + assert!( + !parse(expr).lacks_hash_equality(&schema, &HashMap::new()), + "{expr}" + ); + } +} + +#[test] +fn single_clause_on_the_partition_key_is_not_supported() { + assert_all( + &[ + ("pk > :x", NOT_SUPPORTED), + ("pk >= :x", NOT_SUPPORTED), + ("pk < :x", NOT_SUPPORTED), + ("pk <= :x", NOT_SUPPORTED), + (":x < pk", NOT_SUPPORTED), + (":x >= pk", NOT_SUPPORTED), + ("begins_with(pk, :x)", NOT_SUPPORTED), + ("pk BETWEEN :x AND :y", NOT_SUPPORTED), + ("(pk > :x)", NOT_SUPPORTED), + ], + &table_schema(), + ); +} + +#[test] +fn single_clause_on_an_index_or_a_hash_only_table_is_not_supported() { + assert_all( + &[ + ("g > :x", NOT_SUPPORTED), + ("begins_with(g, :x)", NOT_SUPPORTED), + ("g BETWEEN :x AND :y", NOT_SUPPORTED), + ], + &gsi_schema(), + ); + assert_all( + &[ + ("pk > :x", NOT_SUPPORTED), + ("begins_with(pk, :x)", NOT_SUPPORTED), + ], + &[element("pk", KeyType::Hash)], + ); + // An LSI shares the table's partition key (rows 427-5 and 427-5b). + assert_eq!(refusal("pk > :x", &lsi_schema()), NOT_SUPPORTED); + assert_eq!(refusal("ls > :x", &lsi_schema()), MISSED_PK); +} + +#[test] +fn single_clause_on_a_sort_key_or_a_non_key_attribute_misses_the_partition_key() { + assert_all( + &[ + ("sk > :x", MISSED_PK), + ("sk <= :x", MISSED_PK), + (":x > sk", MISSED_PK), + ("begins_with(sk, :x)", MISSED_PK), + ("sk BETWEEN :a AND :b", MISSED_PK), + ("z > :x", MISSED_PK), + ], + &table_schema(), + ); + assert_eq!(refusal("s > :x", &gsi_schema()), MISSED_G); +} + +#[test] +fn defined_name_references_are_resolved_before_the_wording_is_chosen() { + let schema = table_schema(); + let mut names = HashMap::new(); + for (reference, attr) in [("p", "pk"), ("s", "sk"), ("z", "zz")] { + names.insert(reference.to_owned(), attr.to_owned()); + } + // Rows 2a to 2d, measured 2026-10-23. + for (expr, want) in [ + ("#p > :x", NOT_SUPPORTED), + ("begins_with(#p, :x)", NOT_SUPPORTED), + ("#s > :x", MISSED_PK), + ("#z > :x", MISSED_PK), + ("begins_with(#s, :x)", MISSED_PK), + ("#s > :x AND #z > :y", MISSED_PK), + ("#z > :x AND #p > :y", NOT_SUPPORTED), + ] { + assert_eq!(refusal_with_names(expr, &schema, &names), want, "{expr}"); + } +} + +#[test] +fn an_undefined_name_reference_is_refused_by_name_on_every_schema() { + // Row 427-7 and `#n > :x AND sk > :y`, measured 2026-10-23. + const WANT: &str = "Invalid KeyConditionExpression: An expression attribute name used in \ + the document path is not defined; attribute name: #nope"; + for schema in [table_schema(), gsi_schema(), multi_range_schema()] { + assert_eq!(refusal("#nope > :x", &schema), WANT); + assert_eq!(refusal("begins_with(#nope, :x)", &schema), WANT); + } + assert_eq!( + refusal("#n > :x AND sk > :y", &table_schema()), + "Invalid KeyConditionExpression: An expression attribute name used in \ + the document path is not defined; attribute name: #n" + ); + let mut names = HashMap::new(); + names.insert("other".to_owned(), "pk".to_owned()); + assert_eq!( + refusal_with_names("#nope > :x", &table_schema(), &names), + WANT + ); +} + +#[test] +fn two_clauses_where_a_non_equality_names_the_partition_key_are_not_supported() { + // Measured 2026-10-23 on a table with HASH `pk`, RANGE `sk`. + assert_all( + &[ + ("pk > :x AND sk > :y", NOT_SUPPORTED), + ("sk > :x AND pk > :y", NOT_SUPPORTED), + ("pk > :x AND zz > :y", NOT_SUPPORTED), + ("zz > :x AND pk > :y", NOT_SUPPORTED), + ("pk >= :x AND sk = :y", NOT_SUPPORTED), + ("sk = :x AND pk > :y", NOT_SUPPORTED), + ("pk > :x AND sk = :y", NOT_SUPPORTED), + ("begins_with(pk, :x) AND sk = :y", NOT_SUPPORTED), + ("pk BETWEEN :x AND :y AND sk = :x", NOT_SUPPORTED), + ("zz = :x AND pk > :y", NOT_SUPPORTED), + ], + &table_schema(), + ); + // GSI with HASH `g` and RANGE `s` (the service rows use RANGE `gs`). + assert_all( + &[ + ("g > :x AND s > :y", NOT_SUPPORTED), + ("g > :x AND s = :y", NOT_SUPPORTED), + ], + &gsi_schema(), + ); +} + +#[test] +fn two_clauses_where_none_names_the_partition_key_miss_it() { + assert_all( + &[ + ("sk > :x AND zz > :y", MISSED_PK), + ("zz > :x AND sk > :y", MISSED_PK), + ], + &table_schema(), + ); + // The same rows on the GSI, plus the two that name the table's key. + assert_all( + &[ + ("s > :x AND zz > :y", MISSED_G), + ("pk > :x AND s > :y", MISSED_G), + ], + &gsi_schema(), + ); +} + +#[test] +fn an_equality_on_a_non_key_attribute_with_a_non_key_clause_is_not_refused_here() { + // `zz = :x AND sk > :y`, `zz = :x AND yy = :y`, `sk = :x AND zz = :y` and + // `pk = :x AND s > :y` on the GSI carry an equality; the engine's + // partition key check answers them (`missed key schema element`). + let table = table_schema(); + for expr in [ + "zz = :x AND sk > :y", + "zz = :x AND yy = :y", + "sk = :x AND zz = :y", + ] { + assert!( + !parse(expr).lacks_hash_equality(&table, &HashMap::new()), + "{expr}" + ); + } + assert!(!parse("pk = :x AND s > :y").lacks_hash_equality(&gsi_schema(), &HashMap::new())); +} + +#[test] +fn two_conditions_on_one_key_are_refused_as_such() { + // Rows `pk > :x AND pk < :y` and `sk > :x AND sk < :y`. + for expr in [ + "pk > :x AND pk < :y", + "sk > :x AND sk < :y", + "zz >= :x AND zz <= :y", + "begins_with(pk, :x) AND pk > :y", + ] { + assert_eq!(refusal(expr, &table_schema()), WANT_ONE_PER_KEY, "{expr}"); + } + let mut names = HashMap::new(); + names.insert("a".to_owned(), "pk".to_owned()); + assert_eq!( + refusal_with_names("#a > :x AND pk < :y", &table_schema(), &names), + WANT_ONE_PER_KEY + ); + // The check does not depend on the schema. + assert_eq!( + refusal("h > :x AND h < :y", &multi_range_schema()), + WANT_ONE_PER_KEY + ); +} + +#[test] +fn multi_attribute_index_names_the_hash_attribute_for_every_clause() { + // Rows `3 multi-idx *` and `4 multi-idx h > :n`, measured 2026-10-23. + let want = "HASH key attributes h must have equality conditions specified in the query"; + for expr in [ + "h > :h", + "begins_with(h, :h)", + "h <= :h", + ":h < h", + "a > :x", + "d > :x", + "z > :x", + "begins_with(a, :x)", + "a BETWEEN :x AND :y", + ] { + assert_eq!(refusal(expr, &multi_range_schema()), want, "{expr}"); + } +} + +#[test] +fn multi_hash_index_names_every_hash_attribute() { + // Rows `3 two-hash *`, measured 2026-10-23. + let want = "HASH key attributes h1, h2 must have equality conditions specified in the query"; + for expr in ["h1 > :x", "h2 > :x", "r > :x", "z > :x"] { + assert_eq!(refusal(expr, &multi_hash_schema()), want, "{expr}"); + } + let mut names = HashMap::new(); + names.insert("a".to_owned(), "h1".to_owned()); + assert_eq!( + refusal_with_names("#a > :x", &multi_hash_schema(), &names), + want + ); +} + +#[test] +fn two_range_attributes_with_one_hash_attribute_count_as_multi_attribute() { + let schema = vec![ + element("h", KeyType::Hash), + element("a", KeyType::Range), + element("b", KeyType::Range), + ]; + assert_eq!( + refusal("h > :h", &schema), + "HASH key attributes h must have equality conditions specified in the query" + ); +} + +#[test] +fn a_multi_attribute_schema_with_an_equality_is_left_to_the_classifier() { + let kc = parse("h = :h AND a > :x"); + assert!(!kc.lacks_hash_equality(&multi_range_schema(), &HashMap::new())); +} + +/// The placeholder names of `operands_naming(attribute)` for `expr`. +fn operand_names(expr: &str, attribute: &str, names: &HashMap) -> Vec { + parse(expr) + .operands_naming(attribute, names) + .into_iter() + .map(|(_, value)| match value { + Expr::Placeholder(name) => name.clone(), + other => panic!("{expr}: {other:?}"), + }) + .collect() +} + +#[test] +fn operands_naming_finds_the_clause_on_an_attribute_wherever_it_is() { + let none = HashMap::new(); + for (expr, want) in [ + ("pk > :n", vec!["n"]), + ("pk > :n AND sk > :y", vec!["n"]), + ("sk > :y AND pk > :n", vec!["n"]), + ("sk = :x AND pk > :n", vec!["n"]), + ("pk BETWEEN :n AND :m", vec!["n", "m"]), + ("sk > :y AND pk BETWEEN :n AND :m", vec!["n", "m"]), + ("begins_with(pk, :n)", vec!["n"]), + ("pk = :n", vec!["n"]), + ("sk > :y", vec![]), + ("sk = :x AND zz > :y", vec![]), + ] { + assert_eq!(operand_names(expr, "pk", &none), want, "{expr}"); + } + let mut names = HashMap::new(); + names.insert("p".to_owned(), "pk".to_owned()); + assert_eq!(operand_names("sk > :y AND #p > :n", "pk", &names), ["n"]); + // An undefined `#name` names no attribute. + assert!(operand_names("#q > :n", "pk", &names).is_empty()); +} + +#[test] +fn two_conditions_on_one_attribute_are_refused_only_without_an_equality() { + let none = HashMap::new(); + for expr in ["pk > :x AND pk < :n", "sk > :x AND sk < :y"] { + match parse(expr).refuse_two_conditions_on_one_attribute(&none) { + Err(DynamoDbError::ValidationException(msg)) => { + assert_eq!(msg, WANT_ONE_PER_KEY, "{expr}"); + } + other => panic!("{expr}: {other:?}"), + } + } + for expr in ["pk > :x AND sk < :y", "pk > :x", "sk = :x AND pk > :y"] { + assert!( + parse(expr) + .refuse_two_conditions_on_one_attribute(&none) + .is_ok(), + "{expr}" + ); + } +} diff --git a/crates/core/src/expression/key_condition/references.rs b/crates/core/src/expression/key_condition/references.rs new file mode 100644 index 000000000..547e3f05e --- /dev/null +++ b/crates/core/src/expression/key_condition/references.rs @@ -0,0 +1,120 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The `#name` and `:value` references of a key condition. + +use super::KeyCondition; +use crate::error::DynamoDbError; +use crate::expression::{ExpressionMaps, collect_key_condition_refs}; + +impl KeyCondition { + /// Refuse a key condition that names a `#name` or `:value` the request + /// does not define. + /// + /// The service reports these ahead of every other key condition error, + /// the key schema wording included, and always with the + /// `Invalid KeyConditionExpression: ` prefix (measured 2026-10-23 for + /// `pk = :x`, `pk > :x`, `sk > :x`, `pk > :x AND sk > :y` and + /// `#n > :x AND sk > :y` with the name or value left out). Names are + /// checked before values; within each kind the references are checked in + /// sorted order, so the answer does not depend on hash order. + /// + /// # Errors + /// + /// Returns `ValidationException` naming the first undefined reference. + pub fn validate_references(&self, maps: &ExpressionMaps) -> Result<(), DynamoDbError> { + let (names, values) = collect_key_condition_refs(self); + let mut names: Vec = names.into_iter().collect(); + names.sort(); + for name in names { + if !maps.names.contains_key(&name) { + return Err(DynamoDbError::ValidationException(format!( + "Invalid KeyConditionExpression: An expression attribute name used in \ + the document path is not defined; attribute name: #{name}" + ))); + } + } + let mut values: Vec = values.into_iter().collect(); + values.sort(); + for value in values { + maps.resolve_value_for(&value, "KeyConditionExpression")?; + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + use crate::expression::{parse_key_condition, tokenize}; + use crate::types::AttributeValue; + + fn maps(names: &[(&str, &str)], values: &[&str]) -> ExpressionMaps { + ExpressionMaps::new( + names + .iter() + .map(|(k, v)| ((*k).to_owned(), (*v).to_owned())) + .collect(), + values + .iter() + .map(|k| ((*k).to_owned(), AttributeValue::S("a".to_owned()))) + .collect::>(), + ) + } + + fn check(expr: &str, maps: &ExpressionMaps) -> Result<(), String> { + let kc = parse_key_condition(&tokenize(expr).unwrap()).unwrap(); + kc.validate_references(maps).map_err(|e| match e { + DynamoDbError::ValidationException(msg) => msg, + other => panic!("{other:?}"), + }) + } + + const NO_VALUE: &str = "Invalid KeyConditionExpression: An expression attribute value used \ + in expression is not defined; attribute value: :x"; + + #[test] + fn an_undefined_value_is_refused_with_the_prefix_for_every_shape() { + // Rows P3, 1, 1b and `pk > :x AND sk > :y` (only `:y` given), + // measured 2026-10-23. + let none = maps(&[], &[]); + for expr in ["pk = :x", "pk > :x", "sk > :x"] { + assert_eq!(check(expr, &none).unwrap_err(), NO_VALUE, "{expr}"); + } + let only_y = maps(&[], &["y"]); + assert_eq!(check("pk > :x AND sk > :y", &only_y).unwrap_err(), NO_VALUE); + assert_eq!(check("pk = :x AND sk > :y", &only_y).unwrap_err(), NO_VALUE); + } + + #[test] + fn an_undefined_name_is_refused_with_the_prefix() { + let values = maps(&[], &["x", "y"]); + assert_eq!( + check("#n > :x AND sk > :y", &values).unwrap_err(), + "Invalid KeyConditionExpression: An expression attribute name used in the \ + document path is not defined; attribute name: #n" + ); + } + + #[test] + fn a_name_is_checked_before_a_value() { + let none = maps(&[], &[]); + let err = check("#n > :x", &none).unwrap_err(); + assert!(err.contains("attribute name: #n"), "{err}"); + } + + #[test] + fn defined_references_pass() { + let defined = maps(&[("p", "pk")], &["x", "y"]); + for expr in [ + "#p = :x", + "#p = :x AND sk > :y", + "pk > :x", + "sk BETWEEN :x AND :y", + ] { + assert!(check(expr, &defined).is_ok(), "{expr}"); + } + } +} diff --git a/crates/engine/src/query.rs b/crates/engine/src/query.rs index 848f5fdbb..965e96c74 100755 --- a/crates/engine/src/query.rs +++ b/crates/engine/src/query.rs @@ -313,25 +313,37 @@ pub async fn handle_query( } else { &query_key_info.attribute_definitions }; - key_condition.resolve_key_schema_with( - &query_key_info.key_schema, - &effective_maps.names, - |path, value| check_key_value_type(path, value, type_definitions, effective_maps), - )?; - refuse_begins_with_on_number( - &key_condition, - &query_key_info.attribute_definitions, - effective_maps, - )?; - // On a single-attribute key schema, validate that the partition key is - // actually referenced in the condition. The classifier has already required - // an equality on every HASH attribute of a multi-attribute one. - if !multipart { - let pk_resolved = resolve_path_attr_name(&key_condition.pk_path, &effective_maps.names); - if pk_resolved.as_deref() != Some(pk_attr.as_str()) { - return Err(DynamoDbError::ValidationException(format!( - "Query condition missed key schema element: {pk_attr}" - ))); + // A key condition with no equality clause, and on a schema with one HASH + // attribute an equality on another attribute paired with a non-equality + // on the HASH attribute (`sk = :x AND pk > :y`), has no partition key + // clause to assign or check. It skips the classification and the value + // checks here and is refused after the expression validations below, + // where the key schema picks the wording (measured 2026-10-23: an unused + // value, an invalid FilterExpression or ProjectionExpression and an + // invalid Select are reported ahead of it, and an unused value and an + // invalid FilterExpression ahead of a wrong-typed value in its clause on + // the HASH attribute). See `refuse_without_hash_equality`. + if !key_condition.lacks_hash_equality(&query_key_info.key_schema, &effective_maps.names) { + key_condition.resolve_key_schema_with( + &query_key_info.key_schema, + &effective_maps.names, + |path, value| check_key_value_type(path, value, type_definitions, effective_maps), + )?; + refuse_begins_with_on_number( + &key_condition, + &query_key_info.attribute_definitions, + effective_maps, + )?; + // On a single-attribute key schema, validate that the partition key is + // referenced in the condition. The classifier has already required an + // equality on every HASH attribute of a multi-attribute one. + if !multipart { + let pk_resolved = resolve_path_attr_name(&key_condition.pk_path, &effective_maps.names); + if pk_resolved.as_deref() != Some(pk_attr.as_str()) { + return Err(DynamoDbError::ValidationException(format!( + "Query condition missed key schema element: {pk_attr}" + ))); + } } } @@ -450,6 +462,21 @@ pub async fn handle_query( extenddb_core::validation::IS_QUERY, )?; + // An undefined #name or :value in the key condition is reported first, + // with the KeyConditionExpression prefix, whatever the key schema says. + // Then a key condition that skipped the classification above is refused, + // the key schema of the table or index queried picking the wording. An + // ExclusiveStartKey or a key value size is checked after it (measured + // 2026-10-23). Legacy KeyConditions is desugared above with every + // reference defined and with an equality clause, or refused there, so it + // passes both checks. + refuse_without_hash_equality( + &key_condition, + &query_key_info.key_schema, + &query_key_info.attribute_definitions, + effective_maps, + )?; + // When Select=ALL_PROJECTED_ATTRIBUTES, capture the index info for post-read filtering. let index_proj = if matches!(input.select, Some(Select::AllProjectedAttributes)) { index_info.as_ref() @@ -670,6 +697,72 @@ fn refuse_begins_with_on_number( Ok(()) } +/// Check the references of a key condition, then refuse one that skipped the +/// classification in `handle_query`: a key condition with no equality clause, +/// and on a schema with one HASH attribute an equality on another attribute +/// paired with a non-equality on the HASH attribute. `handle_query` calls this +/// after the expression validations. +/// +/// The order, measured on 2026-11-01 on a schema with one HASH attribute, +/// two samples each: +/// +/// 1. An undefined `#name` or `:value`. +/// 2. Two conditions on one attribute, whatever their value types: +/// `pk > :x AND pk < :n` answers `Invalid KeyConditionExpression: +/// KeyConditionExpressions must only contain one condition per key`. +/// 3. When a clause names the HASH attribute of the key schema queried, a +/// value in that clause whose type differs from the attribute's +/// definition: `pk > :n AND sk > :y`, `sk = :x AND pk > :n` and +/// `pk BETWEEN :n AND :m` answer `One or more parameter values were +/// invalid: Condition parameter type does not match schema type`. Then +/// `begins_with` on a number HASH attribute, alone or with another clause +/// on either side (measured 2026-10-23 for `begins_with(pk, :n)`, and +/// 2026-11-01 for `begins_with(pk, :n) AND sk > :x` and +/// `sk > :x AND begins_with(pk, :n)`: `Incorrect operand type for +/// operator or function; operator or function: begins_with, operand type: +/// N`). The parser keeps a `begins_with` whole in `sk_condition`, which +/// is the clause read here. When both clauses are `begins_with` it keeps +/// the first, so a `begins_with` on the HASH attribute in the second is +/// not recognised. +/// 4. The key schema's wording. A value on another attribute is not +/// checked: `pk > :x AND sk > :n` answers `Query key condition not +/// supported`, and `sk > :n`, with no clause on the HASH attribute, +/// `Query condition missed key schema element: pk`. +/// +/// On a multi-attribute key schema the HASH wording comes before any value +/// check (measured 2026-10-23: `h > :n` with an N value on an S HASH +/// attribute answers `HASH key attributes h must have equality conditions +/// specified in the query`), so step 3 does not run there. +/// +/// Every other key condition went through the classification in +/// `handle_query` and passes. +fn refuse_without_hash_equality( + key_condition: &extenddb_core::expression::KeyCondition, + key_schema: &[extenddb_core::types::KeySchemaElement], + attribute_definitions: &[extenddb_core::types::AttributeDefinition], + maps: &ExpressionMaps, +) -> Result<(), DynamoDbError> { + key_condition.validate_references(maps)?; + if !key_condition.lacks_hash_equality(key_schema, &maps.names) { + return Ok(()); + } + key_condition.refuse_two_conditions_on_one_attribute(&maps.names)?; + if !extenddb_core::types::is_multipart_key_schema(key_schema) + && let Some(hash) = key_schema.iter().find(|ks| ks.key_type == KeyType::Hash) + { + let hash_definition: Vec = attribute_definitions + .iter() + .filter(|def| def.attribute_name == hash.attribute_name) + .cloned() + .collect(); + for (path, value) in key_condition.operands_naming(&hash.attribute_name, &maps.names) { + check_key_value_type(path, value, &hash_definition, maps)?; + } + refuse_begins_with_on_number(key_condition, &hash_definition, maps)?; + } + key_condition.refuse_without_hash_equality(key_schema, &maps.names) +} + /// Validate that all `#name` references in an expression AST are defined. fn validate_name_refs_in_expr( expr: &extenddb_core::expression::Expr, @@ -771,3 +864,6 @@ mod empty_key_condition_tests { assert!(reject_empty_key_condition_expression(Some("pk = :v")).is_ok()); } } + +#[cfg(test)] +mod no_hash_equality_tests; diff --git a/crates/engine/src/query/no_hash_equality_tests.rs b/crates/engine/src/query/no_hash_equality_tests.rs new file mode 100644 index 000000000..2acaf1016 --- /dev/null +++ b/crates/engine/src/query/no_hash_equality_tests.rs @@ -0,0 +1,215 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The order of the checks `refuse_without_hash_equality` runs on a key +//! condition that skipped the classification in `handle_query`. Every +//! expected refusal was measured on the service: `pk > :x` on 2026-10-21, +//! the value type rules and `begins_with` with a second clause on +//! 2026-11-01, the rest on 2026-10-23. + +use std::collections::HashMap; + +use extenddb_core::error::DynamoDbError; +use extenddb_core::expression::{ExpressionMaps, parse_key_condition, tokenize}; +use extenddb_core::types::{ + AttributeDefinition, AttributeValue, KeySchemaElement, KeyType, ScalarAttributeType, +}; + +use super::refuse_without_hash_equality; + +const TYPE_MISMATCH: &str = "One or more parameter values were invalid: Condition parameter \ + type does not match schema type"; +const BEGINS_WITH_ON_N: &str = "Invalid KeyConditionExpression: Incorrect operand type for \ + operator or function; operator or function: begins_with, \ + operand type: N"; +const ONE_PER_KEY: &str = "Invalid KeyConditionExpression: KeyConditionExpressions must only \ + contain one condition per key"; +const NOT_SUPPORTED: &str = "Query key condition not supported"; +const MISSED_PK: &str = "Query condition missed key schema element: pk"; +const MISSED_G: &str = "Query condition missed key schema element: g"; +const HASH_H: &str = "HASH key attributes h must have equality conditions specified in the query"; + +fn key(name: &str, key_type: KeyType) -> KeySchemaElement { + KeySchemaElement { + attribute_name: name.to_owned(), + key_type, + } +} + +fn definition(name: &str, attribute_type: ScalarAttributeType) -> AttributeDefinition { + AttributeDefinition { + attribute_name: name.to_owned(), + attribute_type, + } +} + +/// Run the checks on `expr`, with `:x` and `:y` strings and `:n` and `:m` +/// numbers. +fn check( + expr: &str, + key_schema: &[KeySchemaElement], + definitions: &[AttributeDefinition], +) -> Result<(), String> { + let kc = parse_key_condition(&tokenize(expr).unwrap()).unwrap(); + let values = HashMap::from([ + ("x".to_owned(), AttributeValue::S("a".to_owned())), + ("y".to_owned(), AttributeValue::S("b".to_owned())), + ("n".to_owned(), AttributeValue::N("1".to_owned())), + ("m".to_owned(), AttributeValue::N("2".to_owned())), + ]); + let maps = ExpressionMaps::new(HashMap::new(), values); + refuse_without_hash_equality(&kc, key_schema, definitions, &maps).map_err(|e| match e { + DynamoDbError::ValidationException(msg) => msg, + other => panic!("{expr}: {other:?}"), + }) +} + +fn assert_refusals( + rows: &[(&str, &str)], + key_schema: &[KeySchemaElement], + definitions: &[AttributeDefinition], +) { + for (expr, want) in rows { + assert_eq!( + check(expr, key_schema, definitions).unwrap_err(), + *want, + "{expr}" + ); + } +} + +/// HASH `pk` and RANGE `sk`, both strings. +fn string_table() -> (Vec, Vec) { + ( + vec![key("pk", KeyType::Hash), key("sk", KeyType::Range)], + vec![ + definition("pk", ScalarAttributeType::S), + definition("sk", ScalarAttributeType::S), + ], + ) +} + +/// An index with HASH `g` and RANGE `s` on the table above, which defines +/// its own key attributes as well. All four are strings. +fn string_index() -> (Vec, Vec) { + ( + vec![key("g", KeyType::Hash), key("s", KeyType::Range)], + ["pk", "sk", "g", "s"] + .into_iter() + .map(|name| definition(name, ScalarAttributeType::S)) + .collect(), + ) +} + +#[test] +fn single_attribute_schema_reports_a_wrong_typed_value_before_the_wording() { + // Row 427-8c. + let (schema, definitions) = string_table(); + assert_refusals( + &[("pk > :n", TYPE_MISMATCH), ("pk > :x", NOT_SUPPORTED)], + &schema, + &definitions, + ); +} + +#[test] +fn single_attribute_schema_reports_begins_with_on_a_number_key_before_the_wording() { + // Rows 6 and 6b. + let schema = [key("pk", KeyType::Hash)]; + let definitions = [definition("pk", ScalarAttributeType::N)]; + assert_refusals( + &[ + ("begins_with(pk, :n)", BEGINS_WITH_ON_N), + ("pk > :n", NOT_SUPPORTED), + ], + &schema, + &definitions, + ); +} + +#[test] +fn begins_with_on_a_number_hash_attribute_is_reported_in_either_clause() { + // HASH `pk` (N) and RANGE `sk` (S), as measured. + let schema = [key("pk", KeyType::Hash), key("sk", KeyType::Range)]; + let definitions = [ + definition("pk", ScalarAttributeType::N), + definition("sk", ScalarAttributeType::S), + ]; + assert_refusals( + &[ + ("begins_with(pk, :n) AND sk > :x", BEGINS_WITH_ON_N), + ("sk > :x AND begins_with(pk, :n)", BEGINS_WITH_ON_N), + ], + &schema, + &definitions, + ); +} + +#[test] +fn two_conditions_on_one_attribute_are_refused_before_a_wrong_typed_value() { + let (schema, definitions) = string_table(); + assert_refusals( + &[ + ("pk > :x AND pk < :n", ONE_PER_KEY), + ("pk > :n AND pk < :m", ONE_PER_KEY), + ], + &schema, + &definitions, + ); +} + +#[test] +fn only_the_clause_on_the_hash_attribute_is_type_checked() { + let (schema, definitions) = string_table(); + assert_refusals( + &[ + ("pk > :n AND sk > :y", TYPE_MISMATCH), + ("sk = :x AND pk > :n", TYPE_MISMATCH), + ("pk BETWEEN :n AND :m", TYPE_MISMATCH), + // An N value on the S sort key is not checked. + ("pk > :x AND sk > :n", NOT_SUPPORTED), + ], + &schema, + &definitions, + ); +} + +#[test] +fn no_value_is_type_checked_without_a_clause_on_the_hash_attribute() { + let (schema, definitions) = string_table(); + assert_refusals( + &[("sk > :n", MISSED_PK), ("zz > :n", MISSED_PK)], + &schema, + &definitions, + ); + // Measured on an index with HASH `gsi1_pk` and RANGE `SK` of a table + // with HASH `PK`: `PK > :n` and `SK > :n` answer the missing key sentence + // for `gsi1_pk`. Here `pk` is the table's HASH attribute and `s` the + // index's RANGE attribute. + let (schema, definitions) = string_index(); + assert_refusals( + &[("pk > :n", MISSED_G), ("s > :n", MISSED_G)], + &schema, + &definitions, + ); +} + +#[test] +fn multi_attribute_schema_reports_the_hash_wording_before_a_wrong_typed_value() { + // Row 4. + let mut schema = vec![key("h", KeyType::Hash)]; + let mut definitions = vec![definition("h", ScalarAttributeType::S)]; + for range in ["a", "b", "c", "d"] { + schema.push(key(range, KeyType::Range)); + definitions.push(definition(range, ScalarAttributeType::S)); + } + assert_refusals(&[("h > :n", HASH_H)], &schema, &definitions); +} + +#[test] +fn a_key_condition_with_a_hash_equality_is_left_to_the_classification() { + // The classification in `handle_query` has already checked the value + // types of this condition; these checks do not repeat it. + let (schema, definitions) = string_table(); + assert_eq!(check("pk = :n", &schema, &definitions), Ok(())); +} diff --git a/crates/server/src/leading_keys/tests.rs b/crates/server/src/leading_keys/tests.rs index c81e3baaa..404faa3f7 100644 --- a/crates/server/src/leading_keys/tests.rs +++ b/crates/server/src/leading_keys/tests.rs @@ -290,6 +290,34 @@ fn query_kce_sort_key_before_partition_key() { assert_eq!(r, Some(vec!["alice".to_owned()])); } +#[test] +fn query_kce_without_a_partition_key_equality_binds_only_a_partition_operand() { + // Clauses with no equality parse without a partition key condition, and + // an equality on another attribute with a non-equality on the partition + // key is no partition key equality either; the engine refuses these + // requests by the key schema. Authorization binds the operand of the one + // clause that names the partition key, as + // `query_non_equality_partition_key_binds_the_operand` does for `pk > :v`, + // and never the operand of a clause on another attribute: `sk = :v AND + // pk > :s` binds `:s`, not `:v`. + for (kce, want) in [ + ("pk > :v", Some("alice")), + ("begins_with(pk, :v)", Some("alice")), + ("sk > :v", None), + ("pk > :v AND sk > :s", Some("alice")), + ("sk > :v AND zz > :s", None), + ("sk = :v AND pk > :s", Some("x")), + ] { + let input = json!({ + "TableName": "t", + "KeyConditionExpression": kce, + "ExpressionAttributeValues": {":v": {"S": "alice"}, ":s": {"S": "x"}}, + }); + let r = single_table_leading_keys(&input, "Query", Some(&main_table()), MAX_TOKENS); + assert_eq!(r, want.map(|v| vec![v.to_owned()]), "{kce}"); + } +} + #[test] fn query_legacy_key_conditions() { let input = json!({ @@ -769,30 +797,34 @@ fn query_non_equality_on_a_gsi_partition_key_binds_the_index_operand() { assert_eq!(r, Some(vec!["galice".to_owned()])); } -/// The single-clause shapes of the fix are shapes the engine's own parser -/// rejects, so the engine never reads a partition value for them and cannot -/// disagree with the value authorization binds. The two-clause shapes the fix -/// also binds (`pk > :v AND sk = :s`, `pk > :v AND sk < :s`) parse or not -/// depending on the sort clause, and the engine refuses them when it resolves -/// the partition attribute; the wire test -/// `test_non_equality_partition_with_sort_clause_is_refused` covers that. The -/// equality shapes the engine accepts keep the value they had, which the -/// accepted-shape tests above cover. +/// The shapes of the fix have no equality on the partition key. The engine +/// refuses them by the key schema before it reads anything, so it never reads +/// a partition value for them and cannot disagree with the value authorization +/// binds. The wire test `test_non_equality_partition_with_sort_clause_is_refused` +/// covers the two-clause shapes. The equality shapes the engine accepts keep the +/// value they had, which the accepted-shape tests above cover. #[test] -fn the_parser_still_rejects_every_single_clause_shape_whose_operand_is_now_bound() { +fn the_engine_still_refuses_every_shape_whose_operand_is_now_bound() { + let key_schema = main_table().key_schema; for kce in [ "pk > :v", "pk < :v", "pk <= :v", "pk >= :v", "begins_with(pk, :v)", + "pk > :v AND sk = :s", + "pk > :v AND sk < :s", ] { let tokens = extenddb_core::expression::tokenize(kce).unwrap(); - let err = extenddb_core::expression::parse_key_condition(&tokens).unwrap_err(); + let kc = extenddb_core::expression::parse_key_condition(&tokens).unwrap(); + let err = kc + .refuse_without_hash_equality(&key_schema, &HashMap::new()) + .unwrap_err(); assert!( matches!( - err, - extenddb_core::error::DynamoDbError::ValidationException(_) + &err, + extenddb_core::error::DynamoDbError::ValidationException(msg) + if msg == "Query key condition not supported" ), "{kce}: {err:?}" ); diff --git a/tests/python/test_query_lone_non_equality.py b/tests/python/test_query_lone_non_equality.py new file mode 100644 index 000000000..b343056db --- /dev/null +++ b/tests/python/test_query_lone_non_equality.py @@ -0,0 +1,597 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Query whose key condition has no equality clause on the HASH attribute. + +Every row below is one measured on the service in us-east-1: the issue table +rows on 2026-10-21, the rows of ``TestValueTypes`` and the two-clause +``begins_with`` rows on 2026-11-01, the rest on 2026-10-23. Rows 427-8c, 5a +and 5b were measured again on 2026-10-31 with the same answers. The table has +HASH ``pk`` and RANGE ``sk``, an LSI ``lsi`` on RANGE ``ls``, an index +``single-idx`` with HASH ``g`` and RANGE ``s``, an index ``multi-idx`` with +HASH ``h`` and RANGE ``a``, ``b``, ``c``, ``d`` and an index ``two-hash`` with +HASH ``h1``, ``h2`` and RANGE ``r``. A second table has only HASH ``pk``, a +third only HASH ``pk`` (N) and a fourth HASH ``pk`` (N) and RANGE ``sk``. All +attributes are strings unless stated. + +The service answers with one of these, chosen by the key schema of the table +or index being queried and by the attributes the clauses name: + +- a non-equality clause names the single HASH attribute: ``Query key condition + not supported`` +- no clause names the single HASH attribute: ``Query condition missed key + schema element: `` +- the schema has several key attributes: ``HASH key attributes must + have equality conditions specified in the query`` +- two conditions on one attribute: ``Invalid KeyConditionExpression: + KeyConditionExpressions must only contain one condition per key`` +- a ``#name`` or ``:value`` the request does not define, ahead of all of the + above: ``Invalid KeyConditionExpression: An expression attribute ...`` + +An unused value, an invalid ``FilterExpression`` or ``ProjectionExpression`` +and an invalid ``Select`` are reported ahead of the key schema wording. An +``ExclusiveStartKey`` and a key value size are reported after it. + +On a schema with one HASH attribute, two conditions on one attribute are +reported first. Then a wrong-typed value in the clause on the HASH attribute, +and ``begins_with`` on a number HASH attribute in either clause, are reported +before the key schema wording and after an unused value and an invalid +``FilterExpression`` (rows 427-8c, 5a, 5b and 6, and ``TestValueTypes``). No +other value is checked. On an index with several key attributes the HASH +wording comes first (row 4, ``h > :n``). +""" + +from __future__ import annotations + +import pytest +from botocore.exceptions import ClientError + +from helpers import unique_name, wait_for_active, wait_for_deleted + +NOT_SUPPORTED = "Query key condition not supported" +MISSED_PK = "Query condition missed key schema element: pk" +MISSED_G = "Query condition missed key schema element: g" +HASH_H = "HASH key attributes h must have equality conditions specified in the query" +HASH_H1_H2 = "HASH key attributes h1, h2 must have equality conditions specified in the query" +ONE_PER_KEY = ( + "Invalid KeyConditionExpression: KeyConditionExpressions must only contain one " + "condition per key" +) +NO_VALUE = ( + "Invalid KeyConditionExpression: An expression attribute value used in expression is " + "not defined; attribute value: :x" +) +TYPE_MISMATCH = ( + "One or more parameter values were invalid: Condition parameter type does not match " + "schema type" +) +BEGINS_WITH_ON_N = ( + "Invalid KeyConditionExpression: Incorrect operand type for operator or function; " + "operator or function: begins_with, operand type: N" +) + + +def _attr(name: str, kind: str = "S") -> dict: + return {"AttributeName": name, "AttributeType": kind} + + +def _key(name: str, key_type: str) -> dict: + return {"AttributeName": name, "KeyType": key_type} + + +def _index(name: str, schema: list[dict]) -> dict: + return {"IndexName": name, "KeySchema": schema, "Projection": {"ProjectionType": "ALL"}} + + +def _drop(client, name: str) -> None: + try: + client.delete_table(TableName=name) + except client.exceptions.ResourceNotFoundException: + return + wait_for_deleted(client, name) + + +@pytest.fixture(scope="module") +def table(dynamodb_client): + """HASH ``pk`` and RANGE ``sk``, the LSI and the three indexes above.""" + name = unique_name("qlne") + names = ("pk", "sk", "ls", "g", "s", "h", "a", "b", "c", "d", "h1", "h2", "r") + dynamodb_client.create_table( + TableName=name, + AttributeDefinitions=[_attr(n) for n in names], + KeySchema=[_key("pk", "HASH"), _key("sk", "RANGE")], + BillingMode="PAY_PER_REQUEST", + LocalSecondaryIndexes=[_index("lsi", [_key("pk", "HASH"), _key("ls", "RANGE")])], + GlobalSecondaryIndexes=[ + _index("single-idx", [_key("g", "HASH"), _key("s", "RANGE")]), + _index( + "multi-idx", + [_key("h", "HASH")] + [_key(n, "RANGE") for n in ("a", "b", "c", "d")], + ), + _index( + "two-hash", + [_key("h1", "HASH"), _key("h2", "HASH"), _key("r", "RANGE")], + ), + ], + ) + try: + wait_for_active(dynamodb_client, name) + yield name + finally: + _drop(dynamodb_client, name) + + +def _plain_table(client, prefix: str, attributes: list[dict], schema: list[dict]): + """Create a table with no index, yield its name, then drop it.""" + name = unique_name(prefix) + client.create_table( + TableName=name, + AttributeDefinitions=attributes, + KeySchema=schema, + BillingMode="PAY_PER_REQUEST", + ) + try: + wait_for_active(client, name) + yield name + finally: + _drop(client, name) + + +@pytest.fixture(scope="module") +def hash_only_table(dynamodb_client): + """A table with only HASH ``pk`` (S).""" + yield from _plain_table(dynamodb_client, "qlne-hash", [_attr("pk")], [_key("pk", "HASH")]) + + +@pytest.fixture(scope="module") +def number_table(dynamodb_client): + """A table with only HASH ``pk`` (N).""" + yield from _plain_table(dynamodb_client, "qlne-num", [_attr("pk", "N")], [_key("pk", "HASH")]) + + +@pytest.fixture(scope="module") +def number_range_table(dynamodb_client): + """A table with HASH ``pk`` (N) and RANGE ``sk`` (S).""" + pk_sk = [_key("pk", "HASH"), _key("sk", "RANGE")] + yield from _plain_table(dynamodb_client, "qlne-numr", [_attr("pk", "N"), _attr("sk")], pk_sk) + + +def _refused_request(client, message: str, **request) -> None: + """Run the Query and assert the refusal's code, status and exact text.""" + with pytest.raises(ClientError) as exc_info: + client.query(**request) + err = exc_info.value.response + assert err["Error"]["Code"] == "ValidationException", err["Error"]["Message"] + assert err["ResponseMetadata"]["HTTPStatusCode"] == 400 + assert err["Error"]["Message"] == message + + +def _refused_prefix(client, prefix: str, **request) -> None: + """Like ``_refused_request`` for a refusal whose text ExtendDB words differently.""" + with pytest.raises(ClientError) as exc_info: + client.query(**request) + err = exc_info.value.response + assert err["Error"]["Code"] == "ValidationException", err["Error"]["Message"] + assert err["Error"]["Message"].startswith(prefix), err["Error"]["Message"] + + +def _refused_suffix(client, suffix: str, **request) -> None: + """Like ``_refused_request`` for a refusal whose leading text ExtendDB words differently.""" + with pytest.raises(ClientError) as exc_info: + client.query(**request) + err = exc_info.value.response + assert err["Error"]["Code"] == "ValidationException", err["Error"]["Message"] + assert err["Error"]["Message"].endswith(suffix), err["Error"]["Message"] + + +def _values(expression: str) -> dict: + """Those of ``:x``, ``:y`` (strings), ``:n`` and ``:m`` (numbers) the expression uses.""" + values = {":x": {"S": "a"}, ":y": {"S": "b"}, ":n": {"N": "1"}, ":m": {"N": "2"}} + return {name: value for name, value in values.items() if name in expression} + + +def _refused( + client, + table: str, + message: str, + expression: str, + index: str | None = None, + **extra, +) -> None: + """``_refused_request`` with the values ``_values`` gives.""" + request: dict = { + "TableName": table, + "KeyConditionExpression": expression, + "ExpressionAttributeValues": _values(expression), + } + if index is not None: + request["IndexName"] = index + request.update(extra) + _refused_request(client, message, **request) + + +class TestPartitionKeyClause: + """Rows 1 to 3 of the issue: the clause names the single HASH attribute.""" + + def test_table_partition_key_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "pk > :x") + + def test_table_partition_key_begins_with(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "begins_with(pk, :x)") + + def test_index_partition_key_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "g > :x", index="single-idx") + + +class TestSortKeyClause: + """Rows 4 and 5 of the issue: the clause names a sort key.""" + + def test_table_sort_key_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, MISSED_PK, "sk > :x") + + def test_index_sort_key_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, MISSED_G, "s > :x", index="single-idx") + + +class TestMultiAttributeIndex: + """Row 6 of the issue and the shapes measured around it.""" + + def test_hash_attribute_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, HASH_H, "h > :x", index="multi-idx") + + @pytest.mark.parametrize( + "expression", + ["a > :x", "d > :x", "z > :x", "begins_with(a, :x)", "a BETWEEN :x AND :y"], + ) + def test_other_attributes_name_the_hash_attribute(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, HASH_H, expression, index="multi-idx") + + def test_a_number_value_does_not_change_the_wording(self, dynamodb_client, table): + # `h` is a string: the service answers the HASH wording, not a type error. + _refused_request( + dynamodb_client, + HASH_H, + TableName=table, + IndexName="multi-idx", + KeyConditionExpression="h > :n", + ExpressionAttributeValues={":n": {"N": "1"}}, + ) + + @pytest.mark.parametrize("expression", ["h1 > :x", "h2 > :x", "r > :x", "z > :x"]) + def test_two_hash_index_names_both_hash_attributes(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, HASH_H1_H2, expression, index="two-hash") + + def test_two_hash_index_with_a_name_reference(self, dynamodb_client, table): + _refused( + dynamodb_client, + table, + HASH_H1_H2, + "#a > :x", + index="two-hash", + ExpressionAttributeNames={"#a": "h1"}, + ) + + +class TestMeasuredShapes: + """Rows 427-1 to 427-9, measured 2026-10-23: shapes the issue table did not cover.""" + + def test_table_partition_key_between(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "pk BETWEEN :x AND :y") + + def test_index_partition_key_between(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "g BETWEEN :x AND :y", index="single-idx") + + def test_hash_only_table_begins_with(self, dynamodb_client, hash_only_table): + _refused(dynamodb_client, hash_only_table, NOT_SUPPORTED, "begins_with(pk, :x)") + + @pytest.mark.parametrize( + "fixture,expression,message", + [ + ("number_table", "pk > :n", NOT_SUPPORTED), + ("number_table", "begins_with(pk, :n)", BEGINS_WITH_ON_N), + ("number_range_table", "begins_with(pk, :n) AND sk > :x", BEGINS_WITH_ON_N), + ("number_range_table", "sk > :x AND begins_with(pk, :n)", BEGINS_WITH_ON_N), + ], + ) + def test_number_keyed_tables(self, dynamodb_client, request, fixture, expression, message): + """Rows 6b and 6 (2026-10-23); ``begins_with`` with a clause on ``sk`` (2026-11-01).""" + _refused(dynamodb_client, request.getfixturevalue(fixture), message, expression) + + def test_number_value_on_a_string_key_reports_the_type(self, dynamodb_client, table): + # Row 427-8c: the type check comes before the key schema wording. + _refused_request( + dynamodb_client, + TYPE_MISMATCH, + TableName=table, + KeyConditionExpression="pk > :n", + ExpressionAttributeValues={":n": {"N": "1"}}, + ) + + def test_lsi_partition_key_greater_than(self, dynamodb_client, table): + _refused(dynamodb_client, table, NOT_SUPPORTED, "pk > :x", index="lsi") + + def test_lsi_lone_range_clause(self, dynamodb_client, table): + _refused(dynamodb_client, table, MISSED_PK, "ls > :x", index="lsi") + + def test_non_key_attribute(self, dynamodb_client, table): + _refused(dynamodb_client, table, MISSED_PK, "z > :x") + + def test_legacy_key_conditions_greater_than_on_partition_key(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + NOT_SUPPORTED, + TableName=table, + KeyConditions={"pk": {"ComparisonOperator": "GT", "AttributeValueList": [{"S": "a"}]}}, + ) + + def test_legacy_key_conditions_sort_key_only(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + MISSED_PK, + TableName=table, + KeyConditions={"sk": {"ComparisonOperator": "GT", "AttributeValueList": [{"S": "a"}]}}, + ) + + +class TestNameReferences: + """Rows 2a to 2d, measured 2026-10-23: a defined ``#name`` is resolved.""" + + @pytest.mark.parametrize( + "expression,reference,attribute,message", + [ + ("#p > :x", "#p", "pk", NOT_SUPPORTED), + ("#s > :x", "#s", "sk", MISSED_PK), + ("#z > :x", "#z", "zz", MISSED_PK), + ("begins_with(#s, :x)", "#s", "sk", MISSED_PK), + ], + ) + def test_defined_name_is_resolved( + self, dynamodb_client, table, expression, reference, attribute, message + ): + _refused( + dynamodb_client, + table, + message, + expression, + ExpressionAttributeNames={reference: attribute}, + ) + + +class TestUndefinedReferences: + """Undefined references win over the key schema wording, with the prefix.""" + + def test_undefined_name(self, dynamodb_client, table): + _refused( + dynamodb_client, + table, + "Invalid KeyConditionExpression: An expression attribute name used in the " + "document path is not defined; attribute name: #nope", + "#nope > :x", + ) + + def test_undefined_name_in_the_first_of_two_clauses(self, dynamodb_client, table): + _refused( + dynamodb_client, + table, + "Invalid KeyConditionExpression: An expression attribute name used in the " + "document path is not defined; attribute name: #n", + "#n > :x AND sk > :y", + ) + + @pytest.mark.parametrize("expression", ["pk > :x", "sk > :x", "pk = :x"]) + def test_undefined_value(self, dynamodb_client, table, expression): + _refused_request( + dynamodb_client, NO_VALUE, TableName=table, KeyConditionExpression=expression + ) + + def test_undefined_value_in_the_first_of_two_clauses(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + NO_VALUE, + TableName=table, + KeyConditionExpression="pk > :x AND sk > :y", + ExpressionAttributeValues={":y": {"S": "b"}}, + ) + + +class TestTwoClauses: + """Measured 2026-10-23: one or two clauses, no HASH equality.""" + + @pytest.mark.parametrize( + "expression", + [ + "pk > :x AND sk > :y", + "sk > :x AND pk > :y", + "pk > :x AND zz > :y", + "zz > :x AND pk > :y", + "pk >= :x AND sk = :y", + "sk = :x AND pk > :y", + "pk > :x AND sk = :y", + "begins_with(pk, :x) AND sk = :y", + "pk BETWEEN :x AND :y AND sk = :x", + "zz = :x AND pk > :y", + ], + ) + def test_a_non_equality_names_the_partition_key(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, NOT_SUPPORTED, expression) + + @pytest.mark.parametrize("expression", ["sk > :x AND zz > :y", "zz > :x AND sk > :y"]) + def test_no_clause_names_the_partition_key(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, MISSED_PK, expression) + + @pytest.mark.parametrize( + "expression", + ["zz = :x AND sk > :y", "zz = :x AND yy = :y", "sk = :x AND zz = :y"], + ) + def test_equalities_on_other_attributes_miss_the_partition_key( + self, dynamodb_client, table, expression + ): + _refused(dynamodb_client, table, MISSED_PK, expression) + + @pytest.mark.parametrize("expression", ["g > :x AND s > :y", "g > :x AND s = :y"]) + def test_index_clause_names_the_index_partition_key(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, NOT_SUPPORTED, expression, index="single-idx") + + @pytest.mark.parametrize( + "expression", ["s > :x AND zz > :y", "pk > :x AND s > :y", "pk = :x AND s > :y"] + ) + def test_index_clauses_that_miss_the_index_partition_key( + self, dynamodb_client, table, expression + ): + _refused(dynamodb_client, table, MISSED_G, expression, index="single-idx") + + @pytest.mark.parametrize("expression", ["pk > :x AND pk < :y", "sk > :x AND sk < :y"]) + def test_two_conditions_on_one_key(self, dynamodb_client, table, expression): + _refused(dynamodb_client, table, ONE_PER_KEY, expression) + + +class TestOrderAmongOtherRefusals: + """Rows 5a, 5b, 8, 9, 10, 11 and P4, measured 2026-10-23. + + Reported ahead of the key schema wording: an unused value, an invalid + ``FilterExpression`` or ``ProjectionExpression`` and an invalid ``Select``. + Reported after it: an ``ExclusiveStartKey`` and a key value size. + """ + + def test_unused_value_is_reported_first(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + "Value provided in ExpressionAttributeValues unused in expressions: keys: {:y}", + TableName=table, + KeyConditionExpression="pk > :x", + ExpressionAttributeValues={":x": {"S": "a"}, ":y": {"S": "b"}}, + ) + + def test_unused_value_is_reported_before_a_wrong_typed_value(self, dynamodb_client, table): + # Row 5a: `:n` is an N value on the S key. + _refused_request( + dynamodb_client, + "Value provided in ExpressionAttributeValues unused in expressions: keys: {:y}", + TableName=table, + KeyConditionExpression="pk > :n", + ExpressionAttributeValues={":n": {"N": "1"}, ":y": {"S": "b"}}, + ) + + def test_invalid_filter_expression_is_reported_before_a_wrong_typed_value( + self, dynamodb_client, table + ): + # Row 5b: `:n` is an N value on the S key. Only the label is asserted, + # as in the test below. + _refused_prefix( + dynamodb_client, + "Invalid FilterExpression: ", + TableName=table, + KeyConditionExpression="pk > :n", + FilterExpression="nope(", + ExpressionAttributeValues={":n": {"N": "1"}}, + ) + + def test_invalid_filter_expression_is_reported_first(self, dynamodb_client, table): + # The service's text is 'Invalid FilterExpression: Syntax error; token: + # "", near: "("'. ExtendDB words it differently, so only the label + # is asserted: the filter error comes before the key condition wording. + _refused_prefix( + dynamodb_client, + "Invalid FilterExpression: ", + TableName=table, + KeyConditionExpression="pk > :x", + FilterExpression="nope(", + ExpressionAttributeValues={":x": {"S": "a"}}, + ) + + def test_invalid_projection_expression_is_reported_first(self, dynamodb_client, table): + # The service's text is 'Invalid ProjectionExpression: Syntax error; token: + # ",", near: ",,b"'. + _refused_prefix( + dynamodb_client, + "Invalid ProjectionExpression: ", + TableName=table, + KeyConditionExpression="pk > :x", + ProjectionExpression="a,,b", + ExpressionAttributeValues={":x": {"S": "a"}}, + ) + + def test_count_with_a_projection_is_reported_first(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + "1 validation error detected: Cannot specify the ProjectionExpression when " + "choosing to get only the Count", + TableName=table, + KeyConditionExpression="pk > :x", + Select="COUNT", + ProjectionExpression="a", + ExpressionAttributeValues={":x": {"S": "a"}}, + ) + + def test_all_projected_attributes_without_an_index_is_reported_first( + self, dynamodb_client, table + ): + # The service's text starts with "1 validation error detected: "; + # ExtendDB's does not (the Select check is shared with Scan). + _refused_suffix( + dynamodb_client, + "ALL_PROJECTED_ATTRIBUTES can be used only when Querying using an IndexName", + TableName=table, + KeyConditionExpression="pk > :x", + Select="ALL_PROJECTED_ATTRIBUTES", + ExpressionAttributeValues={":x": {"S": "a"}}, + ) + + @pytest.mark.parametrize( + "start_key", + [ + {"pk": {"S": "other"}, "sk": {"S": "a"}}, + {"pk": {"S": "other"}}, + ], + ids=["other-partition", "missing-sort-key"], + ) + def test_exclusive_start_key_is_checked_after_the_wording( + self, dynamodb_client, table, start_key + ): + _refused(dynamodb_client, table, NOT_SUPPORTED, "pk > :x", ExclusiveStartKey=start_key) + + def test_partition_key_size_is_checked_after_the_wording(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + NOT_SUPPORTED, + TableName=table, + KeyConditionExpression="pk > :x", + ExpressionAttributeValues={":x": {"S": "x" * 2049}}, + ) + + def test_sort_key_size_is_checked_after_the_wording(self, dynamodb_client, table): + _refused_request( + dynamodb_client, + MISSED_PK, + TableName=table, + KeyConditionExpression="sk > :x", + ExpressionAttributeValues={":x": {"S": "x" * 1025}}, + ) + + +class TestValueTypes: + """A wrong-typed value with no HASH equality, measured on 2026-11-01. + + Two conditions on one attribute are refused first. Only the clause on the + HASH attribute is type-checked, and with no such clause no value is. The + index was measured with HASH ``gsi1_pk`` and RANGE ``SK`` of a table with + HASH ``PK``; here ``pk`` is outside ``single-idx`` and ``s`` its RANGE. + """ + + @pytest.mark.parametrize( + "expression,index,message", + [ + ("pk > :x AND pk < :n", None, ONE_PER_KEY), + ("pk > :n AND pk < :m", None, ONE_PER_KEY), + ("pk > :n AND sk > :y", None, TYPE_MISMATCH), + ("sk = :x AND pk > :n", None, TYPE_MISMATCH), + ("pk BETWEEN :n AND :m", None, TYPE_MISMATCH), + ("pk > :x AND sk > :n", None, NOT_SUPPORTED), + ("sk > :n", None, MISSED_PK), + ("zz > :n", None, MISSED_PK), + ("pk > :n", "single-idx", MISSED_G), + ("s > :n", "single-idx", MISSED_G), + ], + ) + def test_value_type(self, dynamodb_client, table, expression, index, message): + _refused(dynamodb_client, table, message, expression, index=index)