Repository navigation
fix(query): bound begins_with on a string sort key by the next prefix - #588
Open
LeeroyHannigan wants to merge 2 commits into
Open
LeeroyHannigan wants to merge 2 commits into
LeeroyHannigan wants to merge 2 commits into
Conversation
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
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
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.