Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 4 additions & 0 deletions src/binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ char *DecimalChars::data() {
return characters.data();
}

SQLWCHAR *DecimalChars::wide_data() {
return wide_characters.data();
}

ScannerBlob::ScannerBlob() {
}

Expand Down
8 changes: 8 additions & 0 deletions src/include/binary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@
#include <vector>

#include "duckdb_extension_api.hpp"
#include "odbc_api.hpp"

namespace odbcscanner {

struct DecimalChars {
std::vector<char> characters;
// Optional wide buffer populated by BindOdbcParam<DecimalChars> 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<SQLWCHAR> wide_characters;

DecimalChars();

Expand All @@ -26,6 +32,8 @@ struct DecimalChars {
}

char *data();

SQLWCHAR *wide_data();
};

struct ScannerBlob {
Expand Down
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 and re-tags it as
// TYPE_DECIMAL_AS_CHARS so the scanner binds via BindOdbcParam<DecimalChars>
// 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<DecimalChars> itself, which
// widens the buffer on demand.
void TransformNumericToChars();

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
30 changes: 30 additions & 0 deletions src/params.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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. Wide targets (SQL_WCHAR/WVARCHAR/WLONGVARCHAR)
// are handled by BindOdbcParam<DecimalChars>, which widens the buffer on demand.
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()) {
Expand All @@ -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);
}
}

Expand Down
56 changes: 56 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,59 @@ void ScannerValue::TransformIntegralToDecimal() {
*this = ScannerValue(dec, false);
}

void ScannerValue::TransformNumericToChars() {
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));
}

// Destroy the current (POD) value and repurpose the union as DecimalChars.
// Wide character targets are handled downstream by BindOdbcParam<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
32 changes: 30 additions & 2 deletions src/types/decimal_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "connection.hpp"
#include "diagnostics.hpp"
#include "scanner_exception.hpp"
#include "widechar.hpp"

DUCKDB_EXTENSION_EXTERN

Expand Down Expand Up @@ -82,10 +83,37 @@ 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 → 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<DecimalChars>();

// 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(), static_cast<size_t>(dc.size<SQLLEN>()));
Comment thread
fdcastel marked this conversation as resolved.
Outdated
dc.wide_characters = std::move(wstr.vec);
}
SQLLEN length_chars = static_cast<SQLLEN>(dc.wide_characters.size() - 1);
param.LengthBytes() = length_chars * static_cast<SQLLEN>(sizeof(SQLWCHAR));
Comment thread
fdcastel marked this conversation as resolved.
Outdated
SQLRETURN ret = SQLBindParameter(
ctx.hstmt(), param_idx, SQL_PARAM_INPUT, SQL_C_WCHAR, sqltype, static_cast<SQLULEN>(length_chars), 0,
reinterpret_cast<SQLPOINTER>(dc.wide_data()), param.LengthBytes(), &param.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,
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'))
Loading
Loading