Add `RowWriter` sink API so consumers receive string/binary payloads without `mssql-tds` allocating (row decode perf, axis: value handoff)

[saurabh500]

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 (pg_fabric) 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 Remove per-row dispatch overhead from the row-decode path #264.

API

Six methods added to RowWriter, every one with a default implementation, so existing writers compile and behave identically without edits.

/// 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<u8> 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<u8>)	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:

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::<u8>();
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<u8> 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<u8> — 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: POC: reduce per-row cost in the TDS row decode path #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<u8>> 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.

[saurabh500]

Benchmarked: the sign of this change depends on the writer

Measured using the in-crate decode microbenchmark described in the results comment on #247. The spike is c38e5683, measured against the preceding commit eb64a9d1 so the effect is isolated from #252.

Against DefaultRowWriter — a regression

Case	Before (ms)	After (ms)	Δ
39 INT + 9 VARCHAR(6)	58.8	60.7	+3.3%
same, every 4th column NULL	49.4	51.2	+3.7%
8 × VARCHAR(512)	23.3	26.4	+13.1%

This is inherent, not an implementation defect. DefaultRowWriter produces Vec<ColumnValues>, so each value must end up in its own owned allocation either way. commit_value does mem::take(&mut self.scratch), handing the buffer away and leaving nothing to reuse, and reserve must zero-fill what it returns because safe Rust cannot hand out uninitialized memory. Net effect versus the old bytes.to_vec(): same allocation, same copy, plus a zero-fill. The larger the string, the worse it gets — hence +13% on VARCHAR(512).

Against a contiguous-buffer writer — a solid win

To measure the case this API actually targets, I added a ContiguousRowWriter that appends every value into one reusable Vec<u8> — the shape mssql-js's BinaryRowWriter uses. On the old trait it receives an owned SqlString/Vec<u8> (so the decoder must assemble a temporary buffer first and the writer copies again); on the new trait the decoder writes straight into the writer's buffer.

Case	base	after #252	after #253	#253's contribution
48 cols	128.2	45.4	39.2	−13.7%
8 × VARCHAR(512)	36.7	21.3	14.8	−30.5%

Another way to see it: on the old trait, switching from DefaultRowWriter to a contiguous writer buys only ~10% (153.3 → 138.2), because the writer is still forced to receive owned, separately-allocated values. On the new trait the same switch buys ~37% (61.6 → 39.2). The accumulator API is what lets a contiguous writer actually be contiguous.

What this means for the PR

1. Benchmark this on mssql-js, not mssql-tds. The in-crate number understates the change by design — the crate's own writer is the one shape that cannot exploit it. The triple-buffering this issue was filed to remove lives in BinaryRowWriter.
2. Consider keeping write_str/write_blob as a fast path. They cost nothing to retain and let DefaultRowWriter avoid paying for an API it cannot use. Without that, every native Vec<ColumnValues> consumer regresses 3–13% to benefit the FFI consumers.
3. Frame it as a trade, not a free win. It is very likely the right trade given where the row volume is, but the PR should say so explicitly rather than claim a uniform improvement.

Caveat carried over from #247: this workload has no PLP/BLOB data at all. The chunked reserve path — where the old API's cost is worst, since it buffers fragments and reassembles — is not exercised here. Re-benchmark against wide VARBINARY(MAX)/NVARCHAR(MAX) before claiming the full win.

[saurabh500]

Porting findings here from #260, which duplicated this issue and is being closed.

Agreeing first with the measured conclusion in the benchmark comment: keeping write_str/write_blob as a fast path is right. In the #260 design they fall out for free, because the one-shot forms are provided methods over the sink. An implementor that cannot exploit reserve never implements it, and DefaultRowWriter keeps its current behaviour. The trait grows 25 to 29 methods, and the 20 fixed-length methods are untouched.

write_xml / write_json / write_vector need no signature change

Grep-verified: they are never called from decoder.rs. Their only call site is the ColumnValues replay fan-out at row_writer.rs:257-259. XML reaches the writer as a string via plp_encoding() -> Utf16Text -> write_string. That keeps the diff off three methods that look like they need it.

Two defects worth fixing inside this change rather than separately

• sql_string.rs:60 does String::from_utf8(self.bytes.clone()).unwrap(). That is a live panic on invalid UTF-8, plus a redundant clone. Should be from_utf8_lossy.
• decoder.rs:1020 sizes its pre-allocation from the declared PLP length with no clamp, and MAX_PLP_SIZE is i32::MAX. A corrupt or hostile declared length can request a ~2 GB allocation up front. Wants a PREALLOC_CAP (1 MiB is a reasonable start), growing as chunks actually arrive.

EncodingType needs Copy

For ValueKind::Str(EncodingType) to be ergonomic. It currently derives only PartialEq, Clone. It also lacks Debug, which is mildly painful in tests.

Settled semantics from the #260 design

Offered as input rather than instruction:

• Infallible. A sink that cannot satisfy a reservation has no useful recovery, and a Result would infect every arm of the decode match.
• LenHint::Exact is trusted for pre-sizing, which is only sound if the PLP validation keeps checks 1 (stream did not end early) and 6 (stream did not overrun its declared length). That is the real dependency between this issue and Cleanups surfaced by row decode perf work (PLP validation, cfg-gated duplication, sentinels) #258.
• commit_value is mandatory. With LenHint::Unknown, the end of a value cannot be inferred from the reservation sequence alone.
• Row-level abandon_row, not a value-level abort_value. There is no case where one value is abandoned while the row continues.
• reserve must return exactly n (a short slice silently desynchronizes the stream) and must return initialized memory: Vec::resize, never with_capacity + set_len, which is UB once handed out as &mut [u8]. The zero-fill cost measured above is the price of that second invariant, and it is not avoidable in safe Rust.

Why the FFI side is where this pays

Concretely, a PLP nvarchar(max) reaching mssql-js today is three allocations:

1. read_plp_bytes fills plp_buffer (decoder.rs:1020)
2. to_utf8_string() runs UTF_16LE.decode().into_owned() (sql_string.rs:61-65)
3. string_map.insert(s.clone(), idx) (binary_row_writer.rs:82)

plus a UTF-16 -> UTF-8 -> UTF-16 round trip crossing into Node. That is the shape behind the -30.5% on contig_wide_strings.

One copy is the floor, not zero. Under any buffered path the slice handed to the writer points into working_buffer, which the next read_tds_packet() overwrites, so the value must be copied out before the next read.

Open question worth a look while here

Whether the mssql-js interner should be bypassed above some size threshold. It is roughly a third of the js-side cost and is a poor trade for large values, which rarely repeat. That is separable from this API but is measured by the same mssql-js benchmark this issue should carry.

[saurabh500]

Design verdict: keep the pull model, drop reserve(&mut [u8])

The issue's goal is right, but the proposed begin_value / reserve / commit_value shape is over-built for the return it provides. In particular, reserve(col, n) -> &mut [u8] forces every implementation to initialize n bytes before the transport immediately overwrites them. That is not incidental: it explains the measured DefaultRowWriter regression, and the exact-length slice contract adds protocol/desynchronization surface that exists only because the decoder is asking the writer to manufacture initialized storage.

I am going to implement the simpler destination-Vec variant:

fn begin_value(
    &mut self,
    col: usize,
    kind: ValueKind,
    hint: LenHint,
) -> Option<&mut Vec<u8>> { None }

fn commit_value(&mut self, col: usize) {}
fn abandon_row(&mut self) {}
fn wants_values(&self) -> bool { true }

The decoder will append PLP chunks to the returned writer-owned Vec<u8>. NetworkTransport will gain an internal append operation that copies buffered packet bytes with extend_from_slice, so no zero-filled destination is created. Other packet readers retain a safe fallback. The existing owned write_string(SqlString) / write_bytes(Vec<u8>) methods stay unchanged as the one-shot path, and non-PLP values keep using them; this avoids adding calls to the narrow/common row shape.

Why this is a better trade:

• DefaultRowWriter can hand out the final SqlString.bytes / ColumnValues::Bytes allocation. Compared with today's vec![0; declared_len], it keeps the same allocation and transport copy but removes the full zero-fill, so wide PLP values can improve instead of regress.
• BinaryRowWriter can return row_data for blobs (after writing its tag/length placeholder) and a scratch Vec for strings, preserving the direct contiguous write win.
• PyRowWriter can use the same final owned allocation as DefaultRowWriter.
• The trait remains dyn-compatible; all new methods have object-safe signatures and defaults.
• DiscardRowWriter remains a zero-sized unit struct by returning false from wants_values; PLP framing is still parsed and validated, but payload chunks are consumed through skip_bytes.
• The existing PlpChunkStreamReader becomes the single validator for materialize, append, and discard paths. I will not add another PLP validation loop.
• abandon_row remains part of the change. The transport boundary can invoke it on decode errors, timeout, or cancellation after the inner future is dropped, making the rollback hook cancellation-safe for BinaryRowWriter.

I am intentionally applying the destination API to PLP values first. That is the workload this issue requires us to prove, and retaining the current one-shot path for short strings/blobs gives the fixed/narrow negative control the best chance of remaining neutral.

[saurabh500]

Pre-committed benchmark decision rule

This rule is posted before running any benchmark and will not be relaxed after seeing results.

Prediction

The writers have different mechanisms and therefore different expected ceilings:

• DefaultRowWriter: no copy is removed. Today a known-length PLP value allocates, zero-fills N, then copies N bytes from the packet buffer. The candidate allocates capacity and appends those packet bytes without first setting the vector length. That changes value traffic from roughly 3N to 2N (allocation/memset/copy to allocation/copy). A modest gain is expected, not the prior contiguous-writer result.
• No cross-row reuse is available for DefaultRowWriter. take_row() moves the row vector and every value allocation to the caller. Changing that ownership contract is out of scope.
• Contiguous/FFI-shaped writer: PLP blobs also remove the second plp_buffer -> row_data copy, so the expected gain is materially larger. PLP strings retain a reusable writer scratch buffer because transcoding/interner lookup still require complete input.
• DiscardRowWriter: wants_values=false skips payload bytes while still parsing and validating PLP framing. This is a changed/optimized cell and will be reported as a bonus, not used as the negative control.

Method

• Base: dev/saurabh/sync-buffered-reads pinned at 2bfac1d4c284f565dc4269e9b1cf7bacbb63d4c8 (the current stacked base), never main.
• In-crate harness driving the real receive_row_into_internal path through concrete NetworkTransport over pre-built, packetized in-memory TDS input.
• --release, separately compiled hash-distinct base/candidate binaries.
• 3 warmups + 9 measured passes per cell; 4 alternating base/candidate runs; compare the median of the four per-run medians.
• Report base/candidate medians, paired delta, and distribution-overlap status for every cell.
• The temporary harness will not be committed.

Workloads

1. PLP-heavy VARBINARY(MAX): four 64 KiB columns per row, chunked across normal TDS packet boundaries.
2. PLP-heavy NVARCHAR(MAX): four 64 KiB wire-byte columns per row, likewise chunked.
3. Untouched negative control: 39 INT + 9 non-PLP VARCHAR(6) columns.

Each workload will run with DefaultRowWriter; the two wide workloads and control will also run with a contiguous FFI-shaped writer. The wide discard cells will be reported separately but are not gates.

Ship/drop bars

Ship only if all of these hold:

• DefaultRowWriter is at least 3% faster in each wide PLP cell, with the candidate faster in all four alternating rounds. This is what counts as “DefaultRowWriter gains.”
• The contiguous writer is at least 10% faster for PLP VARBINARY(MAX) and at least 5% faster for PLP NVARCHAR(MAX).
• The non-PLP control regresses by no more than 2% for either writer, and there is no non-overlapping regression distribution.
• Correctness checks pass and both binaries consume the complete input and produce identical row/value counts.

If any bar fails, the recommendation is drop the optimization and do not open a PR. The only permitted rerun is to correct a demonstrated harness/build defect; the thresholds will not be changed and proxy metrics (future size, struct size, forced inlining) will not be substituted for throughput.

Cancellation scope is also narrowed from my design comment: the transport invokes abandon_row after an operation timeout/cancel or decode error returns through that boundary. Arbitrary caller-driven dropping of the outer row future is not claimed to run rollback code.

[saurabh500]

Decision-rule amendment (adds coverage; no bar is relaxed)

This amendment is also posted before the first timing run.

Added small-PLP regression gate

Add a MAX-declared, known-length PLP workload with 24 columns × 128 payload bytes × 20,000 rows. It is intentionally the weak-benefit case: two extra writer calls per value compete with removing initialization of only 128 bytes. It will run with both DefaultRowWriter and the contiguous writer and is gated exactly like the untouched control: no more than 2% regression and no non-overlapping regression distribution.

Cover both PLP framing shapes

The wide matrix is expanded to both wire forms for both data types:

1. known-length VARBINARY(MAX)
2. SQL_PLP_UNKNOWNLEN VARBINARY(MAX)
3. known-length NVARCHAR(MAX)
4. SQL_PLP_UNKNOWNLEN NVARCHAR(MAX)

Each wide cell remains 4 columns × 64 KiB wire bytes × 256 rows (about 64 MiB of payload per pre-built input), chunked across normal packet boundaries. Both DefaultRowWriter and the contiguous writer are measured. Wide discard cells remain report-only.

This distinction matters. The known-length baseline uses vec![0u8; n] once. The unknown-length baseline grows with resize(..., 0) per chunk and can also reallocate/copy accumulated bytes, so it has a larger removable cost. Claims will be reported separately by framing rather than generalized from one to the other.

Allocator caveat / corrected prediction

vec![0u8; n] may use alloc_zeroed; it is not necessarily a literal user-space allocation followed by memset. For sufficiently large fresh mappings, zero pages can make initialization cheap. The 64 KiB workload is below the typical large-allocation mmap regime and is therefore favorable to observing the removed initialization, but the result remains allocator- and size-dependent. The unknown-length resize(..., 0) cost is an explicit per-growth initialization and does not have the same caveat.

Accordingly, the earlier 3N -> 2N description is only a memory-traffic upper-bound model, not a universal prediction. The report/PR, if any, will scope results to the measured sizes and allocator/platform.

Remaining row counts and page-fault discipline

• wide known/unknown cells: 256 rows (about 64 MiB payload per input)
• small PLP cell: 20,000 rows (about 61 MiB payload)
• 39 INT + 9 non-PLP VARCHAR(6) control: 30,000 rows

The input is built once outside timing. The three warmup passes traverse the complete buffer before the nine measured passes, so measured samples do not include first-touch page faults for the pre-built input.

All previously posted ship/drop bars remain unchanged, including DefaultRowWriter ≥3% faster in every wide cell, contiguous ≥10% binary / ≥5% nvarchar, faster sign in all four wide rounds, and drop/no PR on any failed gate.

[saurabh500]

Decision-rule amendment 2 (final added cells; no bar changes)

Posted before timing, incorporating an isolated Windows allocation probe as hypothesis only—not end-to-end evidence.

Small PLP now covers both framings

The 24 columns × 128 B × 20,000 row small-PLP workload will run in both:

• known-length PLP
• SQL_PLP_UNKNOWNLEN

Both framings run with DefaultRowWriter and the contiguous writer. The existing small known-length gate remains ≤2% regression with no non-overlapping regression. The unknown-length small cell is added as a measured/reportable cell; it does not relax or replace that gate.

Prediction recorded before measurement: known-length 128 B is neutral-to-slightly-negative because begin_value/commit_value have little initialization cost to offset; unknown-length 128 B should improve because the baseline performs per-chunk resize(..., 0). If the framing results invert, I will investigate the implementation/harness rather than reinterpret the bars.

Added large-value curve point

Add a report-only 4 columns × 256 KiB × 64 rows cell (about 64 MiB payload) for known-length VARBINARY(MAX) and NVARCHAR(MAX), using both materializing writers. This cell is not a gate; it tests whether the benefit tapers once Windows allocations are served by already-zero OS pages.

Any PR claim will show the size curve rather than imply a universal large-value gain. The isolated primitive probe will not be cited as throughput evidence; only the real row-path results will be reported.

No existing ship/drop threshold changes. In particular, the 128 B known-length regression gate and every DefaultRowWriter wide-cell ≥3% bar remain binding.