fix(dynamodb): order Query by full composite sort key - #2114
Conversation
Query ordered results using only the first RANGE attribute returned by getSortKeyName(). For an index with a composite sort key (more than one RANGE attribute, e.g. "state RANGE, createdAt RANGE"), the remaining components were ignored: when the first component tied across rows, the stable sort left items in base-table storage order and ScanIndexForward =false merely reversed that order, so the secondary component (e.g. createdAt) never drove ordering. Sort by the full sort key instead, comparing each RANGE attribute in key-schema order and short-circuiting on the first non-equal component, then reverse for ScanIndexForward=false. Single-sort-key indexes are unaffected (one-element list preserves previous behavior). Adds getSortKeyNames() to GlobalSecondaryIndex, LocalSecondaryIndex and TableDefinition, and a regression test whose base-key order deliberately disagrees with the secondary sort component. Fixes floci-io#1675
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbService.java | Query ordering now compares every composite sort-key component, while pagination emits and consumes a uniquely identifying cursor. |
| src/main/java/io/github/hectorvent/floci/services/dynamodb/model/GlobalSecondaryIndex.java | Adds ordered retrieval of all RANGE attribute names for GSI query ordering. |
| src/main/java/io/github/hectorvent/floci/services/dynamodb/model/LocalSecondaryIndex.java | Adds ordered retrieval of all RANGE attribute names for LSI query ordering. |
| src/main/java/io/github/hectorvent/floci/services/dynamodb/model/TableDefinition.java | Exposes all table RANGE attribute names while retaining the existing first-sort-key accessor. |
| src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbCompositeSortKeyQueryIntegrationTest.java | Adds wire-protocol coverage for full composite ordering, reverse ordering, cursor shape, and complete page traversal. |
| src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbServiceTest.java | Adds service-level regression coverage for ascending and descending composite GSI ordering. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Q[Query request] --> K[Resolve table or index key schema]
K --> F[Filter matching and non-expired items]
F --> S[Compare all RANGE attributes in schema order]
S --> D{ScanIndexForward false?}
D -->|Yes| R[Reverse ordered results]
D -->|No| P[Keep ascending order]
R --> C[Apply ExclusiveStartKey]
P --> C
C --> L[Apply Limit or response-size cutoff]
L --> E[Emit cursor with index components and base-table keys]
Reviews (3): Last reviewed commit: "fix(dynamodb): paginate composite sort-k..." | Re-trigger Greptile
|
Thank you for this, and for fixing the cursor path along with the visible ordering symptom, the pagination collapse for rows sharing the first range value was the subtler half of the bug. One context note rather than a change request: a multi range key schema is a Floci extension, real DynamoDB caps KeySchema at one partition plus one sort key, so this improves behavior on a surface AWS itself would reject at CreateTable. Since the single sort key path is preserved by construction and the extension already exists, that is fine here, it is just worth both of us knowing where the deviation line sits. (blocking) Commit cf8bfbc carries an AI co-author trailer; this repo keeps commit attribution with human contributors (see AGENTS.md). Would you amend that commit and force push so the attribution is yours alone? That attribution fix is the only blocker, the code itself is ready. |
The composite-sort-key ordering fix sorted by every RANGE attribute, but
pagination still used only the first RANGE component (getSortKeyName()).
LastEvaluatedKey construction and ExclusiveStartKey matching therefore
dropped the later components, so a cursor no longer uniquely identified the
row a page stopped on — rows sharing the first RANGE value could be skipped
or duplicated across pages.
Thread the full sort-key list (getSortKeyNames()) through buildKeyNode() and
buildItemKeyFromNode(): the emitted cursor now carries every sort-key
component, and cursor matching compares the full composite key. Single
sort-key indexes and the scan/table-key paths keep the previous behavior via
the retained single-key overloads.
Addresses PR review comment r3710297799 ("Composite pagination loses key
identity"). Adds paginated regression tests over the composite index that
fail without this change. Refs floci-io#1675.
cf8bfbc to
a06ebd5
Compare
Summary
Query ordered results using only the first RANGE attribute returned by
getSortKeyName(). For an index with a composite sort key (more than one RANGEattribute, e.g.
state RANGE, createdAt RANGE), the remaining components wereignored: when the first component tied across rows, the stable sort left items
in base-table storage order and
ScanIndexForward=falsemerely reversed thatorder, so the secondary component (e.g.
createdAt) never drove ordering.Sort by the full sort key instead — comparing each RANGE attribute in
key-schema order, short-circuiting on the first non-equal component, then
reversing for
ScanIndexForward=false. Single-sort-key indexes are unaffected(a one-element list preserves previous behavior).
Adds
getSortKeyNames()toGlobalSecondaryIndex,LocalSecondaryIndexandTableDefinition.Closes #1675
Type of change
fix:)feat:)feat!:orfix!:)AWS Compatibility
Incorrect behavior: a
Queryagainst an index with a multi-attribute compositesort key returned items that were not ordered by the full key. Ties on the first
sort-key attribute fell back to storage order, and
ScanIndexForward=falseonlyreversed that — so results diverged from DynamoDB, which orders by the entire
composite sort key. DynamoDB natively supports composite sort keys on secondary
indexes, so emulating full-key ordering is required for parity.
Verified with a wire-protocol (
DynamoDB_20120810.Query) integration test: a GSIkeyed
[memberName HASH, state RANGE, createdAt RANGE], items whose storageorder deliberately disagrees with
createdAt, asserting ascending order and theexact reverse for
ScanIndexForward=false.Checklist
./mvnw testpasses locally