Azure / Azure/azure-sdk-for-rust

Cosmos: Make ORDER BY envelope pages binary-aware (stop transcoding every document ~3x)

Open
#5,283 0 comments 1 reaction 1 assignee Claimed by @ananth7592 View on GitHub
binary-encoding Cosmos
Dominant language
Rust
Stars
884
Forks
365
Avg merge
2d 19h
Merged PRs (30d)
109

Description

## Summary

A binary-negotiated cross-partition `ORDER BY` query currently transcodes every document it fetches up to **three times** before the caller sees it, and the bytes the caller receives are **not** the service's original bytes — they are driver-re-serialized text that has been re-encoded back to binary.

This is tracked as item 2 in the "Pending work" table of
[`sdk/cosmos/docs/specs/0014-binary-encoding-high-level-design.md`](../../sdk/cosmos/docs/specs/0014-binary-encoding-high-level-design.md).

**This is an efficiency and byte-fidelity issue, not a correctness gap** — values are preserved. It is nonetheless customer-observable (see [Observable side effects](#observable-side-effects)).

## Background

Cross-partition `ORDER BY` uses the service's *rewritten envelope* shape. Each backend page is `{"_rid": ..., "Documents": [{"_rid": ..., "orderByItems": [...], "payload": {...}}, ...]}`, and the driver k-way-merges rows across partitions before emitting an output page.

The driver only needs `orderByItems` and `_rid` to order rows. The `payload` is opaque to the merge — it is carried through untouched, logically speaking.

## Current behavior

Per backend page, on the binary-negotiated path:

| # | Step | Where |
| - | ---- | ----- |
| 1 | Whole page binary → text | [`normalize_page_body`](../../sdk/cosmos/azure_data_cosmos_driver/src/driver/dataflow/query_response.rs) (called from `parse_envelope_page`) |
| 2 | `serde_json` envelope parse into `RawFeedBody` / `EnvelopeItem` | [`parse_envelope_page`](../../sdk/cosmos/azure_data_cosmos_driver/src/driver/dataflow/query_response.rs) |
| 3 | Per-item text → binary re-encode | [`PageAggregator::encode_item`](../../sdk/cosmos/azure_data_cosmos_driver/src/driver/dataflow/query_response.rs) |

Step 1 decodes the *entire* page even when the query emits only a subset of its rows. Step 3 then re-encodes each surviving payload from the canonicalized text.

Callers of `parse_envelope_page`:

- [`streaming_ordered_merge.rs`](../../sdk/cosmos/azure_data_cosmos_driver/src/driver/dataflow/streaming_ordered_merge.rs)
- [`non_streaming_ordered_merge.rs`](../../sdk/cosmos/azure_data_cosmos_driver/src/driver/dataflow/non_streaming_ordered_merge.rs)

### Observable side effects

`EnvelopeRow`'s doc comment states the consequence directly: on the text path `payload` keeps the item's exact original JSON bytes (via `RawValue`), but on the binary-negotiated path the page is transcoded to text first, so `payload` reflects **canonicalized** text — normalized key order, collapsed duplicate keys, canonical number formatting.

Note this also applies to a **text** page absorbed into a binary-negotiated query: the emitted encoding follows the *negotiated operation*, never the bytes of the absorbed page, so those items get re-encoded too.

The waste is worst for selective queries: an `ORDER BY ... TOP n` fetches a page from every partition and fully transcodes each one, then discards most rows. Roughly, wasted work scales with `fan_out x page_size` while useful output is `n`.

## Proposed change

Make the envelope reader binary-aware so it decodes **only** `orderByItems` and `_rid`, and keeps each `payload` as a **view**: the refcounted page `Bytes` plus an offset into it.

`Reader::new(buf, pos)` already supports reading from an arbitrary offset ([`binary_json/reader.rs`](../../sdk/cosmos/azure_data_cosmos_driver/src/binary_json/reader.rs)), so the machinery exists.

Emitted items would then be the service's original bytes, and pages whose rows are never emitted would never be transcoded.

## :warning: Critical constraint — do not slice-and-re-prefix

**Do not** slice a document out of a binary page and re-prefix it with `0x80`.

Reference strings (`STR_R1`..`STR_R4`) resolve against **absolute page offsets** — see [`Reader::resolve_reference`](../../sdk/cosmos/azure_data_cosmos_driver/src/binary_json/reader.rs), whose `target` is "an absolute byte offset into the buffer... where the `PREAMBLE` is offset `0`". The string-interning scope is the **whole page**.

A detached sub-slice therefore mis-resolves any reference pointing outside it. The failure mode is **silent**: whenever the target bytes happen to begin with a string marker, it returns *wrong text* rather than an error. This is exactly why the fix must retain the page and use offset views rather than detached buffers. A row buffered for the merge pins its entire source page alive (peak retention roughly `fan_out x page_size`) — an accepted trade-off that should be measured.

## Invariant future feed splitters must preserve

`skip_take_page::split_feed_envelope` detects a binary envelope, transcodes it, and splits it into **text** payloads; `skip_take_page::encode_items` then re-encodes each surviving document **standalone**, so every `ResponseBody::Items` producer emits per-document binary that `into_items` auto-detects by preamble.

Any change here must keep that invariant: slicing a single-preamble envelope without re-encoding per document yields preamble-less sub-documents that get misrouted to the text path.

Ordering matters too. `SkipTake` splits, applies its window, and only then encodes the survivors — so a document the window discards costs no transcode. A splitter that encodes at split time pays for every document the page carried.

## Acceptance criteria

- [ ] A binary-negotiated `ORDER BY` no longer transcodes an entire backend page to text solely to read `orderByItems` / `_rid`.
- [ ] Emitted items on the binary path are the service's **original bytes** (byte-for-byte), not re-serialized text re-encoded to binary.
- [ ] Rows discarded by a `TOP` / `OFFSET`/`LIMIT` window are never transcoded.
- [ ] No document is ever produced by slicing a page and re-prefixing `0x80`; reference strings continue to resolve against the full page.
- [ ] `ResponseBody::Items` consumers (SDK + native FFI) build unchanged, or the API change is reviewed and the FFI header regenerated.
- [ ] `request_text_response` still yields text items on a binary wire.
- [ ] Text pages absorbed into a binary-negotiated query still round-trip correctly.
- [ ] Peak memory is measured: a buffered row pins its source page, so report
retention under a wide fan-out.

## References

- HLD pending work item 2 + "Detail: item 2, binary-aware `parse_envelope_page`":
[`0014-binary-encoding-high-level-design.md`](../../sdk/cosmos/docs/specs/0014-binary-encoding-high-level-design.md)
- [`0015-binary-encoding.md`](../../sdk/cosmos/docs/specs/0015-binary-encoding.md)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.