Resolve SQL_C_DEFAULT bound columns at fetch time

[David Engel (David-Engel)]

Summary

• Accept SQL_C_DEFAULT in SQLBindCol instead of rejecting it with HY003.
• Resolve default C types from the current result column metadata and ODBC version when each fetch snapshots its bindings, via resolve_default_bindings.
• Keep the persistent binding unresolved so it can be reused correctly across result sets.
• Refuse to resolve a fixed-width target wider than the application's declared BufferLength, so deferred resolution cannot overrun a bound slot.
• Document the parity deviations that now also apply on the fetch path, and cover them with tests.
• Remove the obsolete documented limitation and update the changelog.

AB#47481

Why this is needed

Since AB#47437 (#436) the ARD is the fetch's single source of truth, and SQLBindCol and an equivalent SQLSetDescField sequence are meant to be indistinguishable. SQL_C_DEFAULT was the exception:

Path before this PR	Result
SQLBindCol(1, SQL_C_DEFAULT, buf, n, &ind)	SQL_ERROR, HY003
SQLSetDescField(ard, 1, SQL_DESC_CONCISE_TYPE, SQL_C_DEFAULT) + SQL_DESC_DATA_PTR	succeeds, then HYC00 per row

The descriptor route gets there because set_type takes the kind.is_application() branch, canonical_c_type leaves SQL_C_DEFAULT alone, and is_valid_c_type lists it — while ColumnBinding::from_record keys "bound" off a non-null SQL_DESC_DATA_PTR rather than the concise type. Both doors now resolve identically, which restores the equivalence #436 was built to guarantee.

Parity deviations

Resolution reuses type_rules::resolve_default_c_type, shared with SQLBindParameter, so the driver gives one answer to what SQL_C_DEFAULT means regardless of direction. That resolver carries two deliberate deviations from msodbcsql's Sql2CDefault, accepted in #323 and registered in parameters_plan.md, mssql-odbc.instructions.md, and AB#47365. This PR makes them apply on the fetch path too, so they are restated where the mapping is used, and the registry entry — which previously scoped them to SQLBindParameter — is widened to cover both directions:

SQL type	This driver	msodbcsql
SQL_WCHAR, SQL_WVARCHAR, SQL_WLONGVARCHAR	SQL_C_WCHAR	SQL_C_CHAR
SQL_GUID	SQL_C_GUID	SQL_C_CHAR

Both cells are measured against msodbcsql18, not inferred from the Sql2CDefault tables. Binding an nvarchar and a uniqueidentifier column with SQL_C_DEFAULT and BufferLength 64 returns the wide value as the three narrow bytes 6F 6E 65 with indicator 3, and the GUID as the 36-character text form with indicator 36.

SQL_SS_XML is deliberately absent from that table: msodbcsql maps it to SQL_C_WCHAR as well (an xml column bound SQL_C_DEFAULT returns UTF-16 with indicator 30), so there is no deviation, and it is unreachable on this path because describe_col.rs reports xml and json columns as SQL_WLONGVARCHAR.

The wide deviation exists because this driver's SQL_C_CHAR is UTF-8 by design; msodbcsql's narrow default reads the client code page, which has no equivalent here. The SQL_GUID deviation is the one that also changes the bound rowset layout: a fixed-width target strides by its C type rather than by BufferLength, so the stride becomes sizeof(SQLGUID). A slot at least 16 bytes wide — including the 36-character text form msodbcsql would fill — takes the narrower stride and stays inside the application's array.

Narrow-buffer guard

BufferLength is ignored for a fixed-width target, which is safe when the application named that type and so accepted its width contract. A SQL_C_DEFAULT binding names nothing, so resolving a uniqueidentifier column to SQL_C_GUID would write 16 bytes into whatever slot the application declared — 4 bytes, say — where msodbcsql resolves to SQL_C_CHAR and truncates inside BufferLength. Deferred resolution is what makes that reachable without the application naming the C type.

Such a binding is therefore left unresolved and the row fails with HYC00 rather than writing. The check is generic over element_stride rather than GUID-specific: SQL_C_TYPE_DATE / TIME / TIMESTAMP and, at ODBC 3.8, SQL_C_SS_TIME2 / SQL_C_SS_TIMESTAMPOFFSET all route through the same fixed_width > 0 && buffer_length > 0 && buffer_length < fixed_width comparison. GUID is the motivating case and the one unit-tested directly, since the guard's correctness depends on the arithmetic rather than on which C type triggers it.

BufferLength 0 is exempt: it is the documented idiom for a fixed-width target and carries no width claim. The width is SQL_DESC_OCTET_LENGTH on the ARD record, which an application can rewrite after binding without touching the type — checking it per fetch against the same snapshot the fill loop uses is what keeps a slot narrowed after the bind from slipping through. Whether these should instead report 01004 and deliver a truncated value, matching msodbcsql more closely, is open and untracked.

Other newly-reachable cases, registered not changed

• Resolution is ODBC-version aware, with the version read from the environment on each fetch: SQL_SS_TIME2 and SQL_SS_TIMESTAMPOFFSET default to SQL_C_BINARY below ODBC 3.8 and to their SQL_C_SS_* types at 3.8. That 3.8 mapping makes a stride-only divergence reachable: both drivers resolve identically, but msodbcsql's BindOffset strides by BufferLength where element_stride uses 12 / 20. Measured at BufferLength 40, msodbcsql puts row 1 at byte offset 40 and this driver at 12, indicator 12 in both. Ours is the safer direction — BufferLength 0 would make msodbcsql stride 0 and stack every row in slot 0.
• A varbinary / image column resolves to SQL_C_BINARY, which bound delivery does not implement yet (AB#47239), so it fails per row with HYC00. msodbcsql resolves identically and delivers the bytes.

A column whose SQL type has no default keeps SQL_C_DEFAULT and is reported as an unsupported target for any row carrying a value; a NULL row still reports SQL_NULL_DATA through its indicator, because nothing needs converting. A binding whose ordinal is past the end of the result set also stays unresolved, but never reaches delivery — the fill loop skips it, matching msodbcsql.

Known asymmetry, pre-existing and untouched: SQLGetData still refuses SQL_C_DEFAULT with HYC00, so the same placeholder is answered two ways depending on how a column is read. Recorded in typed-columnar-fetch-plan.md; not currently tracked by a work item.

Testing

• cargo nextest run -p mssqlodbc: 1,237 passed, 1 skipped.
• cargo clippy --workspace --all-features --all-targets -- -D warnings
• cargo fmt -- --check
• Unit coverage: wide and GUID columns resolving to their typed targets including the resulting element_stride; a binding whose column is absent from the result set staying unresolved; a_default_binding_follows_the_declared_odbc_version, a fetch-level test driving a SQL_SS_TIME2 column at ODBC 3.0 and 3.8 that pins both the per-row SqlReturn and its SQLSTATE (HYC00 vs 07006); two tests for the narrow-buffer guard, one of which uses a 64-byte backing array behind a declared BufferLength of 4 so a regression surfaces as bytes written past the declared width rather than as a crash; and a_default_binding_set_through_the_descriptor_api_also_resolves, which binds entirely through SQLSetDescField to cover the second door into the resolver. Each guard was confirmed to fail with its guard removed.
• E2E DefaultTargetResolvesWideAndGuidToTypedTargets pins the wide indicator in UTF-16 bytes and the GUID stride at sizeof(SQLGUID), binding four GUID slots with a BufferLength of two so a BufferLength-driven stride fails an assertion inside the array rather than running off the end. It asserts a deliberate deviation, so it carries SKIP_IF_COMPARING_MSODBCSQL().
• E2E DefaultTargetResolvesPerResultSet drives two result sets of different column types through one SQL_C_DEFAULT binding. It deliberately uses int and narrow varchar, which both drivers resolve identically, so it runs and compares on the msodbcsql leg too.
• Full C++ e2e comparison against mssql-odbc and retail msodbcsql on the Linux and Windows PR legs: all test executables passed on each driver with zero parity divergences.

Breaking changes / migration notes

None. The narrow-buffer guard only affects SQL_C_DEFAULT bindings that would previously have overrun the application's declared buffer.

[Copilot]

Pull request overview

Enables deferred SQL_C_DEFAULT resolution for bound fetch columns. However, the shared resolver introduces undocumented fetch-side parity deviations for wide-character and GUID columns.

Changes:

• Accept and retain SQL_C_DEFAULT in SQLBindCol.
• Resolve bindings from result metadata and ODBC version during fetch.
• Add unit/e2e coverage and update documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
CHANGELOG.md	Records the behavior change.
mssql-odbc/docs/typed-columnar-fetch-plan.md	Removes the previous limitation.
mssql-odbc/src/api/bind_col.rs	Defers default target resolution.
mssql-odbc/src/api/fetch_scroll.rs	Resolves default types per fetch.
mssql-odbc/tests/e2e/tests/fetch_scroll_test.cpp	Tests integer and character rowset bindings.

---

💡 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/bind_col.rs (100%)
• mssql-odbc/src/api/fetch_scroll.rs (98.5%): Missing lines 560-561

Summary

• Total: 154 lines
• Missing: 2 lines
• Coverage: 98%

mssql-odbc/src/api/fetch_scroll.rs

  556     // the stmt lock to preserve parent-before-child lock ordering.
  557     let odbc_version = {
  558         let env = stmt.parent_dbc().parent_env();
  559         let Ok(env_state) = env.inner.lock() else {
! 560             error!("SQLFetchScroll: env mutex poisoned");
! 561             return SQL_ERROR;
  562         };
  563         env_state.odbc_version
  564     };

---

---

🔗 Quick Links

View Azure DevOps Build · Coverage Report

[ttk (Theekshna)]

This unattended review found no issues in the proposed change.
Covered deferred SQL_C_DEFAULT resolution, per-result-set rebinding, ODBC-version mappings, buffer strides, lock ordering, unit/e2e test sufficiency, msodbcsql parity and documented divergences, CI/checklist consistency, and PR-description currency.

Category	Count
Blocking	0
Suggestion	0
Nit	0

[ttk (Theekshna)]

Summary

The design matches the reference: msodbcsql also keeps the descriptor unresolved and calls Sql2CDefault at data-fetch time (sqlcdata.h:254-257), and its BindOffset stride rule (sqlcfunc.cpp:2176-2284) matches element_stride — fixed types stride by sizeof(C type), char/binary by cbValueMax, with SQL_C_DEFAULT resolved before the stride is computed. Lock ordering follows the existing bind_param.rs:129-138 precedent. Tests are non-vacuous: removing the resolve_default_bindings call fails default_binding_resolves_from_current_result_metadata, and the e2e GUID test is well built — four slots for a rowset of two makes a BufferLength-driven stride fail an assertion inside the array rather than smash the stack.

No blocking findings. Two suggestions and three nits, the latter all on files outside the diff.

Nit — the registry entry still scopes the deviation to SQLBindParameter

.github/instructions/mssql-odbc.instructions.md:79 reads "SQL_C_DEFAULT in SQLBindParameter resolves the wide character SQL types to SQL_C_WCHAR, and SQL_GUID to SQL_C_GUID". The PR description says these deviations are registered there, but that file isn't in the diff, so the canonical registry now understates the scope. A few words.

(It also names only "the wide character SQL types" and SQL_GUID — no SQL_SS_XML — which corroborates the inline comment on the new table row.)

Nit — SQLGetData(SQL_C_DEFAULT) still answers HYC00

get_data.rs:378-393 rejects it, where msodbcsql resolves it in the same GetColData that serves both paths. Pre-existing and unchanged here, so a legitimate deferral, but the driver now answers the same placeholder two different ways depending on how the column is read. I couldn't find it tracked anywhere; worth a work item while odbc_sql_type + resolve_default_c_type are freshly wired up next door.

Nit — one more unregistered stride divergence, now reachable by default

Under 3.8 both drivers resolve time/datetimeoffset identically (rgbTRANSTYPE380, sqlcmisc.cpp:220-221), but msodbcsql's BindOffset switch has no case for SQL_C_SS_TIME2/SQL_C_SS_TIMESTAMPOFFSET, so they fall to default: dwOffset = lpbindinfo->cbValueMax (sqlcfunc.cpp:2280-2283) while element_stride returns 12 and 20. Pre-existing for explicit binds, but this PR makes it reachable without the application naming the type. Ours is the safer direction — msodbcsql with BufferLength 0 strides 0 and stacks every row in slot 0 — so I'd register it rather than change behaviour.

---

Not verified: no e2e run (no live server), no reproduction of the msodbcsql18 measurements, no cross-platform build.

[David Engel (David-Engel)]

Thanks — all three nits were real, and I acted on all three in dab4f5c.

.github/instructions/mssql-odbc.instructions.md:79 reads "SQL_C_DEFAULT in SQLBindParameter resolves..."

Fixed. It now says the deviation applies in both directions and names SQLFetchScroll resolving per fetch from the IRD through the same resolver. Good catch that the canonical registry was the one place still scoped to the old truth — the PR description pointed at it as authoritative, which made the gap worse than a stale comment.

I also added the SQL_SS_XML non-deviation there explicitly, since you noted the registry corroborated it. Recording why it is absent should stop it being re-derived from the Sql2CDefault tables a third time.

one more unregistered stride divergence, now reachable by default

Registered, and measured first rather than taken on the source read — registering an unverified cell is exactly what went wrong with SQL_SS_XML. A two-row rowset over time(7), bound SQL_C_DEFAULT with BufferLength 40 against msodbcsql18:

ind=[12, 12]
00: 01 00 02 00 03 00 00 00 BC CC 5B 07 EE EE EE EE   <- row 0 at offset 0
...
28: EE EE EE EE EE EE EE EE 0B 00 0C 00 0D 00 00 00   <- row 1 at offset 40

Row 1 lands at 40, not 12, and the indicator is 12 in both drivers — so the resolved type agrees and only the stride differs, exactly as you described. Kept our behaviour for the reason you gave: BufferLength 0 would make msodbcsql stride 0 and stack every row in slot 0. Recorded in both the registry and the plan doc, including that it is pre-existing for an explicit bind and that deferred resolution is what makes it reachable by default.

SQLGetData(SQL_C_DEFAULT) still answers HYC00

Confirmed at get_data.rs:378-393 — SQL_C_DEFAULT is not a typed target, not SQL_C_CHAR/SQL_C_WCHAR, and not a binary probe, so it falls to the HYC00. I have recorded the asymmetry in the plan doc, including that it is untracked.

I have not filed the work item. Creating one is outside the actions I am authorised to take on this run, so I have deliberately left it rather than quietly skipping it — it needs a human to file, and the note in the plan doc is there so it is not lost in a review thread. Happy to be pointed at it if you would rather it were tracked before this merges.

All three are documentation-only; no behaviour changed. Unit suite still 1,183 passing, fmt and clippy clean.

Not verified: no e2e run (no live server), no reproduction of the msodbcsql18 measurements

Worth noting the two measurements above were reproduced independently on my side against a live SQL Server and a retail msodbcsql18, and the SQL_SS_XML one contradicted what I had originally written — so that gap in your review was load-bearing, not incidental.

[ttk (Theekshna)]

Unattended re-review: head moved (418e12bc to dab4f5c5) since my last marked review at that sha (findings=0). In between, a separate review (submitted 09:50 UTC, same sha, no auto-review marker so it didn't drive skip-logic but I treated its findings as mine to check) raised 2 suggestions + 3 nits. All five are now addressed by the two commits since:

• 0d4ed9c6: drops SQL_SS_XML from the deviation table/doc comment (msodbcsql maps it to SQL_C_WCHAR too, confirmed by measurement, and it's unreachable here since odbc_sql_type reports xml/json as SQL_WLONGVARCHAR); adds a fetch-level test (fetch_time_column_row_state) that drives the ODBC-version-dependent SQL_SS_TIME2 resolution at 3.0 and 3.8 and distinguishes the result by diagnostic code, closing the observability gap the prior review flagged (hardcoding the version left all tests passing); corrects the "binding past end of result set" claim to match what the fill loop actually does.
• dab4f5c5: widens the SQL_C_DEFAULT deviation registry entry from "in SQLBindParameter" to cover SQLFetchScroll too; registers the time/datetimeoffset stride divergence (measured: msodbcsql strides by BufferLength, this driver by sizeof) with a stated rationale for keeping rather than matching; registers that SQLGetData(SQL_C_DEFAULT) still answers HYC00 while fetch-bound columns now resolve.

I checked each fix against the actual diff rather than the commit message alone (typed-columnar-fetch-plan.md, mssql-odbc.instructions.md, the new test in fetch_scroll.rs) — all match what was claimed, with the msodbcsql citations (sqlcmisc.cpp:179/218/220-221, sqlcfunc.cpp:2280-2283) consistent with what I'd expect from prior verification of this file (path+line, not quoted).

Test sufficiency: the new fetch-level test is exactly the mutation-informed gap the prior review asked for (a direct resolve_default_bindings call wouldn't have caught the wiring bug). No further gap found in this increment.

msodbcsql parity: covered above — the corrections are accurate and the newly-registered divergences are honest about which direction is safer and why.

Divergences documented: yes, this increment is entirely divergence-documentation work, done at the two locations the repo uses for it (.github/instructions/mssql-odbc.instructions.md and the design doc).

PR description/registry currency: consistent as of this head.

Verbose slop: none in the new doc/comment text — it's measurement-backed, not restating the diff.

CI: no failing checks found via gh pr checks 460.

Category	Count
Blocking	0
Suggestion	0
Nit	0

[Vahid (Vahid-b)]

Summary

SQLBindCol stops rejecting SQL_C_DEFAULT and each fetch resolves the placeholder from the current result set's column types, through the resolver already shared with SQLBindParameter. The design matches the reference driver's shape and every parity claim in the description checks out against the msodbcsql source: rgbTRANSTYPE / rgbTRANSTYPE380 at sqlcmisc.cpp:153 and :192 both read SQL_C_CHAR, // SQL_GUID, both read SQL_C_WCHAR, // SQL_XML_MAPPED, and BindOffset (sqlcfunc.cpp:2176-2284) has a SQL_C_GUID case at :2242 but none for SQL_C_SS_TIME2 / SQL_C_SS_TIMESTAMPOFFSET, so those fall to default: dwOffset = lpbindinfo->cbValueMax; fFixed = FALSE; at :2280-2283. All three newly-registered divergences are accurate.

No blocking findings. Two suggestions and four nits, inline; most are documentation precision rather than code.

Incidental win worth naming: before this PR set_desc_field.rs's set_type already accepted SQL_C_DEFAULT on an ARD record via is_valid_c_type, while SQLBindCol refused it — contradicting that function's own doc comment that "a descriptor-set type can never diverge from what direct binding accepts". This closes that.

Evidence

Measured on this head (dab4f5c5), Linux, cargo nextest run -p mssqlodbc --lib:

• Baseline: 1167 passed / 0 failed / 1 skipped.
• Mutation — drop .saturating_sub(1) from the ordinal lookup at fetch_scroll.rs:539: 3 failures (default_binding_resolves_from_current_result_metadata, default_bindings_resolve_wide_and_guid_columns_to_typed_targets, a_default_binding_follows_the_declared_odbc_version). Guarded.
• Mutation — replace the env read at :557-564 with a hardcoded OdbcVersion::Odbc3_80: exactly 1 failure, a_default_binding_follows_the_declared_odbc_version. Independently reproduces the wiring gap raised earlier and confirms the new test closes it.

Coverage

Row	Status
Primary logic	Examined — resolve_default_bindings, fetch_scroll_safe, sql_bind_col_safe
Siblings	Examined — deliver_bound, deliver_fixed_bound, deliver_encoded_string, deliver_bound_plp; the SQLFetch delegation; all four parent_env() lock sites
Callers & implementers	Examined — every .bindings consumer (only fetch_scroll and handles/stmt), all RowWriter methods on BoundRowWriter
Diagnostics	Examined — RowIssue mapping, poison early-return ordering relative to free_errors
Tests	Examined, plus the two mutations above
Build, packaging, pipelines	Not applicable — no such files in the diff
Docs & comments	Examined — msodbcsql citations verified line by line against a local checkout
Description & work items	Examined — 6/6 files map to description bullets, no riders; AB#47481 linked; Changed is the right heading
CI evidence	Examined — all checks green, coverage comment present at 98% diff

Not verified: no live SQL Server, so the e2e test and the msodbcsql18 probe values in the description are not reproduced; Linux host only, no Windows or macOS build; mssql-python cross-repo leg read as green rather than run.

[Shiwani Gupta (shiwanigupta0809)]

Re-reviewed on this head, after #436 merged. Read the new width guard, resolve_default_bindings, and the descriptor plumbing #436 landed underneath this. Not re-raising anything from the existing threads — the GUID overrun, the binary-PLP registry entry, the NULL wording, the per-result-set test, and the env-lock cadence are all settled.

Blocking: none. Suggestions: 1.

Suggestion

This PR closes a real hole #436 left open, and the rebase is the moment to claim it — with a test — rather than let it look like an incidental side effect.

The branch is CONFLICTING against main now. The mechanical part is small: fetch_scroll.rs:705 still does stmt_state.bindings.clone(), and main:644 is ColumnBinding::all_from_ard_state(&desc_state). resolve_default_bindings mutating the per-fetch snapshot stays correct through that — the snapshot is derived fresh from the ARD each fetch, so the persistent record still keeps SQL_C_DEFAULT and bullet 3 of the summary survives unchanged.

The semantic part is the one worth acting on. SQL_C_DEFAULT can already reach an ARD record on main today, through the descriptor API that #436 added — and it behaves differently from the SQLBindCol path:

Path on current main	Result
SQLBindCol(1, SQL_C_DEFAULT, buf, n, &ind)	SQL_ERROR, HY003 (bind_col.rs:172-176)
SQLSetDescField(ard, 1, SQL_DESC_CONCISE_TYPE, SQL_C_DEFAULT) + SQL_DESC_DATA_PTR	succeeds, then HYC00 + SQL_ROW_ERROR on every row

The descriptor path gets there because set_type takes the kind.is_application() branch, canonical_c_type(99) is a no-op (99 is outside the SQL_C_DATE..=SQL_C_TIMESTAMP fold), and is_valid_c_type lists SQL_C_DEFAULT — so r.concise_type = SQL_C_DEFAULT is written. ColumnBinding::from_record keys "bound" off a non-null SQL_DESC_DATA_PTR, not the concise type, so it hands that binding to the fetch loop, where deliver_bound refuses it per row.

That is precisely the state #436's own summary says cannot occur:

so SQLBindCol/SQLBindParameter and an equivalent SQLSetDescFieldW/SQLSetDescRec sequence can never produce contradictory state.

Two things follow, and neither is a criticism of the change as written:

1. This is a stronger justification than the one the description gives. The current framing is "no known consumer binds SQL_C_DEFAULT on the fetch path — mssql-python uses it only for parameters", which reads as speculative. The concrete version is that the equivalence #436 was built to guarantee is currently broken for exactly this value, and this PR is what restores it. Worth one line in the description.

2. The descriptor route deserves a test, because it reaches resolve_default_bindings by a different door. Every new unit and e2e case goes through SQLBindCol. After the rebase, a SQLSetDescField-established SQL_C_DEFAULT binding will resolve identically — that is a genuine improvement over main's per-row HYC00, and nothing pins it. A SQLSetDescField variant of default_binding_resolves_from_current_result_metadata costs a few lines and covers the door that is currently untested in both directions.

Worth checking while you are in there, since the same rebase decides it: the new width guard reads binding.buffer_length, which post-#436 is record.octet_length and is independently writable via SQLSetDescField(SQL_DESC_OCTET_LENGTH) after the bind. Reading it per fetch from the fresh snapshot keeps that correct, but the guard's rationale — "the width contract belongs to the application only when it named the type" — is now stated about a field the application can change without rebinding, so the comment is worth re-reading against the descriptor model even though I believe the code is right.

[Shiwani Gupta (shiwanigupta0809)]

Reviewed — see #460 (comment)

[David Engel (David-Engel)]

Thanks — the descriptor-route point was the valuable one, and I acted on both parts in 2122e20.

First, a timing note: the rebase you flagged as outstanding is done. #436 merged at 17:20 UTC and I reconciled in 09a2085 a few minutes before your review posted, so fetch_scroll.rs no longer does stmt_state.bindings.clone(). Your read of what the reconciliation needed was right, including that resolve_default_bindings mutating the per-fetch snapshot stays correct through it — the ARD-derived snapshot is rebuilt each fetch, so the persistent record still keeps the placeholder. The one thing the mechanical description missed is that resolve_default_bindings also needed its input changed: column_metadata is only reachable under the STMT lock, and the bindings are now read under the DESC lock after that lock is released, so the column SQL types are snapshotted under STMT and carried out.

the descriptor route deserves a test, because it reaches resolve_default_bindings by a different door

Agreed, and added — a_default_binding_set_through_the_descriptor_api_also_resolves. I verified your causal chain first rather than taking it on the description: set_type takes the kind.is_application() branch, canonical_c_type(99) is a no-op, is_valid_c_type lists SQL_C_DEFAULT, and from_record returns Some on a non-null SQL_DESC_DATA_PTR regardless of concise type. All four hold.

Writing it surfaced something worth recording: the descriptor API exposes SQL_DESC_INDICATOR_PTR and SQL_DESC_OCTET_LENGTH_PTR as separate fields, where SQLBindCol's single StrLen_or_Ind argument feeds both. Setting only the indicator resolves the value correctly but reports no length. So the two doors are not quite interchangeable at the application level even now, and the test sets both to be a faithful equivalent. That is ODBC-correct rather than a defect, but it is exactly the kind of thing a SQLBindCol-only test suite cannot see.

This is a stronger justification than the one the description gives

Taken. The bind/descriptor equivalence framing is now in the plan doc with the mechanism spelled out, replacing the speculative "no known consumer" reasoning.

the guard's rationale ... is now stated about a field the application can change without rebinding

Good catch, and you are right that the code is correct but the comment was not carrying its weight. buffer_length is record.octet_length post-#436 and is independently writable via SQLSetDescField after the bind. The guard is still right precisely because it runs per fetch against the same snapshot the fill loop uses, so a slot narrowed after binding is caught rather than missed — I have said that explicitly rather than leaving the reader to reconstruct it.

1,237 unit tests pass, fmt and clippy clean.

[ttk (Theekshna)]

Unattended re-review: head moved from dab4f5c5 (my last marked review, findings=0) to 2122e20d via two new commits — f90e2419 (refuse a SQL_C_DEFAULT fixed-width target wider than the bound BufferLength) and 2122e20d (cover the descriptor/SQLSetDescField route into the same resolution path) — plus merge commits pulling in unrelated #436/#453/#463 work with no conflicts in this PR's own files.

Reviewed both new commits in full against mssql-odbc/src/api/fetch_scroll.rs and the two docs they touch.

Severity	Count
Blocking	0
Suggestion	0
Nit	0

What I checked:

• The new narrow-buffer guard in resolve_default_bindings is generic over element_stride, not GUID-specific despite the PR description's framing — it correctly also covers SQL_C_TYPE_DATE/TIME/TIMESTAMP and SQL_C_SS_TIME2/SQL_C_SS_TIMESTAMPOFFSET at ODBC 3.8, since all route through the same fixed_width > 0 && buffer_length > 0 && buffer_length < fixed_width check. Only the GUID case is unit-tested directly, but since the guard's correctness depends only on the arithmetic comparison (not on which C type triggers it), one concrete instantiation is adequate coverage; I don't think a test per fixed-width type is needed here.
• BufferLength == 0 exemption verified correct and tested (a_default_binding_too_narrow_for_its_fixed_target_stays_unresolved) — matches the documented idiom for fixed-width targets.
• New overrun test (a_narrow_default_guid_binding_never_writes_past_the_declared_buffer) asserts bytes past BufferLength are untouched using an oversized backing array — good regression shape, confirms the guard stops the write rather than just returning early.
• Descriptor-route fix: verified set_type/is_valid_c_type/canonical_c_type and ColumnBinding::from_record in the current tree — the bind/descriptor disagreement (HY003 via SQLBindCol vs HYC00 per-row via SQLSetDescField) is real and the fix is the right one (key both paths through the same resolve_default_bindings). New unit test drives the descriptor path standalone and asserts identical resolution.
• msodbcsql parity: N/A for these two commits specifically (no new parity claims beyond what was already reviewed and documented at dab4f5c5; GUID/wide-char deviations were already verified against C:\REPOS\Drivers\ODBC_2 in the earlier review of this PR).
• Divergences: both new behaviors (narrow-buffer refusal, descriptor-route equivalence) are documented in .github/instructions/mssql-odbc.instructions.md and mssql-odbc/docs/typed-columnar-fetch-plan.md, consistent with the code.
• PR description: matches current diff and testing claims; linked to AB#47481. cargo check -p mssqlodbc --lib --tests --offline compiles clean; full nextest run not possible in this sandbox (pre-existing local toolchain codegen limitation in crc32fast, unrelated to this PR) — CI (build 171912) was still pending on all build/test legs at review time, no failures observed.
• No AI slop found in the new doc comments — they explain why (lock ordering, snapshot-vs-bind-time consistency, ARD as source of truth), not restating the diff.

No re-filed findings — nothing outstanding from prior threads on this PR to re-check; all were already resolved before this push.