Repository navigation
fix(query): bound begins_with on a string sort key by the next prefix - #588
Open
LeeroyHannigan wants to merge 6 commits into
Open
LeeroyHannigan wants to merge 6 commits into
LeeroyHannigan wants to merge 6 commits into
Conversation
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 26, 2026 00:42
This was referenced Oct 26, 2026
… reach Closes #440. A Query ExclusiveStartKey from another partition, or outside the sort key condition, was accepted and answered an empty page. The service refuses it, with a sentence chosen by where the key falls. Measured on 2026-10-23 and 2026-10-24, every row repeated with identical answers, page-end rows ten times each. After the shape check, the service first asks whether a later key in the scan direction can still satisfy the sort key condition, for either partition: - On a base table a later key must differ from the start key's sort value: ">" and "<" at the bound are refused, ">=" at the bound is accepted, "<=" at the bound refused, BETWEEN at the upper bound refused and at the lower bound accepted, begins_with with a key that does not carry the prefix refused; the reverse direction mirrors each. - On an LSI or GSI a repeat of the start key's sort value counts as a later key, because index keys repeat: "<=", BETWEEN at either bound, ">=" in reverse, begins_with in reverse at the prefix and "=" with the key itself are all accepted; ">" and "<" at the bound are refused. - The sentence is "The provided starting key does not match the range key predicate", with two base-table exceptions in the same partition: "=" with the key's own value: "The provided exclusive start key is invalid"; begins_with in reverse with the key exactly the prefix: "The provided starting key is outside query boundaries based on provided conditions". Then the partition, when the key's partition differs from the condition's (table, LSI, GSI, legacy KeyConditions, either direction): - no sort key condition: "The provided starting key is outside query boundaries based on provided conditions"; - any sort key condition: "The query can return at most one row and cannot be restarted". A hash-only base table refuses every ExclusiveStartKey with "The provided exclusive start key is invalid", the queried partition included, and its pages never carry a key. Partition values compare by value for numbers (1, 1.0, 1e0, 01, +1 are equal); strings by UTF-8 byte order; binary by byte order. Page end: when a base table page ends on the last key that can satisfy the sort key condition in the scan direction, the service returns no LastEvaluatedKey (ten of ten for "=", BETWEEN at the upper bound, "<=" at the bound, reverse ">=" at the bound). ExtendDB now drops the key there, so a client that feeds every returned key back is never refused. Index pages are unchanged. Ordering, measured: filter and projection faults, unused names and values, ConsistentRead on a GSI, Limit, a missing table or index, the key size check and the start key shape check (missing or extra attributes, wrong types) are reported first; the Select checks after. The checks live in crates/engine/src/start_key.rs, called from handle_query after the shape check. The sort key predicate is evaluated with the expression evaluator on the start key's sort value, so every operator and type follows the same comparison as the condition itself. Tests: engine unit tests in start_key_tests.rs and start_key_range_tests.rs; tests/python/test_query_start_key_partition.py, test_query_start_key_range.py and test_query_start_key_index.py (162 rows, the hash-only page row ExtendDB only) pass against the service and on SQLite and PostgreSQL, with test_read_paging.py and the Query wire files. Pagination walks with Limit 1 to 3 in both directions feed every returned key back without a refusal. Pre-existing, measured, not changed here: a sort key bound of another type than the sort key answers InternalServerError (the type check in #429); the service omits the trailing key when the next row exists but fails the condition ("sk < z" with Limit equal to the matches), which needs a storage look-ahead and is filed separately.
…y sentence matches the other backends (cherry picked from commit 0294106)
…key bound A Query with a sort key condition and an ExclusiveStartKey on the same partition produced two restrictions on the same side of the clustering column (`sk_s >= ? AND sk_s > ?`), which Cassandra refuses with "More than one restriction was found for the start bound on sk_s" (end bound in reverse; "cannot be restricted by both an equality and an inequality relation" on the index same-sort-value sub-query). The server answered InternalServerError. The defect exists on main for every such request. The Cassandra Query path now builds the sort key range as at most one lower and one upper bound (SkRange), replaces the bound on the near side of the scan direction with a strict bound at the start key when the start key is at or past it, and renders the range once per CQL statement. The index two-query path runs its same-sort-value sub-query only when the start key's sort value lies inside the condition's range, and renders the merged range for the advance sub-query. Binds follow the rendered clauses in order. Unit tests pin the merge for a lower bound, a bound equal to the start key, a start key short of the bound, BETWEEN in both directions, equality, begins_with in reverse, strict and inclusive containment, and numeric ordering. The three Query start key wire files pass on Cassandra (162 rows), as do test_query_scan, test_composite_keys, test_gsi, test_scan_edge_cases and the storage direct_integration suite.
…roduces Equality renders as one clause, BETWEEN as two inclusive bounds, and begins_with as an inclusive prefix with an exclusive upper bound: the prefix plus U+10FFFF for strings, the incremented last byte below 0xFF for binary, and no upper bound for an all-0xFF prefix.
LeeroyHannigan
force-pushed
the
fix/440-start-key-partition
branch
from
October 29, 2026 23:54
ec0aab6 to
dd5c7ed
Compare
Query with begins_with(sk, :p) on a string sort key read the range sk >= p AND sk < p + U+10FFFF on Cassandra, SQLite and PostgreSQL. A key that starts with p followed by U+10FFFF (p + U+10FFFF, p + U+10FFFF + "b") sorts at or above that bound and was left out. Amazon DynamoDB returns those keys on base tables, GSIs and LSIs (measured 2026-10-25 at 22:31, 22:35 and 23:20 UTC, each request twice), ordered by their UTF-8 bytes, which is how the three backends compare string sort keys. The new extenddb_storage::util::string_prefix_upper_bound returns the prefix with its last character replaced by the next scalar value: trailing U+10FFFF characters are dropped first, and U+D7FF is followed by U+E000. A prefix that is empty or all U+10FFFF has no upper bound, and the three backends then read from the prefix up. MongoDB already computed the same bound in its own next_string_prefix and is unchanged. On PostgreSQL the bound is now a bound parameter instead of `$p || chr(1114111)`, so build_sk_sql takes the expression maps to know whether the placeholder exists, and bind_sk_condition binds the bound it computes from the same function. A numeric prefix on a numeric sort key is refused in build_sk_sql before any SQL is built, so the placeholder and bind counts cannot disagree; it answered InternalServerError before and still does. On Cassandra the old string_upper_bound leaves query.rs.
Dual-target wire tests from measurements on 2026-10-25 at 22:31 and 22:35 UTC: begins_with with prefixes ending in a, U+10FFFF, U+D7FF and U+FFFF, forward and reverse; Limit 1 pages over the prefixes a, a + U+10FFFF and U+10FFFF; and two BETWEEN ranges whose bounds sit next to U+10FFFF. The service returns every key that starts with the prefix, in UTF-8 byte order.
LeeroyHannigan
force-pushed
the
fix/cassandra-begins-with-bound
branch
from
October 30, 2026 10:04
38d70bf to
ab43c05
Compare
LeeroyHannigan
requested review from
eliseong,
hitesh96db and
navareamol
as code owners
November 3, 2026 09:40
This branch has not been deployed
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.
Part of #489 (the Cassandra
begins_withrow). Stacked on #483, which moves the Cassandra sort key range intosk_range.rs.A Query with
begins_with(sk, :p)on a string sort key left out every key that starts with the prefix followed by U+10FFFF, on Cassandra, SQLite and PostgreSQL. It now returns them, as DynamoDB does. The #489 row says SQLite and PostgreSQL return the item; on the base of this branch both excluded it as Cassandra did, so all three change.What happened
The three backends read
begins_with(sk, p)as the rangesk >= p AND sk < p + U+10FFFF. A key such asp + U+10FFFForp + U+10FFFF + "b"starts withpand sorts at or above that bound, so it was not returned:string_upper_boundincrates/storage-cassandra/src/data/query.rsappended\u{10FFFF}.sk < (? || char(1114111)).sk COLLATE "C" < ($p || chr(1114111)).MongoDB already computes the next prefix in
next_string_prefixand is not changed.What the service answers
Measured on 2026-10-25 at 22:31, 22:35 and 23:20 UTC in us-east-1, each request sent twice with the same answer. On sort keys
a,ab,a+ U+D7FF,a+ U+E000,a+ U+FFFF,a+ U+10000,a+ U+10FFFF,a+ U+10FFFF +b,a+ U+10FFFF + U+10FFFF,b, U+10FFFF, U+10FFFF +x, U+10FFFF + U+10FFFF andz,begins_withwith the prefixesa,a+ U+10FFFF,a+ U+10FFFF + U+10FFFF, U+10FFFF, U+10FFFF + U+10FFFF,a+ U+D7FF anda+ U+FFFF returns exactly the keys that start with the prefix, in UTF-8 byte order, forward and reverse.Limit1 pages over the prefixesa,a+ U+10FFFF and U+10FFFF return the same keys in the same order.BETWEENwith a bound next to U+10FFFF includes both bounds. A GSI and an LSI with a string sort key answer the same way, including keys written twice.The change
extenddb_storage::util::string_prefix_upper_boundreturns the prefix with its last character replaced by the next scalar value. Trailing U+10FFFF characters are dropped first, and U+D7FF is followed by U+E000. When the prefix is empty or all U+10FFFF there is no upper bound, and the backends read from the prefix up. Cassandra and SQLite already did that for an all-0xFF binary prefix; PostgreSQL's binary bound is unchanged.SkRange::from_conditionuses it; the oldstring_upper_boundis removed fromquery.rs.build_sk_sql_and_bindsbinds the bound as a second value, or emits the lower bound alone.build_sk_sqlnow takes the expression maps, so it knows whether the upper bound placeholder exists, and returns aResult;bind_sk_conditionbinds the bound computed by the same function. A numeric prefix on a numeric sort key would passparse_skand bind one value for two placeholders, sobuild_sk_sqlrefuses it before building any SQL. That request answered InternalServerError on the base and still does; the service refuses it with a ValidationException, which is fix(query): classify and order key conditions on a multi-attribute GSI the way the service does #429's area.Tests
crates/storage/src/util/prefix.rs: the bound for ten prefixes, and a check that the range holds every one of the measured keys that starts with the prefix and no other.crates/storage-cassandra/src/data/sk_range.rs: the same check throughSkRange::contains, and fix(query): refuse a Query ExclusiveStartKey the key condition cannot reach #483's pinnedbegins_withbound updated toab<acplus the unbounded U+10FFFF prefix.crates/storage-postgres/src/data/query.rs: the fragment and placeholder count for a bounded and an unbounded string prefix, and the refusal of a numeric prefix on a numeric column.tests/test_query_begins_with_max_character.py: 19 dual-target wire tests from the measurements.Each of these altered versions of
string_prefix_upper_boundfails tests in both crates: appending U+10FFFF again, treating a trailing U+10FFFF as unbounded, and mapping U+D7FF to U+10FFFF. Dropping the numeric refusal fails the PostgreSQL unit test, and a SQLite build that binds a second value for an unbounded prefix failstest_begins_with_pages_through_keys_after_the_prefix[max], which stops after 15 pages instead of looping.Run locally on 2026-10-25:
tests/test_query_scan.pyandtests/test_fgac_leading_keys.py, 162 passed at the first revision of the new filetests/test_query_scan.pyandtests/test_fgac_leading_keys.py, 164 passedOn SQLite the 9 that fail on the base are begins_with with
a,a+ U+10FFFF and U+10FFFF in both directions and theLimit1 walks over those three prefixes; each prefix matches a key that continues with U+10FFFF. The 10 that pass are the other four prefixes in both directions, each matching only the key equal to the prefix, and the two BETWEEN tests.On Cassandra, 16 GSI pagination tests in
tests/test_query_scan.pyand one LeadingKeys test failed locally at the head and on the base alike.devtools/run-testsappliesindex_propagation_delay_ms = 0and two other test settings only when./target/release/extenddbexists, and these runs used binaries from another path; with those settings applied all of them pass at the head.Also run:
cargo +1.97.0 fmt --all -- --check,cargo +1.97.0 clippy --all-targets -- -D warnings, andcargo test --workspace --exclude extenddb-storage-cassandraon stable and on 1.88.0 (1,808 passed each), pluscargo test -p extenddb-storage-cassandra --lib(71 passed) andcargo test -p extenddb-storage-postgres --lib(55 passed). The Cassandra integration tests were not run locally; their helper connects to 127.0.0.1:9042. CI runs the new file inrun-integration,run-integration-sqlite,mongodb-pytestandcassandra-pytest.Files
crates/storage-postgres/src/data/query_scan.rswas 761 lines and grows by five, because rustfmt splits thebuild_sk_sqlcall over several lines once it takes four arguments. No other file over 600 lines grows;crates/storage-cassandra/src/data/query.rsloses six.Merging
This is based on #483's branch. After #483 merges, retarget it to
main.git merge-tree --write-treeagainst the open PRs that touch these files adds five conflicts. #429 callsbuild_sk_sqlwith three arguments, so whichever lands second passesmapsand adds?. The other four are adjacent lines incrates/storage/src/util/mod.rs(#401, #474, #543) and theuseline incrates/storage-postgres/src/data/query.rs(#368); keep both sides.