fix: KeyConditionExpression rejects parenthesized sub-expressions#7
Merged
Merged
Conversation
Member
|
Nice - the nested and mixed parens coverage looks thorough. Reviewing alongside the other two. |
2bd42d4 to
233af0f
Compare
Member
|
Two small edits pushed to your branch:
As you were a first time contributor the auto CI checks needed approval. They should run now. Will merge once green. |
DynamoDB accepts parentheses in KeyConditionExpression — e.g.
(#pk = :pk) AND (#sk = :sk) or ((pk = :pk AND sk = :sk)). The parser
rejected them with "Expected attribute name, got (".
Fix: strip balanced outer parens from the token list before parsing,
and handle per-condition parens in parse_single_condition.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
233af0f to
9eb867e
Compare
Member
|
Quick follow-up: the earlier CI run failed on a pre-existing clippy lint on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
KeyConditionExpressionparser rejects parentheses around sub-expressions, returningValidationException: Expected attribute name, got (. Both DynamoDB and DynamoDB Local accept these expressions.ConditionExpressionhandles parentheses correctly — the bug is specific to theKeyConditionExpressionparser.Version: dynoxide 0.9.8
Reproduction
Root cause
parse_single_conditioninkey_condition.rsdoesn't handleToken::LParen— it immediately tries to parse an attribute path, which fails on(.Fix
Two levels of parenthesis support:
strip_outer_parensremoves balanced parens wrapping the entire expression before parsing (handles(pk = :pk AND sk = :sk)and((expr)))parse_single_conditioncounts and consumes wrapping parens around individual conditions (handles(pk = :pk) AND (sk = :sk))Unit tests cover: per-condition parens, outer parens, nested parens
((pk = :pk)), mixed nesting(((pk = :pk)) AND ((begins_with(...)))), and parens with#namereferences.Also submitted as a conformance test: nubo-db/dynamodb-conformance#1
Fixes #4