Add streaming ORDER BY queries#4800
Conversation
Implement resumable cross-partition ORDER BY merging with exact Cosmos value ordering, durable continuation state, and split-safe recovery. Add cross-SDK scenario coverage, emulator validation, and live split tests.\n\nFixes Azure#4756\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 80371d09-63a0-4ebc-8bfc-6af45c217151
Replace the draft pull request placeholders with the upstream PR number.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 80371d09-63a0-4ebc-8bfc-6af45c217151
Resolve planner and Cosmos status-code conflicts while preserving upstream query-range normalization and streaming ORDER BY support.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 80371d09-63a0-4ebc-8bfc-6af45c217151
analogrelay
left a comment
There was a problem hiding this comment.
Some initial comments. Looking like a good start though! I still need to go through the actual merge processes so not ready to approve quite yet.
| Future node types (e.g., streaming ORDER BY, hybrid search) may require partial parsing of item bodies. The backend query plan can rewrite the query to use a standardized envelope (promoting ordering keys to top-level fields and demoting the raw user document to a `payload` field). This varied-shape pattern must be considered in the overall design direction, but does not need to be accommodated in the current implementation. | ||
| `StreamingOrderedMerge` (cross-partition `ORDER BY`) is the first node type that parses item bodies: the backend query plan's `rewrittenQuery` reshapes each per-partition result into a standardized envelope (`{"_rid": ..., "orderByItems": [...], "payload": ...}` — sort keys promoted to top-level fields, the raw user document demoted to `payload`). The pipeline parses just enough of this envelope to compare rows and emit `payload` — see `driver::dataflow::order_by` (the value/comparator/resume-value model) and `driver::dataflow::query_response` (envelope parsing and response reassembly). Hybrid search, if implemented, would need the same pattern for its own envelope shape. | ||
|
|
||
| Resume for streaming `ORDER BY` is per-range and topology-change-safe. Each still-active range persists its last-emitted key tuple and envelope `_rid`. On resume — including after a partition split or merge — a **scalar** boundary is turned into a key-only seek filter (`... > boundary OR ... = boundary`) that returns every row at or after the boundary, including the full-key tie run; a `_rid` predicate is deliberately not part of this SQL, since `_rid` compares as an ordinal base64 string in Cosmos SQL, not the backend's numeric document order. The already-emitted prefix of that tie run is instead discarded client-side by a numeric `_rid` comparison (see `models::resource_id::compare_document_rids`), so resume stays exact per row and splits/clips to sub-ranges without any positional row count. An **array/object** (complex) sort key is persisted only as a bounded 128-bit hash, so it cannot form a seek filter; it resumes by a whole-range positional re-scan, which is valid only while the range's topology is unchanged — a resume that would cross a split is rejected with a typed error (`CosmosStatus::CLIENT_STREAMING_MERGE_COMPLEX_BOUNDARY_TOPOLOGY_CHANGE`) rather than risk silently dropping rows. |
There was a problem hiding this comment.
An array/object (complex) sort key is persisted only as a bounded 128-bit hash, so it cannot form a seek filter;
Is that what other SDKs do? Just want to make sure we're not introducing an arbitrary limitation here that other SDKs don't have. In general, I think array/object sort keys are an anti-pattern anyway, so I'm not really that worried.
|
|
||
| /// Ascending Cosmos type rank. Must match | ||
| /// `crate::query::eval::sort_type_order`. | ||
| fn type_rank(item: &OrderByItem) -> u8 { |
There was a problem hiding this comment.
nit: I'd make this a function in impl OrderByItem, but that's purely aesthetics given that this is completely private to the module.
| /// Compares an `i64` with a finite `f64` exactly. `2^63` is exactly | ||
| /// representable, so it bounds the range where `floor` fits an `i64`; the | ||
| /// fractional part then breaks a floor-tie without any lossy cast. | ||
| fn cmp_i64_f64(a: i64, b: f64) -> Ordering { |
There was a problem hiding this comment.
Is this something other SDKs do? Do we need to support it?
| /// arrays compare lexicographically; objects by key, value, then | ||
| /// length. No canonical intra-type order is documented for | ||
| /// arrays/objects — only internal determinism is promised. | ||
| pub(crate) fn cosmos_cmp(&self, other: &Self) -> Ordering { |
There was a problem hiding this comment.
I think it'd be fine to just make this an impl Ord for OrderByItem. The whole point of this type is to canonically sorts items using the same algorithm Cosmos uses.
|
|
||
| /// A bounded 128-bit hash of a complex (array/object) `ORDER BY` value, | ||
| /// split into low/high halves for JSON round-tripping (mirrors .NET's | ||
| /// complex-key encoding); hashes a canonical encoding so key order doesn't |
There was a problem hiding this comment.
Ah, ok, so this mirrors .NET 👍🏻
|
|
||
| /// One of the seven Cosmos type-rank buckets, used to render `IS_*` | ||
| /// builtin guards for cross-type-aware resume filters. | ||
| fn type_check_builtin(rank: u8) -> &'static str { |
There was a problem hiding this comment.
Might want to make a CosmosType enum to represent the rank, so that you don't have the unreachable! panic.
| // One collision-safe parameter per column carrying a bound value; | ||
| // `Undefined`/`Null` need none. Names share a prefix no caller | ||
| // parameter starts with, so a resume binding can't overwrite one. | ||
| let prefix = collision_free_prefix(existing_parameter_names); |
There was a problem hiding this comment.
It'd be cheaper, if it's valid, to use a UUID (rendered without hyphens) as the prefix: @cosmosResult_[uuid]_[counter]. Then we don't really have to worry about collisions, nor scan the existing names.
| operation: &Arc<CosmosOperation>, | ||
| ) -> crate::error::Result<()> { | ||
| let query_text = query_response::query_text(operation.body())?; | ||
| let program = crate::query::parse(&query_text).map_err(|source| { |
There was a problem hiding this comment.
Since this depends on our query parser, do we do it in other SDKs? I'm a little wary of bringing it in here.
| /// Handles a child's `SplitRequired` result: ignores `Request`'s own | ||
| /// replacement nodes (unsafe to combine a forwarded continuation with |
There was a problem hiding this comment.
The intent of the SplitRequired result carrying the new children was to allow the parent to be ignorant of the specific child nodes below it. Maybe that's just unnecessary abstraction though.
Summary
ORDER BYexecutionTesting
cargo test -p azure_data_cosmos_driver --all-featurescargo clippy -p azure_data_cosmos_driver --all-features --all-targets -- -D warningscargo clippy -p azure_data_cosmos --all-features --all-targets -- -D warningsFixes #4756