Skip to content

Commit fb356d6

Browse files
fdcastelfdcastel
committed
Fix FB master regression: write VARYING length prefix instead of mutating sqltype
Follow-up to the previous Issue #161 commit. On Firebird master (6.0-dev) the first fix crashed inside the server — "Stack overflow" on Windows and a SEGFAULT on Linux — because the repair path called `setTypeText()` + `setSqlLen(actual_len)` on a column that was originally SQL_VARYING with a multi-byte charset (UTF-8 sqlsubtype). Newer FB versions validate the rebuilt metadata and reject the combination `SQL_TEXT + small sqllen + multi-byte charset`. New approach — do not mutate the sqlvar at all. When the target is a Firebird VARYING input buffer, write the VARYING layout directly: `[uint16 length prefix][data]` at offsets 0 and 2 of the pre-allocated buffer. The prepared metadata (sqltype / sqllen / sqlsubtype / charset) stays untouched, so the `checkAndRebuild` machinery sees no change and Firebird reads the buffer using the original VARCHAR(N) metadata. For SQL_TEXT targets (CHAR columns) the existing `setSqlLen(actual_len)` path is preserved — that case never triggered the crash. Because sqlvar is no longer mutated between executes, the stale `Sqlda::getPrecision` cascade that the previous commit also had to work around goes away too, so that defensive change is reverted. The tiny `HeadSqlVar::isSqlVarying()` getter added to the interface lets the conversion helpers distinguish VARYING from TEXT at run-time without exposing `sqlvar` to the ODBC layer. Verified locally: - Firebird 5.0.3 UTF-8: 200/200 tests pass. - Firebird 6.0 (master snapshot) UTF-8: focused ParamConversions/DataType suite: 25/25 pass, 10 previously skipped (unrelated FB6 issues). - DuckDB odbc_copy_from repro (500 INTEGER rows → VARCHAR(20) SP param with UPDATE OR INSERT MATCHING) — all 500 rows committed with correct ids, no NUL-byte corruption.
1 parent 8bce4b2 commit fb356d6

3 files changed

Lines changed: 99 additions & 68 deletions

File tree

‎IscDbc/Connection.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ class HeadSqlVar
524524
//virtual void setSqlSubType ( short subtype ) = 0;
525525
virtual void setSqlLen ( short len ) = 0;
526526
virtual short getSqlMultiple () = 0;
527+
virtual bool isSqlVarying () = 0;
527528

528529
virtual char * getSqlData() = 0;
529530
virtual short * getSqlInd() = 0;

‎IscDbc/IscHeadSqlVar.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class IscHeadSqlVar : public HeadSqlVar
102102
inline short getSqlMultiple () { return sqlMultiple; }
103103
inline char * getSqlData() { return sqlvar->sqldata; }
104104
inline short * getSqlInd() { return sqlvar->sqlind; }
105+
inline bool isSqlVarying() { return sqlvar->sqltype == SQL_VARYING; }
105106

106107
// not used
107108
//void setSqlInd( short *ind ) { sqlvar->sqlind = ind; }

‎OdbcConvert.cpp‎

Lines changed: 97 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,24 +1335,26 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to)
13351335
\
13361336
ODBCCONVERT_CHECKNULL( pointer ); \
13371337
\
1338-
/* Issue #161: when writing into a Firebird input buffer, force sqltype to */ \
1339-
/* SQL_TEXT. This routine writes raw ASCII digits at offset 0 of the target */ \
1340-
/* buffer — that is the correct layout for SQL_TEXT, but for SQL_VARYING the */ \
1341-
/* first 2 bytes are reserved for a length prefix. Leaving the buffer as */ \
1342-
/* SQL_VARYING causes Firebird to read the digits as the length prefix, */ \
1343-
/* silently corrupting the parameter (e.g. INTEGER→VARCHAR stored-procedure */ \
1344-
/* parameters would swallow the data and commit only a fraction of rows). */ \
1345-
if ( to->isIndicatorSqlDa ) \
1346-
to->headSqlVarPtr->setTypeText(); \
1338+
/* Issue #161: the buffer layout differs between SQL_TEXT and SQL_VARYING */ \
1339+
/* on the Firebird side — VARYING reserves the first two bytes for a length */ \
1340+
/* prefix. Detect the target at run-time and write accordingly. We do NOT */ \
1341+
/* mutate sqltype/sqllen so the prepared metadata stays intact (recent FB */ \
1342+
/* versions reject metadata where sqllen is not a multiple of the charset */ \
1343+
/* element size — e.g. sqllen=1 on a UTF-8 VARCHAR column — leading to */ \
1344+
/* stack-overflow crashes at execute time). */ \
1345+
const bool isVarying = \
1346+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying(); \
1347+
char *writeStart = (char*)pointer + (isVarying ? sizeof(short) : 0); \
1348+
int maxLen = to->length; \
13471349
\
1348-
int len = to->length; \
1350+
int len = maxLen; \
13491351
\
1350-
if ( !len && to->dataPtr) \
1352+
if ( !maxLen && to->dataPtr) \
13511353
*(char*)to->dataPtr = 0; \
13521354
else \
13531355
{ /* Original source from IscDbc/Value.cpp */ \
13541356
C_TYPE_FROM number = *(C_TYPE_FROM*)getAdressBindDataFrom((char*)from->dataPtr); \
1355-
char *string = (char*)pointer; \
1357+
char *string = writeStart; \
13561358
int scale = -from->scale; \
13571359
\
13581360
if (number == 0) \
@@ -1397,8 +1399,8 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to)
13971399
if (negative) \
13981400
*q++ = '-',++l; \
13991401
\
1400-
if ( p - temp > len - l ) \
1401-
p = temp + len - l; \
1402+
if ( p - temp > maxLen - l ) \
1403+
p = temp + maxLen - l; \
14021404
\
14031405
while (p > temp) \
14041406
*q++ = *--p; \
@@ -1408,10 +1410,16 @@ int OdbcConvert::conv##TYPE_FROM##ToString(DescRecord * from, DescRecord * to)
14081410
} \
14091411
} \
14101412
\
1411-
if ( to->isIndicatorSqlDa ) { \
1413+
if ( isVarying ) \
1414+
{ \
1415+
/* Write the VARYING length prefix; leave the sqlvar's metadata alone. */ \
1416+
*(unsigned short*)pointer = (unsigned short)len; \
1417+
} \
1418+
else if ( to->isIndicatorSqlDa ) \
1419+
{ \
14121420
to->headSqlVarPtr->setSqlLen(len); \
1413-
} else \
1414-
if ( indicatorTo ) \
1421+
} \
1422+
else if ( indicatorTo ) \
14151423
setIndicatorPtr( indicatorTo, len, to ); \
14161424
\
14171425
return SQL_SUCCESS; \
@@ -1426,11 +1434,9 @@ int OdbcConvert::conv##TYPE_FROM##ToStringW(DescRecord * from, DescRecord * to)
14261434
\
14271435
ODBCCONVERT_CHECKNULLW( pointer ); \
14281436
\
1429-
/* Issue #161: see ODBCCONVERT_CONV_TO_STRING — same reasoning for the wide */ \
1430-
/* variant. Data is written at offset 0 without a length prefix, so the */ \
1431-
/* target must be SQL_TEXT when it is a Firebird input buffer. */ \
1432-
if ( to->isIndicatorSqlDa ) \
1433-
to->headSqlVarPtr->setTypeText(); \
1437+
/* Issue #161: getAdressFunction routes Firebird-side numeric-to-string */ \
1438+
/* binds to the byte variant, so this wide variant is only reached with */ \
1439+
/* application-owned targets. No VARYING prefix handling is needed here. */ \
14341440
\
14351441
int len = to->length; \
14361442
\
@@ -1814,16 +1820,23 @@ int OdbcConvert::convFloatToString(DescRecord * from, DescRecord * to)
18141820

18151821
ODBCCONVERT_CHECKNULL( pointerTo );
18161822

1817-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
1818-
if ( to->isIndicatorSqlDa )
1819-
to->headSqlVarPtr->setTypeText();
1823+
// Issue #161: write the VARYING length prefix when the target is a
1824+
// Firebird VARYING buffer; see ODBCCONVERT_CONV_TO_STRING for rationale.
1825+
const bool isVarying =
1826+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying();
1827+
char *writeStart = pointerTo + (isVarying ? sizeof(short) : 0);
1828+
int maxLen = to->length;
18201829

1821-
int len = to->length;
1830+
int len = maxLen;
18221831

1823-
if ( len )
1824-
ConvertFloatToString<char>(*(float*)getAdressBindDataFrom((char*)from->dataPtr), pointerTo, len, &len);
1832+
if ( maxLen )
1833+
ConvertFloatToString<char>(*(float*)getAdressBindDataFrom((char*)from->dataPtr), writeStart, maxLen, &len);
18251834

1826-
if ( to->isIndicatorSqlDa ) {
1835+
if ( isVarying )
1836+
{
1837+
*(unsigned short*)pointerTo = (unsigned short)len;
1838+
}
1839+
else if ( to->isIndicatorSqlDa ) {
18271840
to->headSqlVarPtr->setSqlLen(len);
18281841
} else
18291842
if ( indicatorTo )
@@ -1840,9 +1853,8 @@ int OdbcConvert::convFloatToStringW(DescRecord * from, DescRecord * to)
18401853

18411854
ODBCCONVERT_CHECKNULLW( pointerTo );
18421855

1843-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
1844-
if ( to->isIndicatorSqlDa )
1845-
to->headSqlVarPtr->setTypeText();
1856+
// Issue #161: wide variant is only reached with application-owned targets
1857+
// — see ODBCCONVERT_CONV_TO_STRINGW.
18461858

18471859
int len = to->length;
18481860

@@ -1929,16 +1941,22 @@ int OdbcConvert::convDoubleToString(DescRecord * from, DescRecord * to)
19291941

19301942
ODBCCONVERT_CHECKNULL( pointerTo );
19311943

1932-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
1933-
if ( to->isIndicatorSqlDa )
1934-
to->headSqlVarPtr->setTypeText();
1944+
// Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale.
1945+
const bool isVarying =
1946+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying();
1947+
char *writeStart = pointerTo + (isVarying ? sizeof(short) : 0);
1948+
int maxLen = to->length;
19351949

1936-
int len = to->length;
1950+
int len = maxLen;
19371951

1938-
if ( len ) // MAX_DOUBLE_DIGIT_LENGTH = 15
1939-
ConvertFloatToString<char>(*(double*)getAdressBindDataFrom((char*)from->dataPtr), pointerTo, len, &len);
1952+
if ( maxLen ) // MAX_DOUBLE_DIGIT_LENGTH = 15
1953+
ConvertFloatToString<char>(*(double*)getAdressBindDataFrom((char*)from->dataPtr), writeStart, maxLen, &len);
19401954

1941-
if ( to->isIndicatorSqlDa ) {
1955+
if ( isVarying )
1956+
{
1957+
*(unsigned short*)pointerTo = (unsigned short)len;
1958+
}
1959+
else if ( to->isIndicatorSqlDa ) {
19421960
to->headSqlVarPtr->setSqlLen(len);
19431961
} else
19441962
if ( indicatorTo )
@@ -1955,9 +1973,8 @@ int OdbcConvert::convDoubleToStringW(DescRecord * from, DescRecord * to)
19551973

19561974
ODBCCONVERT_CHECKNULLW( pointerTo );
19571975

1958-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
1959-
if ( to->isIndicatorSqlDa )
1960-
to->headSqlVarPtr->setTypeText();
1976+
// Issue #161: wide variant is only reached with application-owned targets
1977+
// — see ODBCCONVERT_CONV_TO_STRINGW.
19611978

19621979
int len = to->length;
19631980

@@ -2051,21 +2068,26 @@ int OdbcConvert::convDateToString(DescRecord * from, DescRecord * to)
20512068

20522069
ODBCCONVERT_CHECKNULL( pointer );
20532070

2054-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2055-
if ( to->isIndicatorSqlDa )
2056-
to->headSqlVarPtr->setTypeText();
2071+
// Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale.
2072+
const bool isVarying =
2073+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying();
2074+
char *writeStart = pointer + (isVarying ? sizeof(short) : 0);
20572075

20582076
SQLUSMALLINT mday, month;
20592077
SQLSMALLINT year;
20602078

20612079
decode_sql_date(*(int*)getAdressBindDataFrom((char*)from->dataPtr), mday, month, year);
20622080
int len, outlen = to->length;
20632081

2064-
len = snprintf(pointer, outlen, "%04d-%02d-%02d",year,month,mday);
2082+
len = snprintf(writeStart, outlen, "%04d-%02d-%02d",year,month,mday);
20652083

20662084
if ( len == -1 ) len = outlen;
20672085

2068-
if ( to->isIndicatorSqlDa ) {
2086+
if ( isVarying )
2087+
{
2088+
*(unsigned short*)pointer = (unsigned short)len;
2089+
}
2090+
else if ( to->isIndicatorSqlDa ) {
20692091
to->headSqlVarPtr->setSqlLen(len);
20702092
} else
20712093
if ( indicatorTo )
@@ -2082,9 +2104,8 @@ int OdbcConvert::convDateToStringW(DescRecord * from, DescRecord * to)
20822104

20832105
ODBCCONVERT_CHECKNULLW( pointer );
20842106

2085-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2086-
if ( to->isIndicatorSqlDa )
2087-
to->headSqlVarPtr->setTypeText();
2107+
// Issue #161: wide variant is only reached with application-owned targets
2108+
// — see ODBCCONVERT_CONV_TO_STRINGW.
20882109

20892110
SQLUSMALLINT mday, month;
20902111
SQLSMALLINT year;
@@ -2236,9 +2257,10 @@ int OdbcConvert::convTimeToString(DescRecord * from, DescRecord * to)
22362257

22372258
ODBCCONVERT_CHECKNULL( pointer );
22382259

2239-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2240-
if ( to->isIndicatorSqlDa )
2241-
to->headSqlVarPtr->setTypeText();
2260+
// Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale.
2261+
const bool isVarying =
2262+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying();
2263+
char *writeStart = pointer + (isVarying ? sizeof(short) : 0);
22422264

22432265
SQLUSMALLINT hour, minute, second;
22442266
int ntime = *(int*)getAdressBindDataFrom((char*)from->dataPtr);
@@ -2249,13 +2271,17 @@ int OdbcConvert::convTimeToString(DescRecord * from, DescRecord * to)
22492271
int len, outlen = to->length;
22502272

22512273
if ( nnano )
2252-
len = snprintf(pointer, outlen, "%02d:%02d:%02d.%04lu",hour, minute, second, nnano);
2274+
len = snprintf(writeStart, outlen, "%02d:%02d:%02d.%04lu",hour, minute, second, nnano);
22532275
else
2254-
len = snprintf(pointer, outlen, "%02d:%02d:%02d",hour, minute, second);
2276+
len = snprintf(writeStart, outlen, "%02d:%02d:%02d",hour, minute, second);
22552277

22562278
if ( len == -1 ) len = outlen;
22572279

2258-
if ( to->isIndicatorSqlDa ) {
2280+
if ( isVarying )
2281+
{
2282+
*(unsigned short*)pointer = (unsigned short)len;
2283+
}
2284+
else if ( to->isIndicatorSqlDa ) {
22592285
to->headSqlVarPtr->setSqlLen(len);
22602286
} else
22612287
if ( indicatorTo )
@@ -2272,9 +2298,8 @@ int OdbcConvert::convTimeToStringW(DescRecord * from, DescRecord * to)
22722298

22732299
ODBCCONVERT_CHECKNULLW( pointer );
22742300

2275-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2276-
if ( to->isIndicatorSqlDa )
2277-
to->headSqlVarPtr->setTypeText();
2301+
// Issue #161: wide variant is only reached with application-owned targets
2302+
// — see ODBCCONVERT_CONV_TO_STRINGW.
22782303

22792304
SQLUSMALLINT hour, minute, second;
22802305
int ntime = *(int*)getAdressBindDataFrom((char*)from->dataPtr);
@@ -2436,9 +2461,10 @@ int OdbcConvert::convDateTimeToString(DescRecord * from, DescRecord * to)
24362461

24372462
ODBCCONVERT_CHECKNULL( pointer );
24382463

2439-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2440-
if ( to->isIndicatorSqlDa )
2441-
to->headSqlVarPtr->setTypeText();
2464+
// Issue #161: see ODBCCONVERT_CONV_TO_STRING for rationale.
2465+
const bool isVarying =
2466+
to->isIndicatorSqlDa && to->headSqlVarPtr->isSqlVarying();
2467+
char *writeStart = pointer + (isVarying ? sizeof(short) : 0);
24422468

24432469
QUAD pointerFrom = *(QUAD*)getAdressBindDataFrom((char*)from->dataPtr);
24442470
int ndate = LO_LONG(pointerFrom);
@@ -2453,13 +2479,17 @@ int OdbcConvert::convDateTimeToString(DescRecord * from, DescRecord * to)
24532479
int len, outlen = to->length;
24542480

24552481
if ( nnano )
2456-
len = snprintf(pointer, outlen, "%04d-%02d-%02d %02d:%02d:%02d.%04lu",year,month,mday,hour, minute, second, nnano);
2482+
len = snprintf(writeStart, outlen, "%04d-%02d-%02d %02d:%02d:%02d.%04lu",year,month,mday,hour, minute, second, nnano);
24572483
else
2458-
len = snprintf(pointer, outlen, "%04d-%02d-%02d %02d:%02d:%02d",year,month,mday,hour, minute, second);
2484+
len = snprintf(writeStart, outlen, "%04d-%02d-%02d %02d:%02d:%02d",year,month,mday,hour, minute, second);
24592485

24602486
if ( len == -1 ) len = outlen;
24612487

2462-
if ( to->isIndicatorSqlDa ) {
2488+
if ( isVarying )
2489+
{
2490+
*(unsigned short*)pointer = (unsigned short)len;
2491+
}
2492+
else if ( to->isIndicatorSqlDa ) {
24632493
to->headSqlVarPtr->setSqlLen(len);
24642494
} else
24652495
if ( indicatorTo )
@@ -2476,9 +2506,8 @@ int OdbcConvert::convDateTimeToStringW(DescRecord * from, DescRecord * to)
24762506

24772507
ODBCCONVERT_CHECKNULLW( pointer );
24782508

2479-
// Issue #161: see ODBCCONVERT_CONV_TO_STRING — force SQL_TEXT layout.
2480-
if ( to->isIndicatorSqlDa )
2481-
to->headSqlVarPtr->setTypeText();
2509+
// Issue #161: wide variant is only reached with application-owned targets
2510+
// — see ODBCCONVERT_CONV_TO_STRINGW.
24822511

24832512
QUAD pointerFrom = *(QUAD*)getAdressBindDataFrom((char*)from->dataPtr);
24842513
int ndate = LO_LONG(pointerFrom);

0 commit comments

Comments
 (0)