Problem statement
On the PLP text paths in SQLGetData (mssql-odbc/src/api/get_data.rs), a call whose caller buffer is entirely satisfied from already-transcoded carry still runs the full wire-read path: it takes the dbc lock, calls try_read_active_plp_chunk with an empty slice, and on Pending runs a block_on(read_active_plp_chunk(&mut [])) that can issue a real network read for the next chunk header.
Every byte such a "drain-only" call will deliver is already buffered (pending_utf8 for the UTF-16 to UTF-8 transcode path, pending_units for the narrow-to-wide widening path). So a transient read error or a query timeout on one of these calls returns SQL_ERROR and discards output that was already transcoded successfully. With a 2-byte buffer over a 200-byte value that is roughly 200 round trips through the connection lock, each an opportunity to lose the whole value.
This is a pre-existing shape (the widening path sizes its drain read as 1 rather than 0), not a regression. It became a common path in #505, which made small SQL_C_CHAR buffers legal for nvarchar(max).
Proposed solution
Short-circuit the drain-only case so it is purely local (no dbc lock, no wire read):
- Transcode path: when
max_read == 0 && utf8_carry_len > 0, deliver from pending_utf8 without reading.
- Widening path: the equivalent condition on
pending_units.
This needs reached_end (wire exhausted) persisted on ActivePlpStream in mssql-odbc/src/handles/stmt.rs, so the reached_end && !converted_data_still_held teardown still fires on the final drain call without re-reading the wire. Care is needed around the length-probe shape (buffer_length == 1, payload_capacity == 0) and the prefetch path, which carries its own reached_end. Add unit coverage for the drain-only short-circuit (teardown timing and indicator/01004 reporting).
Affected crate
mssql-odbc
Alternatives considered
Leave as-is: correctness is fine today (the drain eventually completes), the cost is extra lock traffic and a wider window for a transient failure to discard buffered output. Given small SQL_C_CHAR buffers are now legal, the short-circuit is worth doing.
Additional context
Raised by David Engel (@David-Engel) as a non-blocking review suggestion on #505 (thread on get_data.rs utf16le_max_read). Deferred here to keep #505 (the #211 fix) focused, and because a proper fix should cover both the transcode and widening paths consistently.
Problem statement
On the PLP text paths in
SQLGetData(mssql-odbc/src/api/get_data.rs), a call whose caller buffer is entirely satisfied from already-transcoded carry still runs the full wire-read path: it takes the dbc lock, callstry_read_active_plp_chunkwith an empty slice, and onPendingruns ablock_on(read_active_plp_chunk(&mut []))that can issue a real network read for the next chunk header.Every byte such a "drain-only" call will deliver is already buffered (
pending_utf8for the UTF-16 to UTF-8 transcode path,pending_unitsfor the narrow-to-wide widening path). So a transient read error or a query timeout on one of these calls returnsSQL_ERRORand discards output that was already transcoded successfully. With a 2-byte buffer over a 200-byte value that is roughly 200 round trips through the connection lock, each an opportunity to lose the whole value.This is a pre-existing shape (the widening path sizes its drain read as
1rather than0), not a regression. It became a common path in #505, which made smallSQL_C_CHARbuffers legal fornvarchar(max).Proposed solution
Short-circuit the drain-only case so it is purely local (no dbc lock, no wire read):
max_read == 0 && utf8_carry_len > 0, deliver frompending_utf8without reading.pending_units.This needs
reached_end(wire exhausted) persisted onActivePlpStreaminmssql-odbc/src/handles/stmt.rs, so thereached_end && !converted_data_still_heldteardown still fires on the final drain call without re-reading the wire. Care is needed around the length-probe shape (buffer_length == 1,payload_capacity == 0) and the prefetch path, which carries its ownreached_end. Add unit coverage for the drain-only short-circuit (teardown timing and indicator/01004 reporting).Affected crate
mssql-odbc
Alternatives considered
Leave as-is: correctness is fine today (the drain eventually completes), the cost is extra lock traffic and a wider window for a transient failure to discard buffered output. Given small
SQL_C_CHARbuffers are now legal, the short-circuit is worth doing.Additional context
Raised by David Engel (@David-Engel) as a non-blocking review suggestion on #505 (thread on
get_data.rsutf16le_max_read). Deferred here to keep #505 (the #211 fix) focused, and because a proper fix should cover both the transcode and widening paths consistently.