Fix numeric parameters silently sent as NULL after a character-typed bind (#300)

[fdcastel]

Fixes #300.

The problem

A parameter's sqlvar is rewritten on every bind to match the C type being transferred (OdbcConvert::setHeadSqlVar, called at the top of getAdressFunction), but the driver described the parameter from that same live sqlvar. That closes a loop:

execute	describe reads sqlvar	converter chosen	setHeadSqlVar writes sqlvar
NULL bound SQL_C_DEFAULT	SQL_INT64 (correct)	transferStringToAllowedType	SQL_TEXT
value bound SQL_C_SLONG	SQL_TEXT	convLongToString	SQL_TEXT again

SQL_C_DEFAULT resolves to SQL_C_CHAR for SQL_NUMERIC, SQL_DECIMAL and SQL_BIGINT, so an application that binds its NULLs with SQL_C_DEFAULT and its values with the value's own C type retyped the parameter on its first NULL and never got it back. Every later row on that parameter was written as text into a numeric column, and — because the sqlvar's null flag was still set from the NULL row — reached the server as NULL, with SQL_SUCCESS and a correct row count. SQLFreeStmt(SQL_RESET_PARAMS) did not help: the state was in the sqlvar, not in the descriptors. SQLDescribeParam reported SQL_CHAR for the parameter as well.

SQL_INTEGER, SQL_SMALLINT and SQL_DOUBLE escaped only because their SQL_C_DEFAULT never resolves to a character C type.

The fix

Describe INPUT parameters from the prepare-time snapshot (orgSqlProperties) rather than the live sqlvar, which is what getColumnDisplaySize() already did. A bind then always sees the type the statement was prepared with, and setHeadSqlVar shapes the sqlvar back to it. OUTPUT columns keep reading the live sqlvar, which is never rewritten.

getPrecision is deliberately untouched here — this fix does not need it, and #292 already changes it.

Tests

Five tests in tests/test_param_conversions.cpp. Four fail on master and pass with the fix; NullAsDefaultThenIntegerRebind passes both ways and is the control that documents why SQL_INTEGER was unaffected.

Verified on Windows against Firebird 5.0.3 with the Microsoft driver manager, using the reproduction matrix from the issue (24 cells over NUMERIC(9,3), DECIMAL(18,2), BIGINT, INTEGER, SMALLINT, DOUBLE PRECISION): 6 lost cells on master, 0 with this branch. The full suite passes in all three charset configurations.

Still open

Two neighbouring defects on the numeric-to-string parameter path are not addressed here, and both are visible with a single binding whose indicator toggles NULL then value on a VARCHAR/CHAR parameter:

• ODBCCONVERT_CHECKNULL never clears the target null flag on the not-null path, unlike its two sibling macros. For a sqlda target that flag survives between executes, so a value after a NULL is sent as NULL.
• Behind it, the truncation from duckdb/odbc-scanner#161: with that flag cleared, master writes 6 for 60.

They belong with #292, which owns that code path; #292 as it stands fixes neither. Both now sit on that PR, with tests.

[fdcastel]

How this relates to #292, which touches the same code path.

This PR carries one commit: describe INPUT parameters from the prepare-time snapshot. That is the whole fix for #300, and it stands on master by itself — the five tests here fail on master and pass with it, and CI is green.

#292 now carries that same commit plus two more, because a second defect sits next to it and is only testable there:

ODBCCONVERT_CHECKNULL — the null check shared by the whole conv<Type>ToString family that #292 rewrites — sets the target null indicator on the null path but never clears it on the other one, unlike ODBCCONVERT_CHECKNULL_COMMON and ODBCCONVERT_CHECKNULL_SQLDA. For a sqlda target that indicator is the parameter's own null flag and it survives between executes, so a value bound after a NULL on the same parameter is sent as NULL with SQL_SUCCESS. This one needs no rebind and no SQL_C_DEFAULT: a single binding whose indicator toggles NULL then value on a VARCHAR/CHAR parameter is enough, which is the shape applications are told to use as the workaround for #300.

It is not in this PR because on master its payoff is invisible: clearing the flag only uncovers the truncation #292 fixes (master then writes 6 for 60). Measured on Windows against Firebird 5.0.3 with the MS driver manager — one binding, NULL row then value row, VARCHAR(20) and CHAR(20) × SQL_C_SLONG / SSHORT / SBIGINT / DOUBLE:

build	result
master	8/8 rows silently NULL
master + this PR	8/8 rows silently NULL
#292 alone	8/8 rows silently NULL
#292 + null-flag fix	8/8 correct

So the two PRs are independent and can merge in either order: whichever lands first makes the shared commit a no-op in the other.

[fdcastel]

@irodushka when you have a moment: this one is a single commit on current master, stands on its own, and its four tests fail on master and pass here.

The suggested merge order for an rc2 now lives in #312.

[singhpratech]

Tested on Linux with the linux-x64-binaries artifact from this PR's CI run (the driver reports 03.05.000), unixODBC 2.3.12, Firebird 5.0.4, CHARSET=UTF8, side by side with the 3.5.0-rc1 release build. It fixes #300 completely.

The two C programs from #300, PR build:

A: NULL row bound SQL_C_DEFAULT : expected v = 1, NULL, 3, 4; got v = 1 NULL 3 4
B: NULL row bound SQL_C_SBIGINT: expected v = 1, NULL, 3, 4; got v = 1 NULL 3 4

BIGINT, NULL@2 as SQL_C_DEFAULT, values SQL_C_SBIGINT (1,NULL,3,4) got v = 1 NULL 3 4
BIGINT, NULL@1 as SQL_C_DEFAULT, values SQL_C_SBIGINT (NULL,2,3,4) got v = NULL 2 3 4
BIGINT, NULL@2 as SQL_C_CHAR explicitly, values SQL_C_SBIGINT      got v = 1 NULL 3 4
BIGINT, no NULL at all, values SQL_C_SBIGINT (control)             got v = 1 2 3 4
INTEGER, NULL@2 as SQL_C_DEFAULT, values SQL_C_SLONG (control)     got v = 1 NULL 3 4

Every row is the expected one, including NULL-first and the explicit SQL_C_CHAR NULL. The release build, run in the same session, still prints 1 NULL NULL NULL and NULL NULL NULL NULL for the first three.

Your Python matrix from #300, through unixODBC (ODBC_LIB=libodbc.so.2):

build	LOST cells (of 24)
3.5.0-rc1 release	6: shapes A and C on NUMERIC(9,3), DECIMAL(18,2) and BIGINT, the same six as on Windows
this PR	0

So the fix holds on both driver managers, and the SQL_RESET_PARAMS case (shape C) is covered too.

Thank you, sincerely. In one day you took a report that only said "BIGINT, Linux, cause unknown", reproduced it on a second platform, widened it to the two numeric types it was really about, found the exact lines, and shipped a fix with regression tests that pin every shape in your matrix. Anyone loading numeric data into Firebird through ODBC with NULLs in it, from DuckDB, from an ADBC bridge, from a plain C program, will have you to thank for their rows arriving intact and never know it. I will note the fix in the compatibility record I keep, with your name on it, and re-run against the release that carries it when it ships.

(Where this comes from: adbcBridge, an ADBC driver over ODBC that runs Firebird in its compatibility matrix on Linux and Windows, https://github.com/singhpratech/adbcbridge. Its Firebird row and upstream tracker now record this fix.)