microsoft / microsoft/mssql-rs

mssql-odbc: SQLBindCol/SQLBindParameter leave stale ARD/APD length/precision/scale behind on a differently-shaped rebind

Open
#470 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
53
Forks
14
Avg merge
1d 15h
Merged PRs (30d)
137

Description

## Context

Follow-up from [PR #436](https://github.com/microsoft/mssql-rs/pull/436) ([AB#47437](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47437) — descriptor records as the binding/metadata source of truth), flagged by an automated code review's non-blocking question.

## Problem

`ColumnBinding::write_to_record` (ARD, `SQLBindCol`) and the APD half of `BoundParam::write_to_records` (`SQLBindParameter`) only write `SQL_DESC_CONCISE_TYPE`, `SQL_DESC_DATETIME_INTERVAL_CODE`, `SQL_DESC_DATA_PTR`, `SQL_DESC_OCTET_LENGTH`, `SQL_DESC_INDICATOR_PTR`, and `SQL_DESC_OCTET_LENGTH_PTR`. Neither touches `SQL_DESC_LENGTH`, `SQL_DESC_PRECISION`, or `SQL_DESC_SCALE` on that record.

Sequence that reproduces the gap:
1. `SQLBindCol`/`SQLBindParameter` binds ordinal N with a C type whose precision/scale get set to some value.
2. A later `SQLBindCol`/`SQLBindParameter` call rebinds the *same* ordinal to a different C type.
3. `SQLGetDescField`/`SQLGetDescRecW` on `SQL_DESC_LENGTH`/`SQL_DESC_PRECISION`/`SQL_DESC_SCALE` for that record still reports the *previous* binding's value, not a value appropriate to the new C type.

This is metadata-introspection only — execution reads parameter type/size from the IPD (which *is* fully reset on every `SQLBindParameter`; `write_to_records` sets `precision`/`length`/`scale` unconditionally there) and reads column data via the ARD's `concise_type`/`data_ptr`/lengths (also unaffected). No query result or parameter value is wrong; only a direct `SQLGetDescField`/`SQLGetDescRecW` read of those three specific APD/ARD fields after a differently-shaped rebind can observe stale data.

## Verified against msodbcsql

`sqlcdesc.cpp`'s `SetADRec` (`SQLBindCol`/`SQLSetDescRec`, ~line 2615) and `SetADRecBP` (`SQLBindParameter`'s APD write, ~line 2789) both `ZeroMemory` the *entire* bind-info record before repopulating it, then call `SetTypeDefaults` (~line 12344) — a large per-C-type default table (GUID → `SQL_PREC_GUID`, NUMERIC/DECIMAL → `SQL_PREC_NUMERIC` + scale 0, FLOAT/REAL → their own distinct precisions, CHAR/VARCHAR/BINARY family → precision 1, date/time/timestamp/interval types → precision 2 and type-specific scales like `SCALE_DATETIME2`/`SCALE_TIME`/`SCALE_DATETIMEOFFSET`, interval-to-second subtypes → scale 6, etc.) — before writing any of the call's own arguments. `ExportImp::SQLBindCol` (~line 2470) computes this same default table itself (it has no precision/scale parameters of its own) and threads the result through to `SetADRec`. Net effect: msodbcsql always resets these three fields to fresh, C-type-appropriate defaults on every bind call, never leaving a prior binding's values behind.

## Why this wasn't fixed directly in #436

Correctly replicating `SetTypeDefaults` requires enumerating and verifying msodbcsql's full per-`SQL_C_*`-type default precision/scale table against this driver's own type constants — a self-contained, correctness-sensitive piece of work in its own right, not a one-line fix, and not worth rushing into an already large PR for a metadata-only edge case the reviewer who found it explicitly flagged as "not worth holding the PR for."

## Suggested approach

Add a `reset_type_defaults(c_type: SqlSmallInt) -> (length, precision, scale)` helper (or similar) mirroring `SetTypeDefaults`'s table, called from both `ColumnBinding::write_to_record` and the APD half of `BoundParam::write_to_records` before the other fields are written, with unit tests pinning at least one representative case per table branch (GUID, NUMERIC/DECIMAL, FLOAT, REAL, CHAR/VARCHAR/BINARY family, each datetime/interval subtype).

## References

- PR: https://github.com/microsoft/mssql-rs/pull/436
- Review: https://github.com/microsoft/mssql-rs/pull/436#pullrequestreview-5088111197
- Related work item: [AB#47437](https://sqlclientdrivers.visualstudio.com/b95cf060-8083-439d-8ef1-405d5bf219d8/_workitems/edit/47437)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate ColumnBinding::write_to_record and BoundParam::write_to_records, then trace the SQLBindCol and SQLBindParameter entry points and existing descriptor tests. Compare the C-type defaults needed for ARD/APD records and add coverage for each listed type-table branch. Done means differently shaped rebinds expose fresh length, precision, and scale values through SQLGetDescField and SQLGetDescRecW.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.