microsoft / microsoft/mssql-rs
Add `RowWriter` sink API so consumers receive string/binary payloads without `mssql-tds` allocating (row decode perf, axis: value handoff)
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
Give `RowWriter` implementations a way to receive string and binary payloads **without `mssql-tds` allocating for them**, so an external consumer can act as a direct sink for wire bytes and place them straight into its own final representation.
The driving consumer is a **PostgreSQL FDW** that queries SQL Server and materialises results as PG datums. It needs the encoded bytes plus the encoding descriptor; where the encoding permits, it stores them verbatim and lets PostgreSQL own transcoding.
## Non-goals
- **`DefaultRowWriter` is not required to benefit.** It may keep its existing owned `write_string`/`write_bytes` path unchanged. Unifying the two was an aspiration, not a requirement.
- Not a rework of `ColumnValues`, `take_row()`, or row-level buffer reuse.
- Not the dispatch/boxing axis — that landed in #264.
## API
Six methods added to `RowWriter`, **every one with a default implementation**, so existing writers compile and behave identically without edits.
```rust
/// Writes packet-backed encoded string bytes before the packet buffer is reused.
fn write_string_ref(&mut self, col: usize, bytes: &[u8], encoding_type: &EncodingType) {
self.write_string(col, SqlString::new(bytes.to_vec(), encoding_type.clone()));
}
/// Writes packet-backed binary bytes before the packet buffer is reused.
fn write_bytes_ref(&mut self, col: usize, bytes: &[u8]) {
self.write_bytes(col, bytes.to_vec());
}
/// Returns a destination for a known-length binary value, when the consumer
/// can allocate its final representation before the decoder reads the bytes.
fn bytes_destination(&mut self, _col: usize, _length: usize) -> Option<&mut [u8]> {
None
}
/// Completes or abandons a destination returned by `bytes_destination`.
fn finish_bytes_destination(&mut self, _col: usize, _success: bool) {}
/// Returns a destination for a known-length character value, when the consumer
/// can allocate its final representation before the decoder reads the bytes.
///
/// `length` counts encoded bytes on the wire, not characters, so a consumer
/// whose storage differs in length under `encoding_type` returns `None`.
fn string_destination(
&mut self,
_col: usize,
_length: usize,
_encoding_type: &EncodingType,
) -> Option<&mut [u8]> {
None
}
/// Completes or abandons a destination returned by `string_destination`.
fn finish_string_destination(&mut self, _col: usize, _success: bool) {}
```
`&mut [u8]` rather than `&mut Vec` is deliberate and load-bearing — see *Why the destination is a slice* below.
## How the decoder selects a path
Three tiers, tried in order, per value:
| Tier | Path | Applies when | Cost to consumer |
|---|---|---|---|
| 1 | `*_destination` → fill → `finish_*_destination` | length known before the payload is read, and the writer returns `Some` | **0 copies, 0 `mssql-tds` allocations** |
| 2 | `write_*_ref(&[u8])` | length unknown (`PLP_UNKNOWNLEN`), encoding needs transcoding, or the writer declined tier 1 | 1 copy, 0 `mssql-tds` allocations |
| 3 | `write_string(SqlString)` / `write_bytes(Vec)` | writer overrides nothing | unchanged from today |
`finish_*_destination(col, success)` carries the outcome, so a decode error, timeout, or cancellation mid-value lets the consumer release or invalidate the storage it handed out. Only one destination may be outstanding at a time; the `col` argument lets the consumer assert that.
## Why the destination is a slice, not a `Vec`
The consumer's final storage is **not** on the Rust heap. The FDW allocates a PostgreSQL `varlena`, writes the length header, and returns a slice over the payload region:
```rust
let varlena = pg_sys::palloc(allocation_size) as *mut pg_sys::varlena;
pgrx::set_varsize_4b(varlena, allocation_size as i32);
let data = addr_of_mut!((&mut **varattrib_4b).va_data).cast::();
self.pending_bytes = Some((col, Datum::from(varlena as usize)));
Some(std::slice::from_raw_parts_mut(data, length))
```
Consequences:
- `palloc` returns **uninitialised** memory, so there is **no zero-fill**. The objection that a reserve-style API forces a `memset` applies only to a `Vec`-backed writer.
- The decoder writes into the value's **final** location. A `&mut Vec` would force the consumer to copy Rust heap → `palloc` afterwards, which defeats the purpose.
- The consumer is already in `unsafe` FFI territory and constructs the slice itself.
## Why `DefaultRowWriter` cannot regress
It overrides none of the six methods, so it is **structurally excluded** from the new paths rather than merely measured as neutral. Any nonzero delta on `DefaultRowWriter` indicates accidental coupling elsewhere and should be treated as a defect, not a tradeoff.
This is the decisive difference from the earlier `begin_value`/`commit_value` attempt, which changed the path all writers took and regressed `DefaultRowWriter` by up to **+20.9%** on small values.
## Transcoding
`EncodingType` is passed to `string_destination` and `write_string_ref` so the consumer decides:
- **`Utf8`** — accept the bytes verbatim after a validity check; no transcode.
- **`LcidBased`** — if the payload is ASCII, accept verbatim (below `0x80` every single-byte code page agrees with UTF-8); otherwise transcode.
- **`Utf16`** — the UTF-8 output length is not known from the wire length, so tier 1 is declined (`None`) and the value arrives via `write_string_ref`.
## Prerequisite: borrowed payload slices
Tier 2 requires handing out `&[u8]` borrowed from the packet buffer. `TdsPacketReader` currently exposes only scalar reads, `read_bytes(&mut [u8])`, and `read_u8_varbyte() -> Vec` — there is no slice-returning API. That capability has to be added, and it is the highest-risk part of this work: lifetimes against buffer reuse, and values that span packet boundaries.
Tier 1 needs no new reader capability; `read_bytes(&mut [u8])` already fills a caller-owned slice and works across packet boundaries.
## Scope
In scope: the six `RowWriter` methods, decoder tier selection for string/binary values, and the packet-reader borrow capability.
Out of scope: `next_row_into_unboxed` and the token-stream/dispatch changes carried in the reference branches — that axis landed in #264 and measured no additional gain on its own.
## Measurement gate
Any implementation must be measured **on current `main`**, not on the reference branches. Both are based on `dc8c62d1`, which predates #264; that change alone took ~61.6% out of this path, and #269 demonstrated a change that looked neutral pre-#264 and cost +21.5% after.
Ship criteria:
- The FFI/sink writer gains materially on `VARBINARY(MAX)` and `NVARCHAR(MAX)`.
- `DefaultRowWriter` measures flat (expected exactly 0%, since it takes no new path).
- **Non-PLP values do not regress.** A prior attempt showed an unexplained **+4.1%** on the non-PLP FFI control across all rounds, on a path that gained no sink calls. Reproduce or rule this out before proposing a PR.
## Reference implementations
- Library side: `NivasSA/mssql-rs` @ `poc/tds-row-decode-optimizations` (`9df51f01`), based on `dc8c62d1`. Aspirational PR: #238.
- Consumer side: `pg_fabric` @ `poc/tds-decode-fdw-usage` (`ee3d434`) — `src/pg_fabric_fdw/src/fdw/writes.rs` implements `PgDatumWriter`.
Both are proofs of concept and not merge candidates.
## History
This issue previously proposed a `reserve(col, n) -> &mut [u8]` accumulator, which was revised at review to `begin_value -> Option<&mut Vec>` and closed after that variant regressed `DefaultRowWriter`. The revision was the error: a `Vec` destination cannot write into consumer-owned memory, so the measured design was incapable of delivering the goal above. The original slice-based shape was correct. Reopened with the design restated to match the real consumer.
Parent: #247.
Contributor guide
Research direction
Start by locating RowWriter, the decoder paths for string and binary values, and TdsPacketReader, including read_bytes and read_u8_varbyte. Compare the reference implementations only for context, then measure current main; done means sink gains for VARBINARY(MAX) and NVARCHAR(MAX), no DefaultRowWriter or non-PLP regression, and correct handling across packet boundaries.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, rust
- Domain
- backend-api-design, database, performance
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100