Stringify numeric parameters bound to character columns (re: #163)

src/include/scanner_value.hpp

@@ -163,6 +163,13 @@ class ScannerValue {
 
 	void TransformIntegralToDecimal();
 
+	// Stringifies an integral/float parameter in-place and re-tags it as
+	// TYPE_DECIMAL_AS_CHARS so the scanner can bind the value as SQL_C_CHAR.
+	// This lets us avoid driver code paths that convert numeric-C → character-SQL,
+	// which are historically a common source of silent data corruption across
+	// ODBC drivers (e.g. Firebird ODBC ≤ 3.5.0, some MSSQL/MySQL releases).
+	void TransformNumericToChars();
+
 private:
 	void CheckType(param_type expected);
 

src/include/types.hpp

@@ -42,6 +42,11 @@ struct Types {
 
 	static const std::string UNKNOWN_DUCKDB_TYPE_NAME;
 
+	static bool IsCharacterSQLType(SQLSMALLINT t) {
+		return t == SQL_CHAR || t == SQL_VARCHAR || t == SQL_LONGVARCHAR || t == SQL_WCHAR || t == SQL_WVARCHAR ||
+		       t == SQL_WLONGVARCHAR;
+	}
+
 	static const SQLSMALLINT SQL_SS_TIME2 = -154;
 	static const SQLSMALLINT SQL_SS_TIMESTAMPOFFSET = -155;
 

src/params.cpp

@@ -134,6 +134,35 @@ std::vector<SQLSMALLINT> Params::CollectTypes(QueryContext &ctx) {
 	return param_types;
 }
 
+// When the prepared-parameter's target is a character SQL type and the source is
+// numeric, stringify in the scanner. This short-circuits the driver's
+// numeric-C → character-SQL conversion path, which has produced silent data loss
+// in multiple ODBC drivers (FirebirdSQL/firebird-odbc-driver#292 and older
+// MSSQL/MySQL releases). Done here — before binding — so that post-SetExpectedTypes
+// the param's type_id is final, which lets the single-row bind cache compare shapes
+// reliably.
+static void CoalesceNumericToCharsIfNeeded(ScannerValue &param) {
+	if (!Types::IsCharacterSQLType(param.ExpectedType())) {
+		return;
+	}
+	switch (param.ParamType()) {
+	case DUCKDB_TYPE_TINYINT:
+	case DUCKDB_TYPE_UTINYINT:
+	case DUCKDB_TYPE_SMALLINT:
+	case DUCKDB_TYPE_USMALLINT:
+	case DUCKDB_TYPE_INTEGER:
+	case DUCKDB_TYPE_UINTEGER:
+	case DUCKDB_TYPE_BIGINT:
+	case DUCKDB_TYPE_UBIGINT:
+	case DUCKDB_TYPE_FLOAT:
+	case DUCKDB_TYPE_DOUBLE:
+		param.TransformNumericToChars();
+		break;
+	default:
+		break;
+	}
+}
+
 void Params::SetExpectedTypes(QueryContext &ctx, const std::vector<SQLSMALLINT> &expected,
                               std::vector<ScannerValue> &actual) {
 	if (expected.size() != actual.size()) {
@@ -145,6 +174,7 @@ void Params::SetExpectedTypes(QueryContext &ctx, const std::vector<SQLSMALLINT>
 		ScannerValue &param = actual.at(i);
 		Types::CoalesceParameterType(ctx, param);
 		param.SetExpectedType(expected_type);
+		CoalesceNumericToCharsIfNeeded(param);
 	}
 }
 

src/scanner_value.cpp

@@ -1,5 +1,6 @@
 #include "scanner_value.hpp"
 
+#include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <string>
@@ -522,4 +523,58 @@ void ScannerValue::TransformIntegralToDecimal() {
 	*this = ScannerValue(dec, false);
 }
 
+void ScannerValue::TransformNumericToChars() {
+	std::string str;
+	switch (type_id) {
+	case DUCKDB_TYPE_TINYINT:
+		str = std::to_string(static_cast<int32_t>(Value<int8_t>()));
+		break;
+	case DUCKDB_TYPE_UTINYINT:
+		str = std::to_string(static_cast<uint32_t>(Value<uint8_t>()));
+		break;
+	case DUCKDB_TYPE_SMALLINT:
+		str = std::to_string(Value<int16_t>());
+		break;
+	case DUCKDB_TYPE_USMALLINT:
+		str = std::to_string(Value<uint16_t>());
+		break;
+	case DUCKDB_TYPE_INTEGER:
+		str = std::to_string(Value<int32_t>());
+		break;
+	case DUCKDB_TYPE_UINTEGER:
+		str = std::to_string(Value<uint32_t>());
+		break;
+	case DUCKDB_TYPE_BIGINT:
+		str = std::to_string(Value<int64_t>());
+		break;
+	case DUCKDB_TYPE_UBIGINT:
+		str = std::to_string(Value<uint64_t>());
+		break;
+	case DUCKDB_TYPE_FLOAT: {
+		char buf[32];
+		std::snprintf(buf, sizeof(buf), "%.9g", static_cast<double>(Value<float>()));
+		str = buf;
+		break;
+	}
+	case DUCKDB_TYPE_DOUBLE: {
+		char buf[32];
+		std::snprintf(buf, sizeof(buf), "%.17g", Value<double>());
+		str = buf;
+		break;
+	}
+	default:
+		throw ScannerException("Invalid numeric param type for chars transform: " + std::to_string(type_id));
+	}
+
+	// Destroy the current (POD) value and repurpose the union as DecimalChars.
+	this->Destroy();
+	this->type_id = Params::TYPE_DECIMAL_AS_CHARS;
+	new (&this->val.decimal_chars) DecimalChars;
+	DecimalChars &dc = this->val.decimal_chars;
+	dc.characters.resize(str.size() + 1);
+	std::memcpy(dc.characters.data(), str.data(), str.size());
+	dc.characters[str.size()] = '\0';
+	this->len_bytes = static_cast<SQLLEN>(str.size());
+}
+
 } // namespace odbcscanner

src/types/decimal_type.cpp

@@ -82,10 +82,15 @@ void TypeSpecific::BindOdbcParam<SQL_NUMERIC_STRUCT>(QueryContext &ctx, ScannerV
 
 template <>
 void TypeSpecific::BindOdbcParam<DecimalChars>(QueryContext &ctx, ScannerValue &param, SQLSMALLINT param_idx) {
-	SQLSMALLINT sqltype = param.ExpectedType() != SQL_PARAM_TYPE_UNKNOWN ? param.ExpectedType() : SQL_NUMERIC;
+	SQLSMALLINT expected = param.ExpectedType();
+	// Preserve the driver's column type when it is a CHAR/VARCHAR/WCHAR family:
+	// binding SQL_C_CHAR → the actual expected type avoids an extra driver-side
+	// coercion. For non-character expected types (e.g. SQL_NUMERIC), fall back to
+	// SQL_VARCHAR — the driver then parses the char buffer into the target type.
+	SQLSMALLINT sqltype = Types::IsCharacterSQLType(expected) ? expected : SQL_VARCHAR;
 	DecimalChars &dc = param.Value<DecimalChars>();
 	SQLRETURN ret =
-	    SQLBindParameter(ctx.hstmt(), param_idx, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_VARCHAR, param.LengthBytes(), 0,
+	    SQLBindParameter(ctx.hstmt(), param_idx, SQL_PARAM_INPUT, SQL_C_CHAR, sqltype, param.LengthBytes(), 0,
 	                     reinterpret_cast<SQLPOINTER>(dc.data()), param.LengthBytes(), &param.LengthBytes());
 	if (!SQL_SUCCEEDED(ret)) {
 		std::string diag = Diagnostics::Read(ctx.hstmt(), SQL_HANDLE_STMT);

src/types/float_types.cpp

@@ -54,6 +54,10 @@ static void BindOdbcParamInternal(QueryContext &ctx, SQLSMALLINT ctype, SQLSMALL
 	}
 }
 
+// If the expected SQL type is character, Params::SetExpectedTypes transforms the
+// ScannerValue to TYPE_DECIMAL_AS_CHARS before we get here — see the comment in
+// integer_types.cpp for the rationale.
+
 template <>
 void TypeSpecific::BindOdbcParam<float>(QueryContext &ctx, ScannerValue &param, SQLSMALLINT param_idx) {
 	SQLSMALLINT sqltype = param.ExpectedType() != SQL_PARAM_TYPE_UNKNOWN ? param.ExpectedType() : SQL_FLOAT;

src/types/integer_types.cpp

@@ -174,6 +174,15 @@ static SQLSMALLINT IntegralSQLType(ScannerValue &param, SQLSMALLINT def_sqltype)
 	return def_sqltype;
 }
 
+// Note: if the prepared-parameter's expected SQL type is a character type
+// (CHAR / VARCHAR / WCHAR family), the ScannerValue is expected to have been
+// transformed to TYPE_DECIMAL_AS_CHARS by Params::SetExpectedTypes before we get
+// here — dispatch will then route to BindOdbcParam<DecimalChars> instead of one of
+// the integer specializations below. Stringifying in the scanner avoids the
+// driver's numeric-C → character-SQL path, which has shipped silent-corruption bugs
+// across several ODBC drivers (Firebird ≤ 3.5.0 in
+// FirebirdSQL/firebird-odbc-driver#292; older MSSQL / MySQL releases too).
+
 template <>
 void TypeSpecific::BindOdbcParam<int8_t>(QueryContext &ctx, ScannerValue &param, SQLSMALLINT param_idx) {
 	SQLSMALLINT sqltype = IntegralSQLType(param, SQL_TINYINT);

test/test_copy.cpp

@@ -142,3 +142,114 @@ TEST_CASE("Copy from file into Oracle with table creation", group_name) {
 		REQUIRE(QuerySuccess(res.Get(), st));
 	}
 }
+
+// Regression test for duckdb/odbc-scanner#161 / FirebirdSQL/firebird-odbc-driver#292:
+// copying integer source rows into a VARCHAR primary-key column hit the ODBC driver's
+// least-exercised numeric-C → character-SQL code path and caused silent data loss on
+// the Firebird driver. The bug was silent from ODBC's perspective — every call
+// returned SUCCESS — so a round-trip test that reads the stored VARCHAR back and
+// compares to the source integer as a string is needed to surface regressions.
+// batch_size=1 forces the single-row bind path (SetExpectedTypes + BindToOdbc per
+// row) which is the shape that exposed the original bug.
+TEST_CASE("Copy integer source into VARCHAR primary key", group_name) {
+	// Skip:
+	//  - NoSQL-ish / cloud-only drivers (no ad-hoc DDL): FlightSQL, Spark, Snowflake, ClickHouse.
+	//  - MySQL / MariaDB: CI connection string does not select a default database, so
+	//    `CREATE TABLE <name>` fails outright with "No database selected". The scanner
+	//    bug this test guards against is not MySQL-specific and the other drivers
+	//    cover it, so adding a database-selection dance just for this test is not
+	//    worth the maintenance cost.
+	if (DBMSConfigured("FlightSQL") || DBMSConfigured("Spark") || DBMSConfigured("Snowflake") ||
+	    DBMSConfigured("ClickHouse") || DBMSConfigured("MySQL") || DBMSConfigured("MariaDB")) {
+		return;
+	}
+
+	const std::string table_name = "int_to_varchar_pk_test";
+
+	std::string varchar_type = "VARCHAR(20)";
+	if (DBMSConfigured("Oracle")) {
+		varchar_type = "VARCHAR2(20)";
+	}
+
+	// ignore_exec_failure only rescues SQLExecute failures; DuckDB's ODBC driver
+	// (and some others) reject DROP TABLE <missing> at SQLPrepare. Drivers that
+	// do accept "IF EXISTS" syntax take that branch; the rest keep the classic
+	// ignore-on-exec path.
+	bool supports_if_exists = DBMSConfigured("DuckDB") || DBMSConfigured("PostgreSQL") || DBMSConfigured("MSSQL");
+
+	ScannerConn sc;
+	{
+		Result res;
+		std::string drop_sql = supports_if_exists ? "DROP TABLE IF EXISTS " + table_name : "DROP TABLE " + table_name;
+		std::string ignore_opt = supports_if_exists ? "" : ", ignore_exec_failure=TRUE";
+		std::string sql = "SELECT * FROM odbc_query(getvariable('conn'), '" + drop_sql + "'" + ignore_opt + ")";
+		duckdb_state st = duckdb_query(sc.conn, sql.c_str(), res.Get());
+		REQUIRE(QuerySuccess(res.Get(), st));
+	}
+	{
+		Result res;
+		std::string sql = "SELECT * FROM odbc_query(getvariable('conn'), 'CREATE TABLE " + table_name + " (id " +
+		                  varchar_type + " NOT NULL PRIMARY KEY)')";
+		duckdb_state st = duckdb_query(sc.conn, sql.c_str(), res.Get());
+		REQUIRE(QuerySuccess(res.Get(), st));
+	}
+
+	// Copy 500 integers into the VARCHAR PK column with batch_size=1 to force
+	// the per-row rebind shape that exposed the Firebird silent-corruption bug.
+	// column_quotes='' keeps the INSERT column names unquoted so drivers with
+	// case-sensitive quoted identifiers (Oracle, DB2, Firebird — which fold
+	// unquoted names to uppercase but keep quoted names case-sensitive) still
+	// match the "id" column created above.
+	// rows_processed comes back via the DuckDB side (always BIGINT), so the row-count
+	// assertion does not depend on per-driver DECIMAL/NUMBER coercion — it catches
+	// "row loss" regressions where dedup / UPDATE-OR-INSERT MATCHING collapses
+	// distinct PKs.
+	{
+		Result res;
+		// rows_processed is UBIGINT; sum() collapses any multi-chunk streaming into
+		// a single scalar, and ::DECIMAL(18, 0) lines the column up with
+		// Result::DecimalValue<int64_t>, which is the one int64 accessor that works
+		// uniformly across all drivers here (Result::Value<int64_t> routes through
+		// driver-specific type expectations).
+		std::string sql = std::string() +
+		                  "SELECT sum(rows_processed)::DECIMAL(18, 0) FROM odbc_copy(getvariable('conn'),\n"
+		                  "  dest_table='" +
+		                  table_name +
+		                  "',\n"
+		                  "  batch_size=1,\n"
+		                  "  column_quotes='',\n"
+		                  "  source_query='SELECT i::INTEGER AS id FROM range(1, 501) t(i)')";
+		duckdb_state st = duckdb_query(sc.conn, sql.c_str(), res.Get());
+		REQUIRE(QuerySuccess(res.Get(), st));
+		REQUIRE(res.NextChunk());
+		REQUIRE(res.DecimalValue<int64_t>(0, 0) == 500);
+	}
+
+	// The critical assertion: stored VARCHAR values must equal their source
+	// integers as decimal strings. Row-count alone was not enough — the
+	// Firebird bug stored 500 *corrupted* (NUL-byte) values without any driver
+	// error for one execute shape, and collapsed to 11 distinct values for
+	// another. Check a sample of small, multi-digit, and boundary values.
+	{
+		Result res;
+		std::string inner_sql =
+		    std::string() + "SELECT id FROM " + table_name + " WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id";
+		std::string sql = std::string() + "SELECT id FROM odbc_query(getvariable('conn'), '" + inner_sql + "')";
+		duckdb_state st = duckdb_query(sc.conn, sql.c_str(), res.Get());
+		REQUIRE(QuerySuccess(res.Get(), st));
+		REQUIRE(res.NextChunk());
+		// Lexicographic sort: '1','2','42','500'.
+		REQUIRE(res.Value<std::string>(0, 0) == "1");
+		REQUIRE(res.Value<std::string>(0, 1) == "2");
+		REQUIRE(res.Value<std::string>(0, 2) == "42");
+		REQUIRE(res.Value<std::string>(0, 3) == "500");
+	}
+
+	{
+		Result res;
+		std::string drop_sql = supports_if_exists ? "DROP TABLE IF EXISTS " + table_name : "DROP TABLE " + table_name;
+		std::string sql = "SELECT * FROM odbc_query(getvariable('conn'), '" + drop_sql + "')";
+		duckdb_state st = duckdb_query(sc.conn, sql.c_str(), res.Get());
+		REQUIRE(QuerySuccess(res.Get(), st));
+	}
+}