From f15581d56c2495fdbdae553835dffcde6d7c8187 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sat, 24 Oct 2026 00:35:43 +0000 Subject: [PATCH] fix(query): refuse a key condition with no HASH equality the way the service does Closes #427. A Query whose KeyConditionExpression has no equality clause on the HASH attribute was refused with a parser sentence of ExtendDB's own. The service chooses the sentence from the key schema. Measured on 2026-10-23, every row repeated with identical answers: - Single HASH attribute, any clause names it (so the clause on it is a non-equality): "Query key condition not supported". Rows: pk > :x, pk BETWEEN :x AND :y, begins_with(pk, :x), pk > :x AND sk = :y, sk = :x AND pk > :y, zz = :x AND pk > :y, and the GSI forms. - Single HASH attribute, no clause names it: "Query condition missed key schema element: ". Rows: sk > :x, zz > :x, sk > :x AND zz > :y, zz = :x AND yy = :y, and the GSI forms naming the index HASH attribute. - Several key attributes (two HASH attributes, or one HASH with several RANGE attributes): "HASH key attributes h1, h2 must have equality conditions specified in the query", naming every HASH attribute, for every operator and for an N value on the HASH attribute. - Two conditions on one attribute: "Invalid KeyConditionExpression: KeyConditionExpressions must only contain one condition per key". - An undefined #name or :value in the key condition is refused first, with the "Invalid KeyConditionExpression: " prefix, for equality shapes too. - Legacy KeyConditions is desugared with every reference defined and an equality on the HASH attribute, or refused there; no change. Ordering, measured: unused values, filter syntax, projection syntax, the Select conflicts and undefined key references are reported before this refusal; an ExclusiveStartKey fault and an oversized key value after it. The check lives in crates/core/src/expression/key_condition/no_hash_equality.rs with KeyCondition::lacks_hash_equality and refuse_without_hash_equality, and KeyCondition::validate_references in key_condition/references.rs. The parser now records two non-equality clauses instead of refusing them, so the engine can choose the sentence. handle_query calls validate_references for every key condition, then the refusal after validate_select_projection and before the partition key resolution. Tests: unit tests in no_hash_equality/tests.rs and references.rs; wire file tests/python/test_query_lone_non_equality.py (67 rows) passes against the service and on SQLite. The previous head fails 25 of the 67 rows. Pre-existing, measured, not changed here: "<>", NOT, OR, IN and contains in a key condition get "Invalid operator used in KeyConditionExpression" on the service; pk = :x AND z > :x gets "missed key schema element: sk" on the service and 200 here; begins_with on an N key is the operand type sentence; the Select ALL_PROJECTED_ATTRIBUTES text lacks the "1 validation error detected: " prefix here. Row 427-8c (pk > :n with an N value on an S key, "Condition parameter type does not match schema type") needs the type check in #429, which precedes this refusal on a single-attribute schema and follows the HASH wording on a multi-attribute index. --- crates/core/src/expression/key_condition.rs | 23 +- .../key_condition/no_hash_equality.rs | 284 ++++++++++ .../key_condition/no_hash_equality/tests.rs | 408 ++++++++++++++ .../expression/key_condition/references.rs | 120 ++++ crates/engine/src/query.rs | 103 ++-- crates/server/src/leading_keys/tests.rs | 24 + tests/python/test_query_lone_non_equality.py | 523 ++++++++++++++++++ 7 files changed, 1436 insertions(+), 49 deletions(-) create mode 100644 crates/core/src/expression/key_condition/no_hash_equality.rs create mode 100644 crates/core/src/expression/key_condition/no_hash_equality/tests.rs create mode 100644 crates/core/src/expression/key_condition/references.rs create mode 100644 tests/python/test_query_lone_non_equality.py diff --git a/crates/core/src/expression/key_condition.rs b/crates/core/src/expression/key_condition.rs index 2bdbd8b7d..04173595c 100755 --- a/crates/core/src/expression/key_condition.rs +++ b/crates/core/src/expression/key_condition.rs @@ -18,6 +18,9 @@ use super::parser_common; use super::tokenizer::Token; use crate::error::DynamoDbError; +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 @@ -27,7 +30,10 @@ use crate::error::DynamoDbError; /// `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, @@ -353,9 +359,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)), } } @@ -363,7 +369,7 @@ fn classify_pair(a: RawClause, b: RawClause) -> Result { Ok(KeyCondition { @@ -374,8 +380,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..bcd1fe4fc --- /dev/null +++ b/crates/core/src/expression/key_condition/no_hash_equality.rs @@ -0,0 +1,284 @@ +// 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. A second 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, recorded in the issue's fold files): +//! +//! | 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. + +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. + pub(super) fn without_equality( + first: SortKeyCondition, + second: Option, + ) -> Self { + let pk_value = first_operand(&first).clone(); + let extra_sk_conditions = second.map_or_else(Vec::new, |clause| operands(&clause)); + Self { + pk_path: Vec::new(), + pk_value, + extra_pk_conditions: Vec::new(), + sk_condition: Some(first), + 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(()); + } + let attrs = self + .clause_paths() + .into_iter() + .map(|path| resolve_clause_attr(path, names)) + .collect::, _>>()?; + + if self.pk_path.is_empty() && attrs.len() == 2 && attrs[0] == attrs[1] { + return Err(DynamoDbError::ValidationException( + ONE_CONDITION_PER_KEY.to_owned(), + )); + } + + 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)) + } + + /// The path of each clause, in classification order: the equality, the + /// `sk_condition` and, for two clauses with no equality, the second 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..e4bbd84ab --- /dev/null +++ b/crates/core/src/expression/key_condition/no_hash_equality/tests.rs @@ -0,0 +1,408 @@ +// 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 (the issue's fold files +//! `FOLD-427.md` rows `427-*` and `FOLD-427-R.md` rows from +//! `service-427-review.jsonl` and `service-427-review-2.jsonl`). + +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 of service-427-review.jsonl. + 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` from service-427-review-2.jsonl. + 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() { + // Rows of service-427-review-2.jsonl, 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` of service-427-review.jsonl. + 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 *` of service-427-review.jsonl. + 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())); +} 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..686703c24 --- /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) of the + // review rows, 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 f45526af8..c90f32b2c 100755 --- a/crates/engine/src/query.rs +++ b/crates/engine/src/query.rs @@ -270,51 +270,58 @@ pub async fn handle_query( &maps }; - // Correct PK/SK assignment when both clauses are equality comparisons. - // The parser can't distinguish PK from SK without the key schema. - let pk_attr = &query_key_info.key_schema[0].attribute_name; - key_condition.resolve_pk_sk(pk_attr, &effective_maps.names)?; - - // Validate that the partition key is actually referenced in the condition. - 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}" - ))); - } - - // For multi-part key schemas (GSIs with >1 HASH attribute), reclassify - // the parsed conditions so all HASH attributes go to pk_path/extra_pk_conditions - // and the RANGE condition stays as sk_condition. - if extenddb_core::types::is_multipart_key_schema(&query_key_info.key_schema) { - let hash_elements = extenddb_core::types::hash_key_elements(&query_key_info.key_schema); - let hash_attrs: Vec<&str> = hash_elements - .iter() - .map(|ks| ks.attribute_name.as_str()) - .collect(); - key_condition.resolve_multipart(&hash_attrs, &effective_maps.names)?; + // A key condition with no equality clause on the HASH attribute has no + // partition key clause to assign or check. It 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). + if !key_condition.lacks_hash_equality(&query_key_info.key_schema, &effective_maps.names) { + // Correct PK/SK assignment when both clauses are equality comparisons. + // The parser can't distinguish PK from SK without the key schema. + let pk_attr = &query_key_info.key_schema[0].attribute_name; + key_condition.resolve_pk_sk(pk_attr, &effective_maps.names)?; + + // Validate that the partition key is actually referenced in the condition. + 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}" + ))); + } - // Validate all HASH attributes are present in the KeyConditionExpression. - let provided_count = 1 + key_condition.extra_pk_conditions.len(); - if provided_count != hash_attrs.len() { - // Find the first missing HASH attribute for the error message. - let missing = hash_attrs + // For multi-part key schemas (GSIs with >1 HASH attribute), reclassify + // the parsed conditions so all HASH attributes go to pk_path/extra_pk_conditions + // and the RANGE condition stays as sk_condition. + if extenddb_core::types::is_multipart_key_schema(&query_key_info.key_schema) { + let hash_elements = extenddb_core::types::hash_key_elements(&query_key_info.key_schema); + let hash_attrs: Vec<&str> = hash_elements .iter() - .find(|attr| { - let pk_name = - resolve_path_attr_name(&key_condition.pk_path, &effective_maps.names); - if pk_name.as_deref() == Some(*attr) { - return false; - } - !key_condition.extra_pk_conditions.iter().any(|(path, _)| { - resolve_path_attr_name(path, &effective_maps.names).as_deref() - == Some(*attr) + .map(|ks| ks.attribute_name.as_str()) + .collect(); + key_condition.resolve_multipart(&hash_attrs, &effective_maps.names)?; + + // Validate all HASH attributes are present in the KeyConditionExpression. + let provided_count = 1 + key_condition.extra_pk_conditions.len(); + if provided_count != hash_attrs.len() { + // Find the first missing HASH attribute for the error message. + let missing = hash_attrs + .iter() + .find(|attr| { + let pk_name = + resolve_path_attr_name(&key_condition.pk_path, &effective_maps.names); + if pk_name.as_deref() == Some(*attr) { + return false; + } + !key_condition.extra_pk_conditions.iter().any(|(path, _)| { + resolve_path_attr_name(path, &effective_maps.names).as_deref() + == Some(*attr) + }) }) - }) - .unwrap_or(&hash_attrs[0]); - return Err(DynamoDbError::ValidationException(format!( - "Query condition missed key schema element: {missing}" - ))); + .unwrap_or(&hash_attrs[0]); + return Err(DynamoDbError::ValidationException(format!( + "Query condition missed key schema element: {missing}" + ))); + } } } @@ -433,6 +440,18 @@ 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 with no equality clause on the HASH attribute 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 an equality on the HASH attribute, or + // refused there, so it passes both calls. + key_condition.validate_references(effective_maps)?; + key_condition + .refuse_without_hash_equality(&query_key_info.key_schema, &effective_maps.names)?; + // 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() diff --git a/crates/server/src/leading_keys/tests.rs b/crates/server/src/leading_keys/tests.rs index 7cc17626c..15920ff03 100644 --- a/crates/server/src/leading_keys/tests.rs +++ b/crates/server/src/leading_keys/tests.rs @@ -290,6 +290,30 @@ 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_leaves_the_key_undetermined() { + // 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, so no key is determinable. + for kce in [ + "pk > :v", + "begins_with(pk, :v)", + "sk > :v", + "pk > :v AND sk > :s", + "sk > :v AND zz > :s", + "sk = :v AND pk > :s", + ] { + 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, None, "{kce}"); + } +} + #[test] fn query_legacy_key_conditions() { let input = json!({ 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..26aeba2c2 --- /dev/null +++ b/tests/python/test_query_lone_non_equality.py @@ -0,0 +1,523 @@ +# 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 rest on 2026-10-23 (recorded in the issue's fold +files, rows ``427-*`` and the two review row files). 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`` and a third has an N +partition key. 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. + +Row 427-8c (``pk > :n`` with an N value on an S key, service: ``One or more +parameter values were invalid: Condition parameter type does not match schema +type``) and the ``begins_with(pk, :n)`` row on an N-keyed table (service: +``Invalid KeyConditionExpression: Incorrect operand type for operator or +function; operator or function: begins_with, operand type: N``) are not +asserted here. Both need the key value type check that arrives with the +multi-attribute range query change; a test for them belongs with that merge. +""" + +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" +) + + +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) + + +@pytest.fixture(scope="module") +def hash_only_table(dynamodb_client): + """A table with only HASH ``pk`` (S).""" + name = unique_name("qlne-hash") + dynamodb_client.create_table( + TableName=name, + AttributeDefinitions=[_attr("pk")], + KeySchema=[_key("pk", "HASH")], + BillingMode="PAY_PER_REQUEST", + ) + try: + wait_for_active(dynamodb_client, name) + yield name + finally: + _drop(dynamodb_client, name) + + +@pytest.fixture(scope="module") +def number_table(dynamodb_client): + """A table with only HASH ``pk`` (N).""" + name = unique_name("qlne-num") + dynamodb_client.create_table( + TableName=name, + AttributeDefinitions=[_attr("pk", "N")], + KeySchema=[_key("pk", "HASH")], + BillingMode="PAY_PER_REQUEST", + ) + try: + wait_for_active(dynamodb_client, name) + yield name + finally: + _drop(dynamodb_client, name) + + +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: + """``:x`` and, when the expression uses it, ``:y`` as strings.""" + values = {":x": {"S": "a"}} + if ":y" in expression: + values[":y"] = {"S": "b"} + return values + + +def _refused( + client, + table: str, + message: str, + expression: str, + index: str | None = None, + **extra, +) -> None: + """``_refused_request`` with ``:x`` (and ``:y``) defined as strings.""" + 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: + """Fold file rows 427-1 to 427-9: 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)") + + def test_number_keyed_table_partition_key(self, dynamodb_client, number_table): + # `pk > :n` with an N value on an N key (row 6b of service-427-review.jsonl). + _refused_request( + dynamodb_client, + NOT_SUPPORTED, + TableName=number_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 of service-427-review.jsonl: 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: + """Rows of service-427-review-2.jsonl: 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 of service-427-review.jsonl. + + 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_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}}, + )