fix: honour the declared SQL type and reject phantom parameters

Three defects in the parameter path, fixed in this order because making an
unbound marker an error before hardening the scanner would have turned
working statements into errors.

`count_params` tracked single-quoted string literals and nothing else, so a
`?` inside a delimited identifier or a comment counted as a parameter marker:
`SELECT "a?b" FROM t` reported one parameter and `SELECT 1 -- huh?` reported
one too. `escape.rs` already scanned the same text correctly, so the fix is to
stop having a second scanner: its `copy_*` helpers split into `skip_*` plus a
copying wrapper, and `count_params` calls the same `skip_*` functions with the
identifier delimiters taken from the backend's `EscapeDialect`.

A marker with no binding was padded with `ColumnValue::Null`, so `WHERE x = ?`
with nothing bound ran as `WHERE x = NULL` — no rows, and `SQL_SUCCESS`. Both
execution paths now report 07002, the first clause of that row on the
`SQLExecute` and `SQLExecDirect` tables, neither of them `(DM)`-marked. The
data-at-execution scan rejects the same gap so it is not a second route to the
old behaviour. A `SQL_PARAM_OUTPUT` binding still yields `Null`: it has no
input value, and reading its uninitialised buffer would be unsound.

`read_param_value` matched on the C type alone, so `ParameterBinding::sql_type`
— `SQLBindParameter`'s `ParameterType` — was recorded and never read. For every
C type but the two character ones that lost nothing; for `SQL_C_CHAR` and
`SQL_C_WCHAR` it discarded the only statement of what the text was, and
`SQL_C_CHAR` + `SQL_NUMERIC` reached the backend as a string. The new
`param_convert` module is the spec's "C to SQL: Character" table transcribed,
with the SQLSTATE its third column gives for each outcome. Decimal literals are
carried as digits and a scale rather than through `f64`, so scale survives.

Driver-visible: a backend now receives `Decimal`, `I32`, `Timestamp` and so on
where a character binding previously produced `String`.

Verified with `pre-commit run --all-files` (15 hooks), 937 unit tests, and
Miri (927 passed, no leaks).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: maltesander

AGENTS.md

@@ -790,6 +790,7 @@ Generic framework. Zero database-specific code.
 | `types/version.rs` | Parsed data-source version numbers, for a backend gating capabilities on server version |
 | `types/redacted.rs` | `Redacted<T>` — `Debug` wrapper that prints `*****` for sensitive fields (e.g. passwords) |
 | `column_value.rs` | `write_column_value()` — core data marshalling for `SQLGetData` (NULL, truncation, type coercion) |
+| `param_convert.rs` | `text_to_sql_type()` — the reverse direction: converts `SQL_C_CHAR`/`SQL_C_WCHAR` parameter text to the SQL type `SQLBindParameter` declared. The spec's "C to SQL: Character" table, transcribed |
 | `synthetic.rs` | `SyntheticStatement` — in-memory result set for `SQLGetTypeInfo` and catalog functions |
 | `catalog_sort.rs` | Sorts a catalog result set into its spec-mandated order; NULL placement from `Backend::null_collation` |
 | `catalog_ident.rs` | `SQL_ATTR_METADATA_ID` identifier normalisation and the `SQLTables` `TableType` value-list parser |

CHANGELOG.md

@@ -797,6 +797,57 @@ Everything a driver has to change for the catalog rework, in one place.
 
 ### Fixed
 
+- **Character parameter data ignored the SQL type it was bound as.**
+  `SQLBindParameter` takes two types — `ValueType`, the C type the value
+  arrives in, and `ParameterType`, the SQL type the data source is to receive —
+  and ODBC makes the driver convert between them. `read_param_value` matched on
+  the C type alone, so `ParameterBinding::sql_type` was recorded and never
+  read. For every C type but the two character ones that lost nothing, because
+  the C type already fixes the value's shape; for `SQL_C_CHAR` and
+  `SQL_C_WCHAR` it discarded the only statement of what the text *was*.
+  `SQL_C_CHAR` + `SQL_NUMERIC` — what pyodbc emits for a `Decimal`, and what
+  any client emits for a value it delivers as text — reached the backend as
+  `ColumnValue::String`, so a driver that renders its parameters emitted
+  `WHERE amount = '12.34'` against a decimal column and the data source
+  rejected the comparison. The new `param_convert` module is the spec's
+  "C to SQL: Character" table transcribed: decimal, exact-integer,
+  approximate-numeric, `SQL_BIT`, binary (hexadecimal pairs) and the three
+  datetime targets, each with the SQLSTATE that table's third column gives —
+  `22018` for text that is not a literal of the declared type, `22001` for a
+  conversion that would truncate, `22003` for out of range, `22008` for a
+  datetime component the target cannot hold. **Driver-visible:** a backend now
+  receives `ColumnValue::Decimal`, `I32`, `Timestamp` and so on where it
+  previously received `String` for these bindings; one that parsed the string
+  itself can drop that code, and one that matched only `String` must handle the
+  typed variants. Character SQL types, the interval types, `SQL_GUID` and
+  driver-specific type identifiers are unchanged and still arrive as `String`.
+  `SQLPutData` data-at-execution text goes through the same conversion, so the
+  two routes to a parameter agree.
+
+- **`?` was counted as a parameter marker inside quoted identifiers and
+  comments.** `count_params` tracked single-quoted string literals and nothing
+  else, so `SELECT "a?b" FROM t` reported one parameter and `SELECT 1 -- huh?`
+  reported one too. `SQLNumParams` over-reported, `collect_params` padded the
+  phantom marker with a value, and a driver whose own substitution scan
+  mirrored core's rewrote the identifier along with it. The scan now skips
+  string literals, delimited identifiers, `--` line comments and `/* … */`
+  block comments, taking the identifier delimiters from the backend's
+  `EscapeDialect` rather than assuming `"`. The region helpers are `escape`'s
+  own, shared with `translate_escapes` so the two scans cannot drift apart
+  again.
+
+- **A parameter marker with no bound value was padded with NULL.**
+  `collect_params` emitted `ColumnValue::Null` for a marker the application
+  never called `SQLBindParameter` for, so `WHERE x = ?` with nothing bound ran
+  as `WHERE x = NULL`, matched no row and reported success — the application
+  saw an empty result set rather than its own mistake. Both `SQLExecute` and
+  `SQLExecDirectW` now report `07002` (COUNT field incorrect), which is the
+  first clause of that row on both diagnostics tables and carries no `(DM)`
+  marker. The data-at-execution scan rejects the same gap, so it is not a
+  second route to the old behaviour. A `SQL_PARAM_OUTPUT` binding still yields
+  `Null` and is unaffected: it has no input value by definition, and reading
+  its uninitialised buffer would be unsound.
+
 - The 32 `HY008` doc comments across `src/ffi/` claimed the state could not
   arise, on one of two false grounds: that "the `Backend` trait is synchronous"
   — which says nothing about another thread cancelling — or that it was

src/column_value.rs

@@ -616,7 +616,7 @@ fn civil_from_days(days: i64) -> (i64, u16, u16) {
     clippy::disallowed_methods,
     reason = "SQL_TYPE_TIME -> SQL_C_TYPE_TIMESTAMP is specified as using the current date"
 )]
-fn current_utc_date() -> (i16, u16, u16) {
+pub(crate) fn current_utc_date() -> (i16, u16, u16) {
     // `try_from` rather than `as`: a clock far enough out to exceed i64 seconds
     // is nonsense either way, but wrapping it into a negative would turn a date
     // in the far future into one in the distant past.
@@ -783,7 +783,7 @@ fn parse_time_fields(s: &str) -> Result<(u16, u16, u16, u32), OdbcError> {
     Ok((hour, minute, second, fraction))
 }
 
-fn parse_sql_date(s: &str) -> Result<Date, OdbcError> {
+pub(crate) fn parse_sql_date(s: &str) -> Result<Date, OdbcError> {
     let (year, month, day) = parse_date_fields(s.trim())?;
     Ok(Date { year, month, day })
 }
@@ -793,7 +793,7 @@ fn parse_sql_date(s: &str) -> Result<Date, OdbcError> {
 /// to `SQL_C_TYPE_TIME` must check the returned fraction themselves and report
 /// 01S07 if it is non-zero — this function only parses, it does not decide
 /// whether the drop is acceptable for the caller's target type.
-fn parse_sql_time(s: &str) -> Result<(Time, u32), OdbcError> {
+pub(crate) fn parse_sql_time(s: &str) -> Result<(Time, u32), OdbcError> {
     let (hour, minute, second, fraction) = parse_time_fields(s.trim())?;
     Ok((
         Time {
@@ -805,7 +805,7 @@ fn parse_sql_time(s: &str) -> Result<(Time, u32), OdbcError> {
     ))
 }
 
-fn parse_sql_timestamp(s: &str) -> Result<Timestamp, OdbcError> {
+pub(crate) fn parse_sql_timestamp(s: &str) -> Result<Timestamp, OdbcError> {
     let t = s.trim();
     // Accept either the ODBC space separator or the ISO 8601 'T'.
     let (date_part, time_part) = match t.split_once([' ', 'T']) {

src/escape.rs

@@ -148,7 +148,7 @@ impl EscapeDialect {
         }
     }
 
-    fn ident_close(&self, open: char) -> Option<char> {
+    pub(crate) fn ident_close(&self, open: char) -> Option<char> {
         self.identifier_quotes
             .iter()
             .find(|(o, _)| *o == open)
@@ -230,38 +230,40 @@ fn translate_slice(
     Ok(out)
 }
 
-/// Copy a single-quoted string literal verbatim, including a `''` doubled quote.
-fn copy_string(chars: &[char], i: &mut usize, out: &mut String) {
-    out.push(chars[*i]); // opening '
-    *i += 1;
+// The `skip_*` helpers below advance `*i` past one lexical region without
+// interpreting it; the `copy_*` wrappers do the same and copy what was skipped.
+// Splitting them this way is what lets `ffi::params::count_params` scan for `?`
+// parameter markers with exactly the region boundaries this module translates
+// escapes with. Two independent scanners is how a `?` inside a quoted
+// identifier came to be counted as a marker.
+
+/// Advance past a single-quoted string literal, including any `''` doubled quote.
+pub(crate) fn skip_string(chars: &[char], i: &mut usize) {
+    *i += 1; // opening '
     while *i < chars.len() {
         let c = chars[*i];
-        out.push(c);
         *i += 1;
         if c == '\'' {
             if chars.get(*i) == Some(&'\'') {
-                out.push('\''); // doubled quote — stays inside the string
-                *i += 1;
+                *i += 1; // doubled quote — stays inside the string
             } else {
                 break; // closing quote
             }
         }
     }
 }
 
-fn copy_quoted_ident(chars: &[char], i: &mut usize, out: &mut String, open: char, close: char) {
-    out.push(chars[*i]); // opening quote
-    *i += 1;
+/// Advance past a delimited identifier opened by `open` and closed by `close`.
+pub(crate) fn skip_quoted_ident(chars: &[char], i: &mut usize, open: char, close: char) {
+    *i += 1; // opening quote
     while *i < chars.len() {
         let c = chars[*i];
-        out.push(c);
         *i += 1;
         if c == close {
             // A doubled close-quote escapes it, but only for symmetric quote
             // styles (`"..."`, `` `...` ``). Bracket identifiers (`[...]`) have
             // no doubling — a `]` always closes them.
             if open == close && chars.get(*i) == Some(&close) {
-                out.push(close);
                 *i += 1;
             } else {
                 break;
@@ -270,33 +272,62 @@ fn copy_quoted_ident(chars: &[char], i: &mut usize, out: &mut String, open: char
     }
 }
 
-fn copy_line_comment(chars: &[char], i: &mut usize, out: &mut String) {
+/// Advance past a `--` line comment, up to and including its newline. An
+/// unterminated comment runs to the end of the statement.
+pub(crate) fn skip_line_comment(chars: &[char], i: &mut usize) {
     while *i < chars.len() {
         let c = chars[*i];
-        out.push(c);
         *i += 1;
         if c == '\n' {
             break;
         }
     }
 }
 
-fn copy_block_comment(chars: &[char], i: &mut usize, out: &mut String) {
-    out.push(chars[*i]); // '/'
-    out.push(chars[*i + 1]); // '*'
-    *i += 2;
+/// Advance past a `/* … */` block comment. An unterminated comment runs to the
+/// end of the statement.
+pub(crate) fn skip_block_comment(chars: &[char], i: &mut usize) {
+    *i += 2; // '/' '*'
     while *i < chars.len() {
         if chars[*i] == '*' && chars.get(*i + 1) == Some(&'/') {
-            out.push('*');
-            out.push('/');
             *i += 2;
             break;
         }
-        out.push(chars[*i]);
         *i += 1;
     }
 }
 
+/// Copy a single-quoted string literal verbatim, including a `''` doubled quote.
+fn copy_string(chars: &[char], i: &mut usize, out: &mut String) {
+    copy_skipped(chars, i, out, skip_string);
+}
+
+fn copy_quoted_ident(chars: &[char], i: &mut usize, out: &mut String, open: char, close: char) {
+    copy_skipped(chars, i, out, |chars, i| {
+        skip_quoted_ident(chars, i, open, close);
+    });
+}
+
+fn copy_line_comment(chars: &[char], i: &mut usize, out: &mut String) {
+    copy_skipped(chars, i, out, skip_line_comment);
+}
+
+/// Run `skip` and append every character it advanced over to `out`.
+fn copy_skipped(
+    chars: &[char],
+    i: &mut usize,
+    out: &mut String,
+    skip: impl FnOnce(&[char], &mut usize),
+) {
+    let start = *i;
+    skip(chars, i);
+    out.extend(&chars[start..*i]);
+}
+
+fn copy_block_comment(chars: &[char], i: &mut usize, out: &mut String) {
+    copy_skipped(chars, i, out, skip_block_comment);
+}
+
 /// Find the index of the `}` matching the `{` at `open`, skipping strings,
 /// quoted identifiers, comments and nested braces. Errors if unterminated.
 fn find_matching_brace(