mssql-odbc: resolve SQL_C_DEFAULT in SQLGetData instead of rejecting it with HYC00

[David Engel (David-Engel)]

Description

SQLBindCol has accepted SQL_C_DEFAULT and resolved it per fetch from the IRD since AB#47481. SQLGetData still rejected it with HYC00, so the driver answered the same placeholder two different ways depending only on how an application read a column:

• SQLBindCol(col, SQL_C_DEFAULT, ...) + SQLFetch → resolves from the column's SQL type and delivers.
• SQLGetData(col, SQL_C_DEFAULT, ...) → HYC00.

msodbcsql has no such split — Sql2CDefault is consulted on the GetColData path that serves both — so this was an internal inconsistency rather than just a gap.

SQLGetData now resolves the placeholder through the same describe_col::odbc_sql_type → type_rules::resolve_default_c_type pair the other two directions use. Resolution happens once, ahead of the captured / PLP / continuation dispatch, so every delivery path sees one concrete C type for a column. The declared ODBC version was not previously reachable in get_data.rs; it is read from the ENV before the STMT lock to preserve parent-before-child lock ordering, following bind_param.rs, catalog.rs, and fetch_scroll.rs. The read is gated on the placeholder, which costs nothing here because TargetType is a scalar argument rather than something that has to be read out of the bindings first.

Behaviour

Unchanged for any caller that names a C type — the non-SQL_C_DEFAULT branch is a literal identity.

The placeholder is kept (and the existing target gate reports HYC00) in two cases:

• The column's SQL type has no default — SQL_UNKNOWN_TYPE for a TDS type this driver does not map.
• The resolved target is fixed-width and the caller's BufferLength cannot hold it. A SQL_C_DEFAULT retrieval names nothing, so honouring the C type's width would put the 16 bytes a uniqueidentifier resolves to into a 4-byte buffer. SQLFetchScroll refuses the same shape.

BufferLength 0 is deliberately not exempt here, unlike in the bound resolver. On a binding, 0 is the documented idiom for a fixed-width slot and makes no width claim; on SQLGetData it is also how an application asks for a length without wanting a value written — which is exactly how this file already reads it for SQL_C_BINARY. Exempting it would let the driver write up to 20 bytes into a buffer the caller declared as holding none, in a process the driver was loaded into. Refusing costs an application nothing it had before, since the same call answered HYC00 prior to this change, and the SQL_C_BINARY length probe still works because an application-sized target has no fixed width to test.

Resolved SQL_C_BINARY is still not delivered (AB#47239), and how much of it answers depends on the path — worth stating precisely, because the two differ:

• Non-PLP varbinary(n) / binary(n): the zero-length length probe works; only a read with a real buffer is HYC00.
• PLP varbinary(max) / image: even the probe is HYC00, because stream_active_plp_chunk admits only SQL_C_CHAR / SQL_C_WCHAR and rejects everything else before it looks at BufferLength. NULL is the exception — it never enters the streaming path.

Both are pre-existing for an explicit SQL_C_BINARY read, the same posture the bound path took in AB#47481; resolution only makes them reachable without the application naming the C type.

fetch_scroll::element_stride becomes pub(crate) so both SQL_C_DEFAULT resolvers test a resolved fixed-width target against the caller's declared slot through one definition of the C-type widths.

Tests

Unit tests in get_data.rs:

• Resolution from the column type (int → SQL_C_SLONG).
• The wide-column deviation (nvarchar → SQL_C_WCHAR, indicator in UTF-16 bytes).
• A mutation check on the ODBC version, as the work item asks for: a time column resolves to SQL_C_SS_TIME2 at 3.8 and SQL_C_BINARY below it. Verified that hardcoding the version read fails the 3.0 arm, so this cannot pass if the version stops being read.
• A column type with no default stays unresolved → HYC00.
• A fixed-width target too wide for the buffer is refused with nothing written past the declared length.
• BufferLength 0 on a fixed-width target is refused; the SQL_C_BINARY probe still answers.

E2E tests in tests/e2e/tests/get_data_test.cpp, all run locally against SQL Server 2025 in Docker (64 passed, alongside fetch_scroll and fetch_error_semantics for regressions):

• DefaultTargetResolvesFromTheColumnType — int and narrow varchar, the two types both drivers resolve identically, so it runs on the msodbcsql leg and compares.
• DefaultTargetStreamsAVarcharMaxAcrossChunks — the PLP continuation path, also comparable.
• DefaultTargetResolvesWideAndGuidToTypedTargets, DefaultTargetTooNarrowForItsFixedTargetIsRefused, DefaultTargetOnABinaryColumnIsStillUnimplemented, DefaultTargetOnABinaryMaxColumnRefusesEvenTheProbe, DefaultTargetStreamsAnNvarcharMaxAsWideChunks — assert registered deviations, so they carry SKIP_IF_COMPARING_MSODBCSQL().

Each deviation was measured against a real msodbcsql18 while writing the tests rather than assumed — that is how the wide column's three-narrow-bytes/indicator-3 result and the narrow-buffer GUID's 22003 (after writing inside BufferLength and consuming the column) were established. Those local measurements were against 18.6.1.1; CI pins 18.6.2.1, so the compare leg is the authority, and it has run green on this head.

DefaultTargetOnABinaryMaxColumnRefusesEvenTheProbe asserts that the defaulted spelling gives the same answer as naming SQL_C_BINARY explicitly, plus one assertion that today's shared answer is HYC00. The agreement is what this change owns; the HYC00 belongs to AB#47239, so when that lands only the one literal needs revisiting. It uses a separate result set per spelling because a PLP column whose stream was begun and then refused cannot be re-read on the same row.

Docs updated: mssql-odbc/docs/typed-columnar-fetch-plan.md (the note that flagged this gap as untracked now points at the work item) and the registered deviations in .github/instructions/mssql-odbc.instructions.md.

Related Issues

https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/47815

Checklist

• cargo bfmt passes
• cargo bclippy passes
• cargo btest passes (mssqlodbc: 1246 passed; the mssql-tds integration-test failures are pre-existing on this machine, which has no .env for a live server — confirmed by stashing the change)
• New/changed functionality has tests
• Public API changes are documented

[Copilot]

🟡 Changes recommended

The new streaming tests can exercise buffered delivery instead of the intended PLP path.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Resolves SQL_C_DEFAULT in SQLGetData using column metadata and ODBC version.

Changes:

• Adds default C-type resolution and fixed-width buffer protection.
• Shares fixed-width sizing logic with bound fetches.
• Adds unit/E2E coverage and documents deviations.

File summaries

File	Description
mssql-odbc/src/api/get_data.rs	Implements resolution and tests.
mssql-odbc/src/api/fetch_scroll.rs	Exposes shared stride helper.
mssql-odbc/tests/e2e/tests/get_data_test.cpp	Adds default-target E2E tests.
mssql-odbc/docs/typed-columnar-fetch-plan.md	Documents behavior.
.github/instructions/mssql-odbc.instructions.md	Records parity deviations.

Review details

Suppressed comments (1)

mssql-odbc/tests/e2e/tests/get_data_test.cpp:1912

• This payload only exceeds the buffer at the default 8,000-byte packet size. The test harness accepts connection-string overrides, and with a supported 32,768-byte packet size the 18,000-byte UTF-16 value can be captured whole, making this “streaming” test vacuous. Size it above twice the maximum packet size as well.

    ASSERT_SQL_OK(ExecDirect("SELECT REPLICATE(CAST(N'A' AS NVARCHAR(MAX)), 9000)"),

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 98%

🎯 Overall Coverage 93.4%

📦 Project: mssql-tds + mssql-odbc + mssql-py-core ℹ️ Note: diff coverage is reported, not enforced.

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql-odbc/src/api/fetch_scroll.rs (100%)
• mssql-odbc/src/api/get_data.rs (98.6%): Missing lines 157-158,4564
• mssql-odbc/src/api/type_rules.rs (100%)

Summary

• Total: 233 lines
• Missing: 3 lines
• Coverage: 98%

mssql-odbc/src/api/get_data.rs

  153     // lock-order inversion to avoid.
  154     let default_odbc_version = if target_type == SQL_C_DEFAULT {
  155         let env = stmt.parent_dbc().parent_env();
  156         let Ok(env_state) = env.inner.lock() else {
! 157             error!("SQLGetData: env mutex poisoned");
! 158             return SQL_ERROR;
  159         };
  160         Some(env_state.odbc_version)
  161     } else {
  162         None

---

  4560         let s = sh.inner.lock().unwrap();
  4561         assert!(
  4562             s.diag_records.is_empty(),
  4563             "a NULL must not raise a diagnostic: {:?}",
! 4564             s.diag_records
  4565         );
  4566     }
  4567 
  4568     #[test]

---

---

🔗 Quick Links

View Azure DevOps Build · Coverage Report

[ttk (Theekshna)]

Automated note: this review was generated by an authorized unattended run.

This PR resolves SQL_C_DEFAULT for SQLGetData from column metadata plus the declared ODBC version, reuses the fixed-width slot check from the bound-fetch path, and adds unit/e2e coverage plus divergence docs. I checked the reviewed head against the full worktree diff, prior review threads, and the posted coverage report, then independently verified one finding by reading stream_active_plp_chunk and the new e2e test file directly.

Severity	Count
Blocking	0
Suggestion	1
Nit	0

Blocking: none.

Suggestion:

• mssql-odbc/src/api/get_data.rs:362 — the new doc comment says a varbinary/image column "still serves only as the zero-length length probe" once resolved to SQL_C_BINARY. That is only true on the captured-value path. For a PLP-sized column (VARBINARY(MAX), image), resume_row_to_column returns CursorColumn::PlpStreaming, and stream_active_plp_chunk rejects any target other than SQL_C_CHAR/SQL_C_WCHAR with HYC00 before it ever looks at buffer_length (get_data.rs:746). So a zero-length probe against a non-NULL VARBINARY(MAX) column also returns HYC00, not the length the doc claims. I confirmed the new e2e test DefaultTargetOnABinaryColumnIsStillUnimplemented (get_data_test.cpp:1853) only exercises VARBINARY(8), and NullVarbinaryMaxToBinaryTargetReportsNull (get_data_test.cpp:1604) only covers the NULL leg, which never reaches the PLP-streaming branch — so the MAX/non-NULL case is untested. I'm keeping this as a Suggestion rather than Blocking: the underlying HYC00-for-PLP-binary behavior is pre-existing and already tracked (AB#47239, and this same file says so explicitly at get_data.rs:815-816 for the streaming path), so this PR does not regress anything a caller could previously do. The fix is scoping the doc comment to the non-PLP case (or adding the MAX-column caveat) and adding a regression test that pins today's HYC00 for a non-NULL VARBINARY(MAX) zero-length probe, so a future PR that actually implements the PLP-binary probe has something to flip red.

Nit: none.

Checks specific to this run: msodbcsql parity is not separately applicable here since the observed behavior matches the driver's own explicit posture (HYC00, tracked by AB#47239); the PR description/checklist match the diff and link AB#47815/AB#47239; gh pr checks is green and consistent with the checklist; no undocumented divergence found beyond what's already recorded in the new doc comments; no AI-slop found beyond the one overclaim above.

[ttk (Theekshna)]

Automated unattended run — comment-only, not checked by a human before posting.

Summary

Re-review triggered by head move. My prior review (9fb61ed3, findings=1, Suggestion) flagged that the doc comment at get_data.rs:362 overclaimed the SQL_C_BINARY zero-length probe worked uniformly, when stream_active_plp_chunk actually rejects any non-character target — including the probe — before it looks at buffer_length for a PLP (varbinary(max)/image) column.

Both commits since then (c25da708, 471933dd) fix exactly that:

• The doc comment at get_data.rs:362 (and the mirrored text in typed-columnar-fetch-plan.md and .github/instructions/mssql-odbc.instructions.md) now correctly splits non-PLP (varbinary(n): probe works) from PLP (varbinary(max)/image: probe also refused) — I independently re-read stream_active_plp_chunk (get_data.rs:761) and confirmed the target-type check (SQL_C_CHAR/SQL_C_WCHAR only) runs before any buffer_length inspection, matching the new claim exactly.
• A new e2e test, DefaultTargetOnABinaryMaxColumnRefusesEvenTheProbe (get_data_test.cpp), pins the previously-untested case: asserts a non-NULL VARBINARY(MAX) zero-length probe returns HYC00 for both the defaulted and explicit SQL_C_BINARY spellings, and that the two agree — which is the invariant this PR owns, correctly leaving the HYC00 itself to AB#47239.
• All three doc locations (inline comment, design doc, divergence registry) now say the same thing consistently.

No new issues found in the diff since the last review.

CI note (not a finding): coverage-report shows failed — I inspected the log rather than assuming a code problem: it's the GH Actions poller timing out after 75 minutes waiting for a Cobertura artifact from the ADO build (Coverage artifact not ready yet, 12 attempts, none ever appearing), while ADO's own Merge Coverage job passed. This looks like cross-pipeline artifact-handoff infra flakiness, unrelated to this PR's diff. Did not re-run it per this run's restrictions — flagging for a human to judge, per SKILL.md's guidance that a reviewer's infrastructure trouble isn't the author's problem.

Check	Result
msodbcsql parity	Already addressed correctly in the PR itself — Sql2CDefault on the shared GetColData path is cited as the reference behavior, and the driver's posture (HYC00 for PLP binary) is pre-existing and tracked (AB#47239), not a new divergence.
Test sufficiency	The gap from my prior review is now closed with DefaultTargetOnABinaryMaxColumnRefusesEvenTheProbe. No other untested branch found in the delta since the last review (both new commits are doc + one new test).
Divergence documented	Yes, consistently across all three locations now.
PR description currency	Matches diff; checklist matches gh pr checks modulo the coverage-report infra timeout noted above; AB#47815/AB#47239 linked.
AI slop	None — the new prose narrows an overclaim and adds a citation (decoder.rs's try_decode_buffered bail-out) rather than restating code.

Severity	Count
Blocking	0
Suggestion	0
Nit	0

No new findings — my prior Suggestion is resolved.

[David Engel (David-Engel)]

CI note (not a finding): coverage-report shows failed [...] This looks like cross-pipeline artifact-handoff infra flakiness, unrelated to this PR's diff. [...] flagging for a human to judge

Diagnosed and cleared — it was infra, but with a specific and reproducible cause worth recording rather than filing under general flakiness.

The poller latched onto ADO build 172055 in Status=cancelling:

✅ Found build: ID=172055, Status=cancelling, Result=unknown
📥 Polling for 'CoberturaCoverage' artifact for build 172055...
⏳ Coverage artifact not ready yet (attempt 1)... Available artifacts: none

172055 was the build for c25da708, cancelled the moment I pushed 471933dd about two minutes later. The workflow then polled that dead build for the full 75 minutes. The real build for this head, 172056, completed and published CoberturaCoverage normally — querying the two directly: 172055 returns 404 with no artifacts, 172056 lists CoberturaCoverage among 15.

So the handoff itself is fine; the poller just watched the wrong build. I re-ran the job and it found build 172056 and the artifact on attempt 1. All 19 checks are green on 471933dd, and the diff-coverage comment has reposted.

Confirming your read that it was unrelated to the diff, from two independent angles: PR Code Coverage passed on both preceding commits of this same branch (9fb61ed3, c25da708), and the only delta in 471933dd is a C++ comment in a test file.

Latent workflow bug, for someone to pick up separately. .github/workflows/pr-code-coverage.yml:155 only abandons a build when it is completed and failed:

if [[ "$STATUS" == "completed" && "$RESULT" == "failed" ]]; then

A cancelling/cancelled build passes that guard, so any PR with two pushes inside the build-discovery window burns 75 minutes and reports a false red. Widening the guard to treat cancelled builds as "keep looking" would fix it. I have deliberately not bundled that here — it is a shared CI file and unrelated to this PR's change, so it wants its own PR rather than riding along on an ODBC fix.

[Shiwani Gupta (shiwanigupta0809)]

Read the full diff, the head of get_data.rs, type_rules::resolve_default_c_type, element_stride, and the three doc locations. Checked the parity claims against the msodbcsql 18.6 source and the ODBC spec rather than the description. Not re-raising the binary-probe doc scoping or the PLP packet-size question — both are settled well.

Blocking: none. Suggestions: 1. Nits: 1.

Verified

The width guard has no hole. This is the part that would be a real bug if it were wrong, so I enumerated it rather than spot-checking. resolve_default_c_type can emit 17 distinct C types; element_stride gives a non-zero width for every one that has a fixed width (SQL_C_TYPE_DATE/SQL_C_TYPE_TIME 6, SQL_C_TYPE_TIMESTAMP 16, SQL_C_SS_TIME2 12, SQL_C_SS_TIMESTAMPOFFSET 20, SQL_C_GUID 16, and the scalars), and falls to the _ => buffer_length arm — which is 0 when called with 0 — for exactly the four the application sizes (SQL_C_CHAR, SQL_C_WCHAR, SQL_C_BINARY, SQL_C_SS_VECTOR). So there is no resolvable target that writes a fixed number of bytes while reporting stride 0 and slipping past fixed_width > 0. The struct sizes are right too. Making element_stride the single definition is what makes that checkable in one place.

Precision/scale from the ARD is moot here, correctly. The spec says that for SQL_C_DEFAULT and SQL_ARD_TYPE the data takes SQL_DESC_PRECISION / SQL_DESC_SCALE / SQL_DESC_DATETIME_INTERVAL_PRECISION from the ARD — which this resolver does not read. It does not need to: those fields only bite for SQL_C_NUMERIC and the interval types, and resolve_default_c_type emits neither (SQL_DECIMAL/SQL_NUMERIC go to SQL_C_CHAR). Worth knowing the gap is closed by the mapping rather than by accident, in case a future row adds SQL_C_NUMERIC.

Suggestion

SQLGetData has two placeholder TargetTypes, not one. This PR fixes SQL_C_DEFAULT; SQL_ARD_TYPE is still rejected with HY003 — the one SQLSTATE the spec defines as meaning "not SQL_ARD_TYPE".

SQL_ARD_TYPE (-99) does not exist anywhere in the crate. So it lands in the gate three lines above the new resolution:

if !is_valid_c_type(canonical_c_type(target_type)) {
    post_diag(&mut stmt_state, ERR_INVALID_C_DATA_TYPE);
    return SQL_ERROR;
}

canonical_c_type(-99) is the identity (-99 is outside SQL_C_DATE..=SQL_C_TIMESTAMP), -99 is not in the is_valid_c_type list, and ERR_INVALID_C_DATA_TYPE is HY003 / "Invalid application buffer type".

The spec's own definition of that code:

HY003 Program type out of range — (DM) The argument TargetType was not a valid data type, SQL_C_DEFAULT, SQL_ARD_TYPE (in case of retrieving column data), or SQL_APD_TYPE (in case of retrieving parameter data).

HY003 is the code for "TargetType was not one of {valid type, SQL_C_DEFAULT, SQL_ARD_TYPE}". Returning it for SQL_ARD_TYPE inverts the condition. And it is a driver-level answer, not a DM one — msodbcsql resolves SQL_ARD_TYPE itself, in GetColData, the same function this PR cites for Sql2CDefault:

// Else not a BOOKMARK column, check if the fCType is SQL_ARD_TYPE
if (fCType == SQL_ARD_TYPE)
{
    // Need to get type from ARD
    LPBINDINFO  lpbindinfo;
    if (lpstmt->pARD->lpplbindinfo == NULL || icol > CItemsPl(lpstmt->pARD->lpplbindinfo))
    { wError = IDS_S1_002; SETRC_SERR_GOTO(retcode, ErrorRet); }
    lpbindinfo = (LPBINDINFO)GetPlPtr(lpstmt->pARD->lpplbindinfo, icol);
    fCType = lpbindinfo->fCType;
    ...
}

— Sql/Ntdbms/sqlncli/odbc/sqlcdata.cpp:209. Note it also resolves the SQL_C_NUMERIC precision/scale from the same binding, which is the ARD-derived behaviour the spec describes.

Three reasons I think this belongs on this PR's radar rather than being noticed later:

• It is the same argument of the same function, in the same reference function. The PR opens by describing an internal inconsistency in how one placeholder is answered. After this change the driver answers SQL_C_DEFAULT consistently in all three directions, and still answers the other documented placeholder for the same argument with an error whose text says the application passed a bad buffer type.
• The refusal is not visible as a gap. HY003 reads as "you passed something invalid", so an application author checks their own code rather than filing a driver bug. HYC00 — what SQL_C_DEFAULT used to give, and what the deviations file uses elsewhere for tracked gaps — would at least say "the driver does not do this yet". If implementing it is out of scope, spelling it HYC00 and registering it alongside the other SQL_C_DEFAULT entries would make it a known gap instead of a silent one.
• The ARD is a live path here, not a hypothetical. SQL_ARD_TYPE is the spec's documented way to say "use the type I already set on the ARD" — and SQL_C_DEFAULT is itself settable into the ARD through SQLSetDescField. So the one spelling that reaches a descriptor-set type is the one that errors.

Not blocking: it is pre-existing, out of the stated scope, and nothing regresses. But it is unregistered in all three doc locations this PR touches, and the deviations file is the natural home for it.

Nit

The resolve_default_target doc says the placeholder is kept in two cases, "both of which the caller's own target gate then reports as HYC00". That holds for a non-NULL value; it does not for a NULL. write_captured_column's NULL branch (get_data.rs:1021) answers before deliverable_target is computed (:1058), so a NULL column with an unresolved SQL_C_DEFAULT returns SQL_NULL_DATA and SQL_SUCCESS.

That is the right answer — it is what msodbcsql returns too, since a NULL needs no buffer, so the narrow-buffer refusal correctly does not fire on a value that will not be written. Only the sentence over-reaches. Given the same doc already carves out the NULL exception for the PLP-binary case, adding it here would make the two read consistently.

[Shiwani Gupta (shiwanigupta0809)]

Reviewed - see #481 (comment)

[ttk (Theekshna)]

Unattended run found no new findings. Covered: msodbcsql parity for the SQL_C_DEFAULT resolution path (independently verified Sql2CDefault/GetColData at sqlcdata.h:254-256 and the SQL_ARD_TYPE citation at sqlcdata.cpp:209), the element_stride/width-guard logic, test mutation-resistance (ODBC-version-gated test, width-refusal tests), divergence documentation currency, PR description/checklist vs. green CI, and AI-slop in the new comments/tests — none found. Two open items from Shiwani Gupta (@shiwanigupta0809)'s prior review (SQL_ARD_TYPE HY003 gap, NULL-exception doc wording) remain unanswered; not re-filing.

Severity	Count
Blocking	0
Suggestion	0
Nit	0

[David Engel (David-Engel)]

Thanks Shiwani Gupta (@shiwanigupta0809) — both items were right, and I verified each against the code before acting rather than taking them on faith. Addressed in e768e68.

Nit: the NULL carve-out — fixed

Confirmed exactly as you describe. write_captured_column answers NULL at get_data.rs:1049 and returns, while deliverable_target is not computed until :1058, so an unresolved placeholder over a NULL never reaches either refusal.

The sentence now carries the carve-out, and says why it is right rather than just noting it: the refusals exist to prevent a write, and a NULL writes nothing, so both "target this driver cannot deliver" and "buffer too narrow to hold it" are moot. That also makes it read consistently with the PLP-binary NULL exception a few lines down, which was your point.

I also added get_data_default_over_a_null_reports_null_even_when_unresolvable, because this combination was untested and my change is what makes it reachable. It deliberately uses the shape the width guard would refuse — a uniqueidentifier resolving to the 16-byte SQL_C_GUID with BufferLength 4 — so if anyone ever moves the NULL branch below the gate, the test goes red instead of the behaviour silently changing to HYC00. It also asserts the 16-byte backing array is untouched and that no diagnostic is posted.

Suggestion: SQL_ARD_TYPE — registered, deferred

You're right on all three counts, and I measured it rather than reasoning from the source: SQLGetData with TargetType -99 returns SQL_ERROR / HY003 today. canonical_c_type(-99) is the identity, -99 is in neither is_valid_c_type nor anywhere else in the crate.

The spec-inversion argument is the part that convinced me this needed recording now rather than later: HY003 is defined as TargetType not being a valid type, SQL_C_DEFAULT, or SQL_ARD_TYPE — so returning it for SQL_ARD_TYPE tells an application it passed a bad buffer type when it passed a documented one. And as you note, it reads as "fix your code," so nobody files it.

What I did not do: re-spell it as HYC00. I considered that, since it's a one-line-ish change and would at least name it as unimplemented. I decided against it because your own citation argues for a different fix. sqlcdata.cpp:209 shows msodbcsql resolving SQL_ARD_TYPE off the column's ARD binding, plus the 07009 unbound-column case and SQL_C_NUMERIC precision/scale from that same binding. So the parity-correct answer is to implement it, and swapping the SQLSTATE would replace one wrong answer with a less-wrong one while making the gap look handled — on a path with no test coverage, in a PR scoped to SQL_C_DEFAULT. That's the kind of change that should carry its own work item, tests, and parity measurement.

So it's registered as a known gap in both doc locations this PR touches — mssql-odbc.instructions.md (with your sqlcdata.cpp:209 citation, the measured HY003, and what implementing it actually requires) and typed-columnar-fetch-plan.md, next to the SQL_C_DEFAULT resolution it sits beside. It's marked as needing a work item; I don't have authority to file one in this run, so flagging it for whoever picks it up.

Your element_stride enumeration and the SQL_C_NUMERIC/ARD-precision observation are both good catches to have on the record — especially the latter, since it documents that the gap is closed by the mapping rather than by accident. Worth knowing if a future row ever adds SQL_C_NUMERIC.

Verified after the change: 1287 unit tests, 64 get_data e2e tests against SQL Server 2025, cargo fmt, cargo clippy -D warnings.

[Saurabh Singh (saurabh500)]

Unattended review sweep — comment-only, and not checked by a human before posting.

Verdict: Correct and well-tested. SQLGetData now resolves SQL_C_DEFAULT through the same odbc_sql_type → resolve_default_c_type pair as the bind/fetch paths, the two new guards hold under mutation, and the msodbcsql parity claims check out. No blocking issues — I only COMMENT, the merge is yours. This is my first pass on the PR; it has already been through Copilot, several unattended runs, and Shiwani Gupta (@shiwanigupta0809), and the current head resolves the last of those threads.

What I re-derived myself rather than trusting the prior threads:

• The streaming e2e tests genuinely hit the PLP path (answers Copilot''s suppressed "vacuous at a larger packet size" note). The pause is stop_here && meta.is_plp() at token_stream.rs:589, and ColumnMetadata::is_plp() (query/metadata.rs:79) is a pure TypeInfoVariant::PartialLen check with no size term — so a VARCHAR(MAX) of any size streams here, unlike the bound path. Your inline rebuttal to Copilot is right.
• The two new guards are mutation-resistant. Hardcoding the pre-lock version read to Odbc3_80 reddens get_data_default_follows_the_declared_odbc_version (its 3.0 leg expects SQL_ERROR because SQL_SS_TIME2 falls to SQL_C_BINARY below 3.8); disabling the width guard (if false && ...) reddens both ..._too_narrow_for_its_fixed_target... and ..._with_zero_buffer_length.... Built and ran cargo nextest run -p mssqlodbc --lib get_data_default.
• Parity holds for the default selection. Sql2CDefault (sqlcprot.h:1823) is called from the shared GetColData path (sqlcdata.h:256) as well as bind/fetch (sqlcfunc.cpp), so "one answer in every direction" is msodbcsql''s own posture; and SQL_ARD_TYPE resolution sits at sqlcdata.cpp:209 exactly as the registered gap documents.

Blocking: none.

Suggestion: one, optional — inline at get_data.rs:971, on pinning the width-guard invariant with a test. Additive to the SQL_C_NUMERIC point Shiwani Gupta (@shiwanigupta0809) already raised and you acknowledged; the observation is settled, this is just about turning it into a regression check.

Nit: none.

[ttk (Theekshna)]

Unattended re-review at 6a561a7. Head moved past my last marker only by a merge from main (six already-merged commits, e.g. #490/#489/#461/#485/#444) plus two genuine new commits responding to open review threads: �768e68d "Carve out the NULL exception and register the SQL_ARD_TYPE gap" and 54e0d85 "Pin the width guard's element_stride coupling with an invariant test." Both directly close items I'd noted as open-and-unanswered from @shiwanigupta0809 and @saurabh500 in my prior passes.

Severity	Count
Blocking	0
Suggestion	1
Nit	0

Verified independently, not taken on trust:

• �768e68d's SQL_ARD_TYPE citation — I grepped sqlcdata.cpp:209-210 in the reference tree myself; the if (fCType == SQL_ARD_TYPE) branch and the ARD-lookup comment are exactly where and what the commit describes.
• The �lement_stride catch-all is genuinely _ => buffer_length.max(0) as usize (etch_scroll.rs:1585) — a fixed-width C type absent from the match really does report width 0, so the new �very_default_c_type_has_a_width_or_is_app_sized test (iterating the full i16::MIN..=i16::MAX SQL-type space, not a hand-picked list) is closing a real, previously-untested hole rather than padding. SQL_C_SS_VECTOR's inclusion in APP_SIZED is correct — vectors are variable-length, same class as char/binary.
• I could not run either new test locally — this machine's toolchain fails to codegen crc32fast's AVX-512 intrinsics for any test binary (llvm.x86.pclmulqdq.256/512, simd_xor), a pre-existing environment limitation unrelated to this PR (confirmed in my review of PR #486 earlier this run). Relying on gh pr checks (green) and the commit's own mutation claim ("only this test reddens, the other 1287 pass").

Suggestion — PR description hasn't caught up with these two commits. The top-level description's "Behaviour" section and checklist (cargo btest passes... 1246 passed) predate both follow-up commits: it doesn't mention the NULL-over-unresolvable-placeholder carve-out (get_data.rs:915-921) or the registered-but-untracked SQL_ARD_TYPE gap, and the test count is now stale (at least two tests were added since). Neither is wrong, just missing from the summary a reviewer would read first — worth a short addendum before merge.

Other checks: parity — covered above (SQL_ARD_TYPE) plus everything already exhaustively audited in my prior two passes (18 measured base types, all matching). Divergence docs — both new gaps recorded consistently in .github/instructions/mssql-odbc.instructions.md and yped-columnar-fetch-plan.md. AI slop — the new doc comments and test docs explain non-obvious why (lock ordering, the mutation each test pins) rather than restating code; not flagging length.