Repository navigation
fix(expressions): validate in the service's order, with its messages - #399
LeeroyHannigan wants to merge 1 commit into
Conversation
…ts messages Issue #108 lists 106 requests whose response differs from Amazon DynamoDB. Every expectation in this change was measured against the service (us-east-1, 2026-10-14); the probes and captured responses are in the issue's notes, and every new test file was run against the service before it was run here. Semantic validation of expressions. After the grammar pass, the service walks the parsed expression once, in expression order, checking each node after its operands, then checks overlapping paths, then unused names and values. A new `validate_update_semantics` and `validate_condition_semantics` in the core crate follow that walk; the parsers now do grammar only, keep function names as written, and record section order for the walk. - UpdateExpression: `+` and `-` take numbers, `list_append` takes lists, `ADD` takes a number or a set, `DELETE` takes a set; a placeholder of the wrong type is refused with the service's type vocabulary (`S`, `N`, `L` for operators and functions; `STRING`, `NUMBER`, `Binary`, `BOOL`, `NULL`, `LIST`, `MAP` for ADD and DELETE). Function names are case-sensitive: `IF_NOT_EXISTS(x, :v)` and `foo(x)` are `Invalid function name`, a condition function is `not allowed in an update expression`, `list_append` is refused as an operand of a function or of `+`, both functions take two operands, the first operand of `if_not_exists` is a document path. A repeated section is reported after that section's own actions (`SET a = :v SET b = :q` reports `:q`). Previously `IF_NOT_EXISTS`, `DELETE s :string`, and `SET a = :string + :n` were accepted and written. - ConditionExpression and FilterExpression: the six condition functions are matched as written (`ATTRIBUTE_EXISTS(pk)` was accepted and evaluated); `if_not_exists` and `list_append` are `not allowed in a condition expression`; a boolean function in operand position and `size` in condition position are `not allowed to be used this way`; operand counts are fixed per function; `attribute_exists` and `attribute_not_exists` take a document path; `begins_with` takes a string or binary and `attribute_type` a string naming a type, both checked at request time with the operation's prefix. - Every unused name or value is listed in one set, sorted, so the message is the same on every backend; it named one key chosen by hash order. Request stage on the read APIs. Query and Scan check the FilterExpression, then the KeyConditionExpression (with its `Invalid KeyConditionExpression:` prefix, and ahead of the key schema match, so an undefined `#n` is reported as such), then the ProjectionExpression, then unused names and values, all before the table is looked up. An undefined `:q` in a filter returned 200 on an empty table. Limit and Segment checks precede the expressions on Scan. Single-item order. UpdateItem checks the whole ConditionExpression before the UpdateExpression. PutItem, UpdateItem, and DeleteItem wrap every expression-stage error in `1 validation error detected: ` and omit the `: ConditionExpression is null` suffix; the read, batch, and transaction APIs return the messages bare. The ` expression size: N` suffix follows the operation. Three existing dual-target tests asserted the unprefixed text and fail against the service; they are corrected. Transactions. TransactGetItems and TransactWriteItems validate in phases, each over every item before the next: per item, expressions (UpdateExpression before ConditionExpression), unused names and values, then the item's 400 KB size; then table lookup; then the 4 MB total; then key schema, as a cancellation with ValidationError on the first offending item only and None on every other (it flagged every offender); then duplicates. An undefined reference is the request error the service returns rather than a per-item cancellation reason, and a key schema mismatch leaves other items' conditions unevaluated. TransactWriteItems returns `"ItemCollectionMetrics": {}` when SIZE is requested and no table has a local secondary index, and its ConsumedCapacity entries are in first-appearance order rather than per-process hash order. Key-shaped messages. GetItem, DeleteItem, UpdateItem, BatchGetItem, and BatchWriteItem (Put and Delete) refuse a key with a missing, mistyped, or extra attribute with `The provided key element does not match the schema`; PutItem keeps `Missing the key pk in the item` and `Type mismatch for key`. BatchWriteItem returns `"ItemCollectionMetrics": {}` under SIZE with no LSI. Table ARNs in `TableName` (eight #108 rows) stay unsupported by decision: an ExtendDB table ARN names ExtendDB's account and region, so a table that streams or replicates to Amazon DynamoDB would answer to two ARNs. docs/differences-from-dynamodb.md records this. Remaining #108 gaps after this change, all stated in the PR: the parser's syntax wording (`Syntax error; token: X, near: Y`), the ResourceNotFound message text, the 4 MB message (the service's carries its own payload size), the service's rendering of over-limit list values, and the table ARN rows. Testing: tests/test_transact_validation_order.py (31 cases), tests/test_update_expression_rules.py (37), and tests/test_expression_semantics.py (155) pass against the service and on SQLite, PostgreSQL, and MongoDB; 164 of the 186 cases in the first and third fail on main. The expression, validation, transaction, batch, query, and scan suites pass on all three backends (767 on SQLite and PostgreSQL, 751 on MongoDB with the four #387 cases unchanged). Of the 106 #108 rows, 83 now return the service's status, error type, and message (30 on main). cargo test --workspace 1,500 passed; fmt; clippy -D warnings on 1.97.0; cargo +1.88.0 check. Closes part of #108.
| // itself; whether that function may stand alone, whether its name exists, | ||
| // and how many operands it takes are semantic questions answered after | ||
| // the grammar pass (see `validate_condition_semantics`), so a syntax error | ||
| // further along still wins, as on Amazon DynamoDB. |
There was a problem hiding this comment.
What does "wins" mean in this context?
|
|
||
| let mut args = Vec::new(); | ||
| if *pos < tokens.len() && tokens[*pos] != Token::RParen { | ||
| // `f()` is a syntax error on the service, reported at the `)`. |
There was a problem hiding this comment.
A syntax error because there are no arguments? Please clarify.
| /// Per-table capacity totals in the order the tables first appear in the | ||
| /// request. | ||
| /// | ||
| /// Amazon DynamoDB's own order of the `ConsumedCapacity` entries of a |
There was a problem hiding this comment.
I can't tell if this is saying ExtendDB's behavior (in terms of sorting) is now equivalent to DynamoDB's, or if ExtendDB is providing firmer ordering guarantees than DynamoDB (and why).
| }; | ||
|
|
||
| // Validate begins_with operand types upfront (before any rows are read). | ||
| if let Some(ref f) = filter { |
There was a problem hiding this comment.
Where are these checks happening now?
| } | ||
|
|
||
| // Validate begins_with operand types upfront (before any rows are scanned). | ||
| if let Some(ref f) = filter { |
There was a problem hiding this comment.
Similar question to query.rs: where are these checks happening now?
jcshepherd
left a comment
There was a problem hiding this comment.
Some of the agent-generated comments are read pleasantly but actually are ambiguous which makes it hard to determine whether the code is even solving a real problem. I flagged a couple. Also some operand validation code in query.rs and scan.rs was deleted and it's not obvious that it is still being done somewhere (assuming it was needed).
What
Issue #108 lists 106 requests whose response differs from Amazon DynamoDB. This change makes ExtendDB validate expressions and transactions in the service's order and return the service's messages for the groups below. Every expectation was measured against the service (us-east-1, 2026-10-14), and every new test file was run against the service before it was run against ExtendDB.
Semantic validation of expressions. After the grammar pass, the service walks the parsed expression once, in expression order, checking each node after its operands, then checks overlapping paths, then unused names and values. New
validate_update_semanticsandvalidate_condition_semanticsincrates/core/src/expression/semantics.rsfollow that walk. The parsers now do grammar only, keep function names as written, and record section order for the walk.+and-take numbers,list_appendtakes lists,ADDtakes a number or a set,DELETEtakes a set. A placeholder of the wrong type is refused with the service's type vocabulary (S,N,Lfor operators and functions;STRING,NUMBER,Binary,BOOL,NULL,LIST,MAPfor ADD and DELETE). Function names are case-sensitive:IF_NOT_EXISTS(x, :v)andfoo(x)areInvalid function name, a condition function isnot allowed in an update expression,list_appendis refused as an operand of a function or of+, both functions take two operands, and the first operand ofif_not_existsis a document path. A repeated section is reported after that section's own actions, soSET a = :v SET b = :qreports:q. Before this changeIF_NOT_EXISTS(x, :v),DELETE s :string, andSET a = :string + :nwere accepted and written.ATTRIBUTE_EXISTS(pk)was accepted and evaluated);if_not_existsandlist_appendarenot allowed in a condition expression; a boolean function in operand position andsizein condition position arenot allowed to be used this way; operand counts are fixed per function;attribute_existsandattribute_not_existstake a document path;begins_withtakes a string or binary andattribute_typea string naming a type, both checked at request time with the operation's prefix.Request stage on the read APIs. Query and Scan check the FilterExpression, then the KeyConditionExpression (with its
Invalid KeyConditionExpression:prefix and ahead of the key schema match, so an undefined#nis reported as such), then the ProjectionExpression, then unused names and values, all before the table is looked up. An undefined:qin a filter returned 200 on an empty table. Limit and Segment checks precede the expressions on Scan.Single-item order. UpdateItem checks the whole ConditionExpression before the UpdateExpression. PutItem, UpdateItem, and DeleteItem wrap every expression-stage error in
1 validation error detected:and omit the: ConditionExpression is nullsuffix; the read, batch, and transaction APIs return the messages bare. Theexpression size: Nsuffix follows the operation (present on a Scan FilterExpression and a transaction ConditionExpression, absent on a Query FilterExpression and the single-item ConditionExpression). Three existing dual-target tests asserted the unprefixed text and fail against the service; they are corrected.Transactions. TransactGetItems and TransactWriteItems validate in phases, each over every item before the next: per item, expressions (UpdateExpression before ConditionExpression), unused names and values, then the item's 400 KB size; then table lookup; then the 4 MB total; then key schema, as a cancellation with
ValidationErroron the first offending item only andNoneon every other item (every offender was flagged); then duplicates. An undefined reference is the request error the service returns rather than a per-item cancellation reason, and a key schema mismatch leaves the other items' conditions unevaluated. TransactWriteItems returns"ItemCollectionMetrics": {}when SIZE is requested and no table has a local secondary index, and its ConsumedCapacity entries are in first-appearance order rather than per-process hash order.Key-shaped messages. GetItem, DeleteItem, UpdateItem, BatchGetItem, and BatchWriteItem (Put and Delete) refuse a key with a missing, mistyped, or extra attribute with
The provided key element does not match the schema. PutItem keepsMissing the key pk in the itemandType mismatch for key. BatchWriteItem returns"ItemCollectionMetrics": {}under SIZE with no LSI.Table ARNs in
TableName(eight #108 rows) stay unsupported by decision: an ExtendDB table ARN names ExtendDB's account and region, so a table that streams or replicates to Amazon DynamoDB would answer to two ARNs.docs/differences-from-dynamodb.mdrecords this. Open PR #200 takes the opposite position and needs reconciling before either merges.Why
Closes part of #108. A client that maps on the service's error codes and messages, or that reads
CancellationReasons,ItemCollectionMetrics, orConsumedCapacity, saw different responses from ExtendDB on these requests, and three request shapes the service refuses were accepted and written.Of the 106 rows in #108, 83 now return the service's status, error type, and message (30 on
main). The 23 that remain fall into groups this change does not take on:Syntax error; token: X, near: Y; 14 rows);ResourceNotFoundmessage text (3 rows);1 validation error detectedmessage (2 rows: 101 transaction items, 26 batch writes).Also outside this change: the 4 MB transaction message, where the service's text carries its own serialized payload size; and
ConsumedCapacityentry order, which on the service follows no documented rule.Testing done
tests/test_expression_semantics.py(new, 155 cases): operand types, function names, the walk order, the unused set, condition function rules, Query and Scan request-stage checks, key-shaped messages, BatchWriteItemItemCollectionMetrics.tests/test_transact_validation_order.py(new, 31 cases): the transaction phases, first-offender cancellation, item size and 4 MB positions.tests/test_update_expression_rules.py(new, 37 cases): repeated sections, operands, overlap, the single-item prefix. Every expected response was captured from the service; all three files pass against the service and on SQLite, PostgreSQL, and MongoDB. 164 of the 186 cases in the first two fail onmain.test_transact_absent_item_isolationcases of fix(postgres): lock missing items that TransactWriteItems checks or deletes #387 unchanged). The remaining full-suite failures on a dev-mode SQLite server (metrics, import and export, gsi async, misc operations) are identical onmainand need infrastructure the dev server lacks.semantics.rs(10) pin every ordering rule the wire tests use.cargo test --workspace: 1,500 passed.cargo fmt --check;cargo clippy --workspace --all-targets -- -D warningson 1.97.0;cargo +1.88.0 check --workspace.main, one client, two runs (p50): TransactWriteItems with 100 Puts 23.8 to 23.9 ms against 24.0 to 24.1; UpdateItem with 50 SET actions 1.48 to 1.50 ms against 1.47; PutItem with a 20-clause condition 1.17 to 1.19 ms against 1.18; an UpdateItem with an invalid expression 0.88 ms against 0.88 to 0.89.Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Otherwise, an ADR captures the decision (link below).ADR / RFC: n/a. The change alters wire-visible error text and response members toward the live service, which CONTRIBUTING lists under the RFC trigger; it is a parity fix measured against the service rather than a protocol proposal. A maintainer's call on whether an ADR is wanted.
Breaking changes
Requests the service refuses and ExtendDB accepted are now refused:
UpdateExpressionfunction names in the wrong case,ADDorDELETEwith a value of the wrong type,+or-with a non-numeric placeholder,list_appendwith a non-list placeholder, condition function names in the wrong case, and an undefined:valueor#namein a Query or Scan filter (previously a 200 when no row was evaluated). Error messages for these groups change to the service's text. Key mismatch messages on the key-shaped APIs change toThe provided key element does not match the schema.ItemCollectionMetricsis present as{}on TransactWriteItems and BatchWriteItem under SIZE with no LSI. TheConsumedCapacityarray of both transaction APIs is in first-appearance order.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.