From 4629e109fe76d2ab8fce2880735dbd4f3d6779ef Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sun, 6 Sep 2026 14:07:43 -0300 Subject: [PATCH] Describe INPUT parameters from the prepared type, not the last bind A parameter's sqlvar is rewritten on every bind to match the C type being transferred (OdbcConvert::setHeadSqlVar), but the driver described the parameter from that same live sqlvar. Once a character transfer had turned it into SQL_TEXT nothing restored it, so every later describe reported CHAR, the next bind selected a string converter again, and the parameter stayed text for the life of the statement -- SQLDescribeParam reported CHAR as well. Since SQL_C_DEFAULT resolves to SQL_C_CHAR for SQL_NUMERIC, SQL_DECIMAL and SQL_BIGINT, an application that binds its NULLs with SQL_C_DEFAULT and its values with the value's own C type poisoned the parameter on its first NULL: every row after it reached the server as NULL, with SQL_SUCCESS and a correct row count. SQL_INTEGER, SQL_SMALLINT and SQL_DOUBLE escaped only because their SQL_C_DEFAULT never resolves to a character C type, and SQLFreeStmt(SQL_RESET_PARAMS) did not help because the state was in the sqlvar, not in the descriptors. Describe INPUT parameters from the prepare-time snapshot (orgSqlProperties) instead, as getColumnDisplaySize() already did, so a bind always sees the type the statement was prepared with whatever the previous transfer did to the sqlvar. OUTPUT columns keep reading the live sqlvar, which is never rewritten. --- IscDbc/Sqlda.cpp | 16 +-- IscDbc/Sqlda.h | 17 +++- tests/test_param_conversions.cpp | 170 +++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 10 deletions(-) diff --git a/IscDbc/Sqlda.cpp b/IscDbc/Sqlda.cpp index 4754c833..ec5470a1 100644 --- a/IscDbc/Sqlda.cpp +++ b/IscDbc/Sqlda.cpp @@ -684,7 +684,7 @@ void Sqlda::print() // int Sqlda::getColumnDisplaySize(int index) { - const SqlProperties *var = (SqldaDir == SQLDA_INPUT) ? orgVarSqlProperties(index) : Var(index); + const SqlProperties *var = describedSqlProperties(index); switch (var->sqltype) { @@ -843,7 +843,7 @@ int Sqlda::getNumPrecRadix(int index) int Sqlda::getScale(int index) { - CAttrSqlVar *var = Var(index); + const SqlProperties *var = describedSqlProperties(index); switch (var->sqltype) { @@ -862,20 +862,20 @@ bool Sqlda::isNullable(int index) int Sqlda::getColumnType(int index, int &realSqlType) { - return getSqlType ( Var(index), realSqlType ); + return getSqlType ( describedSqlProperties(index), Var(index)->array, realSqlType ); } const char* Sqlda::getColumnTypeName(int index) { - return getSqlTypeName ( Var(index) ); + return getSqlTypeName ( describedSqlProperties(index) ); } short Sqlda::getSubType(int index) { - return Var( index )->sqlsubtype; + return describedSqlProperties( index )->sqlsubtype; } -int Sqlda::getSqlType(CAttrSqlVar *var, int &realSqlType) +int Sqlda::getSqlType(const SqlProperties *var, const CAttrArray *array, int &realSqlType) { switch (var->sqltype) { @@ -935,7 +935,7 @@ int Sqlda::getSqlType(CAttrSqlVar *var, int &realSqlType) return (realSqlType = JDBC_DATE); case SQL_ARRAY: - if ( var->array->arrOctetLength < MAX_VARCHAR_LENGTH ) + if ( array->arrOctetLength < MAX_VARCHAR_LENGTH ) return (realSqlType = JDBC_VARCHAR); return (realSqlType = JDBC_LONGVARCHAR); } @@ -943,7 +943,7 @@ int Sqlda::getSqlType(CAttrSqlVar *var, int &realSqlType) return (realSqlType = 0); } -const char* Sqlda::getSqlTypeName ( CAttrSqlVar *var ) +const char* Sqlda::getSqlTypeName ( const SqlProperties *var ) { switch (var->sqltype) { diff --git a/IscDbc/Sqlda.h b/IscDbc/Sqlda.h index 9c7b6e9e..2cf6167c 100644 --- a/IscDbc/Sqlda.h +++ b/IscDbc/Sqlda.h @@ -178,8 +178,8 @@ class Sqlda void setArray(CAttrSqlVar* var, Value* value, IscStatement* stmt); void setValue(int slot, Value* value, IscStatement* stmt); const char* getTableName(int index); - int getSqlType(CAttrSqlVar* var, int& realSqlType); - const char* getSqlTypeName(CAttrSqlVar* var); + int getSqlType(const SqlProperties* var, const CAttrArray* array, int& realSqlType); + const char* getSqlTypeName(const SqlProperties* var); bool isNullable(int index); int getScale(int index); int getPrecision(int index); @@ -208,6 +208,19 @@ class Sqlda CAttrSqlVar* Var(int index) { return &sqlvar.at(index - 1); } const SqlProperties* orgVarSqlProperties(int index) { return &sqlvar.at(index - 1).orgSqlProperties; } + // Properties an index should be *described* with. + // + // An INPUT parameter is always described from the prepare-time snapshot: the + // conversion layer rewrites the live sqlvar to match whatever C type was last + // bound (OdbcConvert::setHeadSqlVar), so the live one tells us how the previous + // transfer was shaped, not what the statement was prepared with. Describing + // from it makes a bind on an already-used parameter see that shape and keep it + // forever. OUTPUT columns are never rewritten, so they use the live sqlvar. + const SqlProperties* describedSqlProperties(int index) + { + return (SqldaDir == SQLDA_INPUT) ? orgVarSqlProperties(index) : Var(index); + } + Sqlda(IscConnection* conn, e_sqlda_dir dir); ~Sqlda(); diff --git a/tests/test_param_conversions.cpp b/tests/test_param_conversions.cpp index 5a2661aa..f7d44402 100644 --- a/tests/test_param_conversions.cpp +++ b/tests/test_param_conversions.cpp @@ -87,6 +87,98 @@ class ParamConversionsTest : public OdbcConnectedTest { SQLCloseCursor(hStmt); return std::string((char*)buf); } + + // Read a single column back as a string ("NULL" for a null value). + std::string readBack(const char* colName, int id) { + char selectSql[256]; + snprintf(selectSql, sizeof(selectSql), + "SELECT %s FROM ODBC_TEST_PCONV WHERE ID = %d", colName, id); + SQLRETURN ret = SQLExecDirect(hStmt, (SQLCHAR*)selectSql, SQL_NTS); + if (!SQL_SUCCEEDED(ret)) return ""; + + SQLCHAR buf[256] = {}; + SQLLEN ind = 0; + SQLBindCol(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind); + ret = SQLFetch(hStmt); + if (!SQL_SUCCEEDED(ret)) return ""; + SQLCloseCursor(hStmt); + SQLFreeStmt(hStmt, SQL_UNBIND); + return ind == SQL_NULL_DATA ? "NULL" : std::string((char*)buf); + } + + // Send one NULL row and one value row through a single prepared statement, + // binding the NULL with SQL_C_DEFAULT (which resolves to SQL_C_CHAR for + // SQL_NUMERIC, SQL_DECIMAL and SQL_BIGINT) and the value with SQL_C_SLONG. + // Returns the value row read back. + std::string nullThenValueRebind(const char* colName, bool resetParams) { + const int nullId = nextId_++; + const int valueId = nextId_++; + + char sql[256]; + snprintf(sql, sizeof(sql), + "UPDATE OR INSERT INTO ODBC_TEST_PCONV (ID, %s) VALUES (?, ?)", colName); + SQLRETURN ret = SQLPrepare(hStmt, (SQLCHAR*)sql, SQL_NTS); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLPrepare failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + if (!SQL_SUCCEEDED(ret)) return ""; + + SQLSMALLINT paramType = 0, decimals = 0, nullable = 0; + SQLULEN paramSize = 0; + ret = SQLDescribeParam(hStmt, 2, ¶mType, ¶mSize, &decimals, &nullable); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLDescribeParam failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + if (!SQL_SUCCEEDED(ret)) return ""; + + SQLINTEGER idVal = nullId; + SQLLEN idInd = sizeof(idVal); + SQLINTEGER value = 0; + SQLLEN valueInd = SQL_NULL_DATA; + + auto bindId = [&]() { + return SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + }; + + ret = bindId(); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) << "bind of ID failed"; + + // The NULL row: no C type of its own, no data pointer - only the indicator. + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_DEFAULT, paramType, paramSize, decimals, NULL, 0, &valueInd); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "bind of NULL failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLExecute(hStmt); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "execute of the NULL row failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + // The value row, rebound with a numeric C type. + idVal = valueId; + value = 60; + valueInd = sizeof(value); + + SQLFreeStmt(hStmt, SQL_CLOSE); + if (resetParams) { + SQLFreeStmt(hStmt, SQL_RESET_PARAMS); + ret = bindId(); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) << "rebind of ID failed"; + } + + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_SLONG, paramType, paramSize, decimals, &value, sizeof(value), &valueInd); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "rebind of the value failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLExecute(hStmt); + EXPECT_TRUE(SQL_SUCCEEDED(ret)) + << "execute of the value row failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + Commit(); + ReallocStmt(); + + EXPECT_EQ(readBack(colName, nullId), "NULL") << "the NULL row did not stay NULL"; + return readBack(colName, valueId); + } }; // ===== String → Integer ===== @@ -277,6 +369,84 @@ TEST_F(ParamConversionsTest, NumericAsCharParam) { EXPECT_NEAR(atof(result.c_str()), 1234.5678, 0.001); } +// ===== A character-typed bind must not retype the parameter ===== +// +// 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 sends a character transfer first. That transfer used to retype +// the parameter for the rest of the statement's life: every later bind was described +// as CHAR, and rows following the NULL reached the server as NULL with SQL_SUCCESS. +// SQL_INTEGER / SQL_SMALLINT / SQL_DOUBLE were unaffected only because their +// SQL_C_DEFAULT never resolves to a character C type. + +TEST_F(ParamConversionsTest, NullAsDefaultThenBigintRebind) { + SKIP_ON_FIREBIRD6(); + EXPECT_EQ(atoi(nullThenValueRebind("VAL_BIGINT", false).c_str()), 60); +} + +TEST_F(ParamConversionsTest, NullAsDefaultThenNumericRebind) { + SKIP_ON_FIREBIRD6(); + EXPECT_NEAR(atof(nullThenValueRebind("VAL_NUMERIC", false).c_str()), 60.0, 0.001); +} + +TEST_F(ParamConversionsTest, NullAsDefaultThenIntegerRebind) { + SKIP_ON_FIREBIRD6(); + EXPECT_EQ(atoi(nullThenValueRebind("VAL_INT", false).c_str()), 60); +} + +// SQLFreeStmt(SQL_RESET_PARAMS) does not help: the state that was lost lived in the +// parameter itself, not in the descriptors the reset clears. +TEST_F(ParamConversionsTest, NullAsDefaultThenBigintRebindAfterResetParams) { + SKIP_ON_FIREBIRD6(); + EXPECT_EQ(atoi(nullThenValueRebind("VAL_BIGINT", true).c_str()), 60); +} + +// The same retyping was visible directly: SQLDescribeParam reported CHAR for a +// BIGINT parameter once a character transfer had gone through it. +TEST_F(ParamConversionsTest, DescribeParamStableAfterCharBind) { + SKIP_ON_FIREBIRD6(); + + SQLRETURN ret = SQLPrepare(hStmt, + (SQLCHAR*)"UPDATE OR INSERT INTO ODBC_TEST_PCONV (ID, VAL_BIGINT) VALUES (?, ?)", + SQL_NTS); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLPrepare failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + SQLSMALLINT typeBefore = 0, decimals = 0, nullable = 0; + SQLULEN paramSize = 0; + ret = SQLDescribeParam(hStmt, 2, &typeBefore, ¶mSize, &decimals, &nullable); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLDescribeParam failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + EXPECT_EQ(typeBefore, SQL_BIGINT); + + SQLINTEGER idVal = nextId_++; + SQLLEN idInd = sizeof(idVal); + SQLLEN valueInd = SQL_NULL_DATA; + + ret = SQLBindParameter(hStmt, 1, SQL_PARAM_INPUT, + SQL_C_SLONG, SQL_INTEGER, 0, 0, &idVal, sizeof(idVal), &idInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) << "bind of ID failed"; + + ret = SQLBindParameter(hStmt, 2, SQL_PARAM_INPUT, + SQL_C_CHAR, SQL_BIGINT, paramSize, decimals, NULL, 0, &valueInd); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "bind of NULL failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + ret = SQLExecute(hStmt); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "SQLExecute failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + + SQLSMALLINT typeAfter = 0; + ret = SQLDescribeParam(hStmt, 2, &typeAfter, ¶mSize, &decimals, &nullable); + ASSERT_TRUE(SQL_SUCCEEDED(ret)) + << "second SQLDescribeParam failed: " << GetOdbcError(SQL_HANDLE_STMT, hStmt); + EXPECT_EQ(typeAfter, typeBefore) + << "the parameter was redescribed after a character-typed bind"; + + Commit(); + ReallocStmt(); +} + // ===== Already-covered round-trip tests from test_data_types.cpp ===== // (IntegerParamInsertAndSelect, VarcharParamInsertAndSelect, // DoubleParamInsertAndSelect, DateParamInsertAndSelect,