Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/include/scanner_value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,16 @@ class ScannerValue {

void TransformIntegralToDecimal();

// Stringifies an integral/float parameter in-place so the scanner can bind
// the value as SQL_C_CHAR (narrow) or SQL_C_WCHAR (wide) 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).
// When `wide` is true the buffer is widened to UTF-16 and the value is re-tagged
// as DUCKDB_TYPE_VARCHAR (→ SQL_C_WCHAR path); otherwise it is re-tagged as
// TYPE_DECIMAL_AS_CHARS (→ SQL_C_CHAR path).
void TransformNumericToChars(bool wide);
Comment thread
fdcastel marked this conversation as resolved.
Outdated

private:
void CheckType(param_type expected);

Expand Down
4 changes: 4 additions & 0 deletions src/include/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
32 changes: 32 additions & 0 deletions src/params.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,37 @@ 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. Wide targets (SQL_WCHAR/WVARCHAR/WLONGVARCHAR) are
// widened to UTF-16 so the bind uses SQL_C_WCHAR; some drivers do not reliably
// auto-convert SQL_C_CHAR → SQL_WVARCHAR.
static void CoalesceNumericToCharsIfNeeded(ScannerValue &param) {
SQLSMALLINT expected = param.ExpectedType();
if (!Types::IsCharacterSQLType(expected)) {
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(Types::IsWideCharacterSQLType(expected));
break;
default:
break;
}
}

void Params::SetExpectedTypes(QueryContext &ctx, const std::vector<SQLSMALLINT> &expected,
std::vector<ScannerValue> &actual) {
if (expected.size() != actual.size()) {
Expand All @@ -145,6 +176,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);
}
}

Expand Down
67 changes: 67 additions & 0 deletions src/scanner_value.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "scanner_value.hpp"

#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <string>
Expand Down Expand Up @@ -522,4 +523,70 @@ void ScannerValue::TransformIntegralToDecimal() {
*this = ScannerValue(dec, false);
}

void ScannerValue::TransformNumericToChars(bool wide) {
std::string str;
switch (type_id) {
Comment thread
fdcastel marked this conversation as resolved.
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));
}

if (wide) {
// Widen to UTF-16 and re-tag as DUCKDB_TYPE_VARCHAR so Types::BindOdbcParam
// routes to the SQL_C_WCHAR specialization. Numeric digits are pure ASCII,
// so WideChar::Widen never produces invalid sequences here.
WideString wstr = WideChar::Widen(str.data(), str.size());
this->Destroy();
this->type_id = DUCKDB_TYPE_VARCHAR;
new (&this->val.wstr) WideString;
this->val.wstr = std::move(wstr);
this->len_bytes = val.wstr.length<SQLLEN>() * sizeof(SQLWCHAR);
} else {
// 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
9 changes: 7 additions & 2 deletions src/types/decimal_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment thread
fdcastel marked this conversation as resolved.
reinterpret_cast<SQLPOINTER>(dc.data()), param.LengthBytes(), &param.LengthBytes());
if (!SQL_SUCCEEDED(ret)) {
std::string diag = Diagnostics::Read(ctx.hstmt(), SQL_HANDLE_STMT);
Expand Down
4 changes: 4 additions & 0 deletions src/types/float_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 9 additions & 0 deletions src/types/integer_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions src/types/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 32 additions & 0 deletions test/sql/db2/db2_copy.test
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
34 changes: 34 additions & 0 deletions test/sql/duckdb/duckdb_copy.test
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
38 changes: 38 additions & 0 deletions test/sql/firebird/firebird_copy.test
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
34 changes: 34 additions & 0 deletions test/sql/mssql/mssql_copy.test
Original file line number Diff line number Diff line change
Expand Up @@ -474,5 +474,39 @@ 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.

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)')
Comment thread
fdcastel marked this conversation as resolved.
Outdated

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'))
Loading
Loading