diff --git a/src/binary.cpp b/src/binary.cpp index 8bd3f77..414e61b 100644 --- a/src/binary.cpp +++ b/src/binary.cpp @@ -29,6 +29,10 @@ char *DecimalChars::data() { return characters.data(); } +SQLWCHAR *DecimalChars::wide_data() { + return wide_characters.data(); +} + ScannerBlob::ScannerBlob() { } diff --git a/src/include/binary.hpp b/src/include/binary.hpp index d317401..eabeee0 100644 --- a/src/include/binary.hpp +++ b/src/include/binary.hpp @@ -4,11 +4,17 @@ #include #include "duckdb_extension_api.hpp" +#include "odbc_api.hpp" namespace odbcscanner { struct DecimalChars { std::vector characters; + // Optional wide buffer populated by BindOdbcParam when the + // prepared parameter's expected SQL type is SQL_WCHAR / SQL_WVARCHAR / + // SQL_WLONGVARCHAR. Kept alongside `characters` so the binding's lifetime + // matches the ScannerValue. + std::vector wide_characters; DecimalChars(); @@ -26,6 +32,8 @@ struct DecimalChars { } char *data(); + + SQLWCHAR *wide_data(); }; struct ScannerBlob { diff --git a/src/include/scanner_value.hpp b/src/include/scanner_value.hpp index 8028492..5343938 100644 --- a/src/include/scanner_value.hpp +++ b/src/include/scanner_value.hpp @@ -154,6 +154,8 @@ class ScannerValue { SQLLEN &LengthBytes(); + void SetLengthBytes(SQLLEN value); + SQLSMALLINT ExpectedType(); void SetExpectedType(SQLSMALLINT expected_type_in); @@ -163,9 +165,21 @@ class ScannerValue { void TransformIntegralToDecimal(); + // Stringifies an integral/float parameter in-place and re-tags it as + // TYPE_DECIMAL_AS_CHARS so the scanner binds via BindOdbcParam + // instead of a numeric C type. This avoids 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). Wide character columns (SQL_WCHAR/SQL_WVARCHAR/ + // SQL_WLONGVARCHAR) are handled by BindOdbcParam itself, which + // widens the buffer on demand. + void TransformNumericToChars(); + private: void CheckType(param_type expected); + std::string NumericToString(); + static void AssignByType(param_type type_id, InternalValue &val, ScannerValue &other); }; diff --git a/src/include/types.hpp b/src/include/types.hpp index 3953611..df3bb67 100644 --- a/src/include/types.hpp +++ b/src/include/types.hpp @@ -42,6 +42,10 @@ struct Types { static const std::string UNKNOWN_DUCKDB_TYPE_NAME; + static bool IsCharacterSQLType(SQLSMALLINT t); + + static bool IsWideCharacterSQLType(SQLSMALLINT t); + static const SQLSMALLINT SQL_SS_TIME2 = -154; static const SQLSMALLINT SQL_SS_TIMESTAMPOFFSET = -155; diff --git a/src/params.cpp b/src/params.cpp index cb5fd4a..9819d77 100644 --- a/src/params.cpp +++ b/src/params.cpp @@ -134,6 +134,35 @@ std::vector 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. Wide targets (SQL_WCHAR/WVARCHAR/WLONGVARCHAR) +// are handled by BindOdbcParam, which widens the buffer on demand. +static void CoalesceNumericToCharsIfNeeded(ScannerValue ¶m) { + 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 &expected, std::vector &actual) { if (expected.size() != actual.size()) { @@ -145,6 +174,7 @@ void Params::SetExpectedTypes(QueryContext &ctx, const std::vector ScannerValue ¶m = actual.at(i); Types::CoalesceParameterType(ctx, param); param.SetExpectedType(expected_type); + CoalesceNumericToCharsIfNeeded(param); } } diff --git a/src/scanner_value.cpp b/src/scanner_value.cpp index 7d182bd..46a8637 100644 --- a/src/scanner_value.cpp +++ b/src/scanner_value.cpp @@ -1,5 +1,6 @@ #include "scanner_value.hpp" +#include #include #include #include @@ -450,6 +451,10 @@ SQLLEN &ScannerValue::LengthBytes() { return len_bytes; } +void ScannerValue::SetLengthBytes(SQLLEN value) { + this->len_bytes = value; +} + SQLSMALLINT ScannerValue::ExpectedType() { return expected_type; } @@ -522,4 +527,52 @@ void ScannerValue::TransformIntegralToDecimal() { *this = ScannerValue(dec, false); } +std::string ScannerValue::NumericToString() { + switch (type_id) { + case DUCKDB_TYPE_TINYINT: + return std::to_string(static_cast(Value())); + case DUCKDB_TYPE_UTINYINT: + return std::to_string(static_cast(Value())); + case DUCKDB_TYPE_SMALLINT: + return std::to_string(Value()); + case DUCKDB_TYPE_USMALLINT: + return std::to_string(Value()); + case DUCKDB_TYPE_INTEGER: + return std::to_string(Value()); + case DUCKDB_TYPE_UINTEGER: + return std::to_string(Value()); + case DUCKDB_TYPE_BIGINT: + return std::to_string(Value()); + case DUCKDB_TYPE_UBIGINT: + return std::to_string(Value()); + case DUCKDB_TYPE_FLOAT: { + char buf[32]; + std::snprintf(buf, sizeof(buf), "%.9g", static_cast(Value())); + return std::string(buf); + } + case DUCKDB_TYPE_DOUBLE: { + char buf[32]; + std::snprintf(buf, sizeof(buf), "%.17g", Value()); + return std::string(buf); + } + default: + throw ScannerException("Invalid numeric param type for chars transform: " + std::to_string(type_id)); + } +} + +void ScannerValue::TransformNumericToChars() { + std::string str = NumericToString(); + + // Destroy the current (POD) value and repurpose the union as DecimalChars. + // Wide character targets are handled downstream by BindOdbcParam. + 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(str.size()); +} + } // namespace odbcscanner diff --git a/src/types/decimal_type.cpp b/src/types/decimal_type.cpp index 7869157..9a51596 100644 --- a/src/types/decimal_type.cpp +++ b/src/types/decimal_type.cpp @@ -9,6 +9,7 @@ #include "connection.hpp" #include "diagnostics.hpp" #include "scanner_exception.hpp" +#include "widechar.hpp" DUCKDB_EXTENSION_EXTERN @@ -82,10 +83,37 @@ void TypeSpecific::BindOdbcParam(QueryContext &ctx, ScannerV template <> void TypeSpecific::BindOdbcParam(QueryContext &ctx, ScannerValue ¶m, 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 → the actual expected type avoids an extra driver-side coercion. + // For non-character expected types (e.g. SQL_NUMERIC), fall back to SQL_VARCHAR + // and let the driver parse the char buffer into the target type. + SQLSMALLINT sqltype = Types::IsCharacterSQLType(expected) ? expected : SQL_VARCHAR; DecimalChars &dc = param.Value(); + + // Wide targets: widen the ASCII digits to SQLWCHAR and bind SQL_C_WCHAR. + // Some drivers do not reliably auto-convert SQL_C_CHAR → SQL_W*CHAR. + if (Types::IsWideCharacterSQLType(expected)) { + if (dc.wide_characters.empty()) { + WideString wstr = WideChar::Widen(dc.data(), dc.size()); + dc.wide_characters = std::move(wstr.vec); + } + SQLLEN length_chars = static_cast(dc.wide_characters.size() - 1); + param.SetLengthBytes(length_chars * static_cast(sizeof(SQLWCHAR))); + SQLRETURN ret = SQLBindParameter( + ctx.hstmt(), param_idx, SQL_PARAM_INPUT, SQL_C_WCHAR, sqltype, static_cast(length_chars), 0, + reinterpret_cast(dc.wide_data()), param.LengthBytes(), ¶m.LengthBytes()); + if (!SQL_SUCCEEDED(ret)) { + std::string diag = Diagnostics::Read(ctx.hstmt(), SQL_HANDLE_STMT); + throw ScannerException("'SQLBindParameter' failed, type: " + std::to_string(sqltype) + + ", index: " + std::to_string(param_idx) + ", query: '" + ctx.query + + "', return: " + std::to_string(ret) + ", diagnostics: '" + diag + "'"); + } + return; + } + 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(dc.data()), param.LengthBytes(), ¶m.LengthBytes()); if (!SQL_SUCCEEDED(ret)) { std::string diag = Diagnostics::Read(ctx.hstmt(), SQL_HANDLE_STMT); diff --git a/src/types/float_types.cpp b/src/types/float_types.cpp index bb36a9e..52423ec 100644 --- a/src/types/float_types.cpp +++ b/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(QueryContext &ctx, ScannerValue ¶m, SQLSMALLINT param_idx) { SQLSMALLINT sqltype = param.ExpectedType() != SQL_PARAM_TYPE_UNKNOWN ? param.ExpectedType() : SQL_FLOAT; diff --git a/src/types/integer_types.cpp b/src/types/integer_types.cpp index ad10c31..38a881d 100644 --- a/src/types/integer_types.cpp +++ b/src/types/integer_types.cpp @@ -174,6 +174,15 @@ static SQLSMALLINT IntegralSQLType(ScannerValue ¶m, 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 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(QueryContext &ctx, ScannerValue ¶m, SQLSMALLINT param_idx) { SQLSMALLINT sqltype = IntegralSQLType(param, SQL_TINYINT); diff --git a/src/types/types.cpp b/src/types/types.cpp index 36a3327..84581c6 100644 --- a/src/types/types.cpp +++ b/src/types/types.cpp @@ -32,6 +32,15 @@ const std::string Types::SQL_BLOB_TYPE_NAME = "BLOB"; const std::string Types::SQL_CLOB_TYPE_NAME = "CLOB"; const std::string Types::SQL_DB2_DBCLOB_TYPE_NAME = "DBCLOB"; +bool Types::IsCharacterSQLType(SQLSMALLINT t) { + return t == SQL_CHAR || t == SQL_VARCHAR || t == SQL_LONGVARCHAR || t == SQL_WCHAR || t == SQL_WVARCHAR || + t == SQL_WLONGVARCHAR; +} + +bool Types::IsWideCharacterSQLType(SQLSMALLINT t) { + return t == SQL_WCHAR || t == SQL_WVARCHAR || t == SQL_WLONGVARCHAR; +} + ScannerValue Types::ExtractNotNullParam(DbmsQuirks &quirks, duckdb_type type_id, duckdb_vector vec, idx_t row_idx, idx_t param_idx) { switch (type_id) { diff --git a/test/sql/db2/db2_copy.test b/test/sql/db2/db2_copy.test index 6887c97..866a3d1 100644 --- a/test/sql/db2/db2_copy.test +++ b/test/sql/db2/db2_copy.test @@ -400,5 +400,37 @@ NULL NULL 11 s11 NULL NULL NULL NULL statement ok SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE DUCKDB_TEST_COPY') +# int → VARCHAR primary key round-trip (regression test for duckdb/odbc-scanner#161). +# DB2 folds unquoted identifiers to upper case; column_quotes='' keeps the INSERT +# column names unquoted so `id` resolves to the `ID` column created here. + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK', ignore_exec_failure=TRUE) + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), + 'CREATE TABLE INT_TO_VARCHAR_PK (id VARCHAR(20) NOT NULL PRIMARY KEY)') + +query II +SELECT completed, rows_processed FROM odbc_copy(getvariable('conn'), + dest_table='INT_TO_VARCHAR_PK', + batch_size=1, + column_quotes='', + source_query='SELECT i::INTEGER AS id FROM range(1, 501) t(i)') +---- +1 500 + +query I +SELECT * FROM odbc_query(getvariable('conn'), + 'SELECT id FROM INT_TO_VARCHAR_PK WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id') +---- +1 +2 +42 +500 + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK') + statement ok SELECT odbc_close(getvariable('conn')) diff --git a/test/sql/duckdb/duckdb_copy.test b/test/sql/duckdb/duckdb_copy.test index 1254682..ffa5dc3 100644 --- a/test/sql/duckdb/duckdb_copy.test +++ b/test/sql/duckdb/duckdb_copy.test @@ -88,5 +88,39 @@ _foo_col1_foo_ statement ok SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE duckdb_test_copy') +# int → VARCHAR primary key round-trip (regression test for duckdb/odbc-scanner#161). +# The Firebird ODBC driver ≤ 3.5.0 silently dropped rows on this exact shape; +# Params::SetExpectedTypes now stringifies the parameter in the scanner, which +# sidesteps the driver's numeric-C → character-SQL path on every driver. +# batch_size=1 exercises the per-row bind that was the most fragile shape. + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE IF EXISTS duckdb_int_to_varchar_pk') + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), + 'CREATE TABLE duckdb_int_to_varchar_pk (id VARCHAR(20) NOT NULL PRIMARY KEY)') + +query II +SELECT completed, rows_processed FROM odbc_copy(getvariable('conn'), + dest_table='duckdb_int_to_varchar_pk', + batch_size=1, + column_quotes='', + source_query='SELECT i::INTEGER AS id FROM range(1, 501) t(i)') +---- +1 500 + +query I +SELECT * FROM odbc_query(getvariable('conn'), + 'SELECT id FROM duckdb_int_to_varchar_pk WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id') +---- +1 +2 +42 +500 + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE duckdb_int_to_varchar_pk') + statement ok SELECT odbc_close(getvariable('conn')) diff --git a/test/sql/firebird/firebird_copy.test b/test/sql/firebird/firebird_copy.test index 8e38f7f..7c8fcff 100644 --- a/test/sql/firebird/firebird_copy.test +++ b/test/sql/firebird/firebird_copy.test @@ -397,5 +397,43 @@ NULL NULL 11 s11 NULL NULL NULL NULL statement ok SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE DUCKDB_TEST_COPY') +# int → VARCHAR primary key round-trip (regression test for duckdb/odbc-scanner#161) +# +# Before the scanner stringified numeric parameters bound to a character column, +# this exact shape on the Firebird ODBC driver ≤ 3.5.0 silently stored 11 rows +# with NUL-byte-corrupted PK values. We exercise batch_size=1 because that was +# the shape the original bug dropped rows on; column_quotes='' keeps the INSERT +# column names unquoted so Firebird matches the `ID` column created below. + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK', ignore_exec_failure=TRUE) + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'CREATE TABLE INT_TO_VARCHAR_PK (id VARCHAR(20) NOT NULL PRIMARY KEY)') + +query II +SELECT completed, rows_processed FROM odbc_copy(getvariable('conn'), + dest_table='INT_TO_VARCHAR_PK', + batch_size=1, + column_quotes='', + source_query='SELECT i::INTEGER AS id FROM range(1, 501) t(i)') +---- +1 500 + +# Lexicographic sort: '1','2','42','500'. Checking sampled values guards against +# the original bug, which stored distinct but corrupted strings (NUL bytes) while +# returning SUCCESS from every driver call. +query I +SELECT * FROM odbc_query(getvariable('conn'), + 'SELECT id FROM INT_TO_VARCHAR_PK WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id') +---- +1 +2 +42 +500 + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK') + statement ok SELECT odbc_close(getvariable('conn')) diff --git a/test/sql/mssql/mssql_copy.test b/test/sql/mssql/mssql_copy.test index 23fb175..4c1e488 100644 --- a/test/sql/mssql/mssql_copy.test +++ b/test/sql/mssql/mssql_copy.test @@ -474,5 +474,40 @@ SELECT * FROM odbc_query(getvariable('conn'), 'SELECT * FROM ##duckdb_test_copy_ statement ok SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE ##duckdb_test_copy_temp_global') +# int → VARCHAR primary key round-trip (regression test for duckdb/odbc-scanner#161). +# batch_size=1 forces the single-row bind shape that exposed the original Firebird +# silent-corruption bug; the scanner now stringifies the parameter so every driver +# skips its numeric-C → character-SQL path. The NVARCHAR column additionally +# exercises the wide (SQL_C_WCHAR) branch of BindOdbcParam. + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), + 'IF OBJECT_ID(''int_to_varchar_pk'', ''U'') IS NOT NULL DROP TABLE int_to_varchar_pk') + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), + 'CREATE TABLE int_to_varchar_pk (id VARCHAR(20) NOT NULL PRIMARY KEY, id_n NVARCHAR(20) NOT NULL)') + +query II +SELECT completed, rows_processed FROM odbc_copy(getvariable('conn'), + dest_table='int_to_varchar_pk', + batch_size=1, + column_quotes='', + source_query='SELECT i::INTEGER AS id, i::INTEGER AS id_n FROM range(1, 501) t(i)') +---- +1 500 + +query II +SELECT * FROM odbc_query(getvariable('conn'), + 'SELECT id, id_n FROM int_to_varchar_pk WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id') +---- +1 1 +2 2 +42 42 +500 500 + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE int_to_varchar_pk') + statement ok SELECT odbc_close(getvariable('conn')) diff --git a/test/sql/oracle/oracle_copy.test b/test/sql/oracle/oracle_copy.test index bdaa004..9f415a6 100644 --- a/test/sql/oracle/oracle_copy.test +++ b/test/sql/oracle/oracle_copy.test @@ -487,5 +487,38 @@ SELECT * FROM odbc_query(getvariable('conn'), 'SELECT sum("col1") FROM DUCKDB_TE statement ok SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE DUCKDB_TEST_COPY') +# int → VARCHAR2 primary key round-trip (regression test for duckdb/odbc-scanner#161). +# Oracle folds unquoted identifiers to upper case; column_quotes='' keeps the +# INSERT column names unquoted so the INSERT's `id` resolves to the `ID` column +# created here. + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK', ignore_exec_failure=TRUE) + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), + 'CREATE TABLE INT_TO_VARCHAR_PK (id VARCHAR2(20) NOT NULL PRIMARY KEY)') + +query II +SELECT completed, rows_processed FROM odbc_copy(getvariable('conn'), + dest_table='INT_TO_VARCHAR_PK', + batch_size=1, + column_quotes='', + source_query='SELECT i::INTEGER AS id FROM range(1, 501) t(i)') +---- +1 500 + +query I +SELECT * FROM odbc_query(getvariable('conn'), + 'SELECT id FROM INT_TO_VARCHAR_PK WHERE id IN (''1'', ''2'', ''42'', ''500'') ORDER BY id') +---- +1 +2 +42 +500 + +statement ok +SELECT * FROM odbc_query(getvariable('conn'), 'DROP TABLE INT_TO_VARCHAR_PK') + statement ok SELECT odbc_close(getvariable('conn'))