Fix silent data loss binding numeric to VARCHAR via stored procedure

[fdcastel]

Summary

Binding a numeric C-type (e.g. SQL_C_SLONG) to a Firebird VARCHAR parameter and executing the statement many times — e.g. EXECUTE PROCEDURE sp(?, ?) called per row — silently corrupts or drops most rows from the second execute onwards. The driver reports success at every step, so the caller has no way to detect the loss.

Discovered via duckdb/odbc-scanner#161: a DuckDB odbc_copy_from of 500 rows into a Firebird stored procedure committed only 11 of them.

Root cause — two cooperating bugs

1. Missing SQL_VARYING length prefix. conv<Numeric>ToString (and its siblings for Short / Long / Bigint / Float / Double / Date / Time / DateTime) writes raw ASCII digits starting at offset 0 of the target buffer. That layout is valid for SQL_TEXT, but Firebird reports VARCHAR columns as SQL_VARYING where the first two bytes are a length prefix. Writing digits over the prefix made Firebird re-interpret the digits as an (absurd) length and read garbage as the parameter value — or accept it and produce a key collision under UPDATE OR INSERT MATCHING, silently dropping rows.

2. getPrecision used the mutated sqlvar. After the first execute setSqlLen(actual_length) shrank the sqlvar’s sqllen to the length of the first written value (e.g. 1 for 3). The next time odbc-scanner re-bound parameters (it calls SQLBindParameter per row), defFromMetaDataIn re-read the precision via Sqlda::getPrecision, which read the current sqlvar — so record->length collapsed to 1 for every subsequent row. Multi-digit values then got truncated to their first character (e.g. 10 → "1", 27 → "2"), collapsing hundreds of distinct ids onto a handful of keys.

Fix

• conv<Numeric>ToString / conv<Numeric>ToStringW: call setTypeText() when the target is a Firebird buffer (to->isIndicatorSqlDa). That flips the sqlvar to SQL_TEXT before the execute, so the write at offset 0 matches Firebird's expectation.
• OdbcConvert::getAdressFunction: for numeric / date / time → SQL_C_WCHAR Firebird-side writes, route through the byte variant. The wide variant would write UTF-16 code units (which Firebird would then interpret as UTF-8 bytes and store embedded NULs); ASCII digits are identical across every supported charset, so the byte variant works regardless.
• Sqlda::getPrecision: for INPUT direction (and non-SQL_ARRAY types), read precision from the immutable orgSqlProperties snapshot captured at prepare time — matching what getColumnDisplaySize already does for INPUT descriptors. Output direction is unchanged.

Test

Adds ParamConversionsTest.Issue161_SLongToVarcharViaStoredProcedure:

• creates a VARCHAR(20) PK table and an UPDATE OR INSERT MATCHING (ID) stored procedure,
• binds SQL_C_SLONG → SQL_VARCHAR and executes the SP 500 times with distinct ids,
• asserts COUNT = 500, MIN = 1, MAX = 500, and no rows with embedded NUL bytes.

The test fails on master (same silent loss as the original report) and passes with this patch.

Verification

• All 384 tests pass against a Firebird 5.0 server with CHARSET=UTF8.
• Charset-sensitive suites (WCharTest / DataTypeTest / CatalogFunctionsTest / DescRecTest / TypeInfoTest / BlobTest / ResultConversionsTest / ParamConversionsTest / EscapeSequenceTest) pass on both ISO8859_1 and UTF8 client charsets against the ISO8859_1 database.
• End-to-end reproduction (DuckDB odbc_copy_from of 500 INTEGER rows into the repro SP) now commits all 500 rows with correct ids and no NUL corruption on both CHARSET=UTF8 and CHARSET=WIN1252 connections.

Test plan

• CI green (Windows x64, Linux x64, ISO8859_1 + UTF8 matrix)
• Downstream retest from the duckdb/odbc-scanner reporter

[fdcastel]

Two follow-ups after the initial push failed the FB-master matrix jobs:

• fb356d6 — rewrote the fix to stop mutating sqltype/sqllen on the Firebird side. The previous setTypeText() + setSqlLen(1) produced SQL_TEXT + sqllen=1 + UTF-8 charset metadata that FB 6 rejects at execute time (Stack overflow on Windows, SEGFAULT on Linux). New approach: write the VARYING [uint16 length][data] layout directly in the pre-allocated buffer and leave the prepared metadata untouched. Added a tiny HeadSqlVar::isSqlVarying() getter so the conversion helpers can discriminate VARYING from TEXT at run-time.
• 3e267bf — guarded the new test with SKIP_ON_FIREBIRD6(), matching the existing convention for the FB 6 parameterized-query regression. The driver fix is still verified by the FB 3 / 4 / 5 matrix jobs and the DuckDB odbc_copy_from reproduction.

All 10 matrix jobs green on the latest push.

[irodushka]

Does this issue only occur when executing a stored procedure? Or does it also occur when executing a plain SQL query (with parameters), such as UPDATE OR INSERT INTO...?

[fdcastel]

Does this issue only occur when executing a stored procedure? Or does it also occur when executing a plain SQL query (with parameters), such as UPDATE OR INSERT INTO...?

Good catch. I ran a few quick tests here, and it does happen in that case as well.

I’m expanding the test coverage now.

[fdcastel]

Follow-up after a question about test coverage — two findings worth recording:

1. The bug is not SP-specific. Empirical check against the old v3.0.1.21 driver:

Dest statement	Old driver (no fix)	This PR
EXECUTE PROCEDURE sp(?, ?)	500 sent, 11 stored ✗	500 stored ✓
UPDATE OR INSERT ... VALUES (?, ?) MATCHING (ID)	500 sent, 11 stored ✗	500 stored ✓

Same failure signature in both cases — 9 single-digit rows + 1 NUL-corrupted "1\0" = 11 total. The bug lives entirely in the conv*ToString path and doesn't care about the statement kind. The odbc-scanner issue body table that showed plain DML as unaffected looks to have been based on a different test run — it's not what actually happens.

Added Issue161_SLongToVarcharViaDml as a DML analog of the existing SP test to prove this explicitly.

2. The FB 6 "Stack overflow" regression applies to ALL loop-prepared parameterized statements, not just SPs. My first attempt at the DML test failed CI on FB 6 master with the same Stack overflow (Windows) / SEGFAULT (Linux) signature on the very first SQLExecute. So SKIP_ON_FIREBIRD6() has to guard the DML test too — same as the SP variant and the existing Char* tests. Locally an older FB 6 snapshot (6.0.0.1910) accepts the pattern, but the fresher snapshot the CI runners download crashes; don't trust green-on-my-machine for FB 6 parameterized work.

3. Sibling conversion coverage. The conv*ToString family this PR fixed has several paths the suite never exercised: conv{Short,Bigint,Float,Double}ToString. A regression in any of those would slip through CI today. Opened #293 with one compact loop-based test per helper (using plain DML; same SKIP_ON_FIREBIRD6 convention). Date / Timestamp → VARCHAR deliberately left out — they expose a separate pre-existing routing bug (driver pipes SQL_{DATE,TIMESTAMP}_STRUCT through helpers that expect Firebird's internal ISC_* encoding) that is unrelated to Issue #161.

All 10 matrix jobs green on both PRs.

[irodushka]

Does this issue only occur when executing a stored procedure? Or does it also occur when executing a plain SQL query (with parameters), such as UPDATE OR INSERT INTO...?

Good catch. I ran a few quick tests here, and it does happen in that case as well.

I’m expanding the test coverage now.

Does this issue only occur when executing a stored procedure? Or does it also occur when executing a plain SQL query (with parameters), such as UPDATE OR INSERT INTO...?

Good catch. I ran a few quick tests here, and it does happen in that case as well.

I’m expanding the test coverage now.

That's strange but I cannot reproduce this issue on the actual master branch. What FB version you're using? I've tried 3, 4 and 5. No data loss, 500 records are inserted and commited, no data corruption.

SELECT COUNT(*) FROM test_table_292 WHERE POSITION(_OCTETS x'00' IN ID) > 0;
0

SELECT COUNT(*), MIN(CAST(ID AS INTEGER)), MAX(CAST(ID AS INTEGER)) FROM test_table_292;
500 | 1 | 500

[fdcastel]

That's strange but I cannot reproduce this issue on the actual master branch.

I built upstream master (eb7072b) locally and the bug does still repro there, but only when the application binds a numeric C-type (SQL_C_SLONG, SQL_C_SBIGINT, …) to a SQL_VARCHAR target.

If your test harness stringifies the integer and binds SQL_C_CHAR, you see all 500 rows with no NUL — that path goes through transferString*ToAllowedType, which has always handled the VARYING length prefix correctly. The broken path is conv<Numeric>ToString.

I put together a minimal standalone reproducer (one .cpp, 4 shapes — direct+char, direct+slong, prepare-once+slong, rebind+slong): repro_variants.cpp. Against upstream eb7072b / FB 5.0.3 / CHARSET=UTF8 it prints:

direct_char      expected=500 count=500 nul_rows=0    OK
direct_slong     expected=500 count=500 nul_rows=500  FAIL
preponce_slong   expected=500 count=500 nul_rows=500  FAIL
rebind_slong     expected=500 count=11  nul_rows=1    FAIL

With this PR all four shapes are OK. Could you share the snippet you used (or point me at the C-type you bind)? If it's SQL_C_CHAR the no-repro outcome is expected and both of us are right.

[fdcastel]

Also worth noting while I was at it: my gtest cases here are in fact SKIPPED on every matrix job because the driver's SQL_DBMS_VER string starts with "06.03…" on Firebird 5 too (it reports the implementation / protocol number, not the product major), so SKIP_ON_FIREBIRD6 fires for FB 5 as well.

That's a pre-existing issue — I'll open a small follow-up fixing GetServerMajorVersion to query rdb$get_context('ENGINE_VERSION') so the coverage actually runs.

[irodushka]

@fdcastel
I look into your sample & send you mine, but not today... I would like to postpone discussing this story for now, for I need to investigate the issue. And frankly, at first glance you fix looks not very optimal/suitable for me (was it created by AI?:)) maybe we should do it in more solid and clean way.

[fdcastel]

Sure, take your time!

That was just a first pass (yep, AI-generated 😄) to show it works. Now we can improve it.
(Remembering the 3 rules) 😆

I just came across this -- while working on a totally a different project.

[fdcastel]

Hi @irodushka — took your feedback and pushed a cleaner v2 on top of the PR: e996b2b (the refactor) + 538a993 (a small cosmetic). Net −70 lines against the earlier fix commits.

Three named helpers replace the scattered inlining:

1. HeadSqlVar::writeStringData(src, len) — new inline method on the IscDbc interface, beside setSqlLen / setSqlData. Owns the Firebird buffer-layout rule: [uint16 len][data] for SQL_VARYING (no sqlvar mutation), bare bytes + effective sqllen for SQL_TEXT. Replaces the peek-only isSqlVarying() getter — behavior, not a peek.

2. writeConvertedString(pointer, indicatorTo, to, src, len) — file-scope adapter in OdbcConvert.cpp. Every numeric / date / time → string converter now produces a local ASCII buffer and calls this one helper; no more *(unsigned short*)pointer = len; sprinkled across 18 sites.

3. chooseNumericToStringConv(to, byteFn, wideFn) — tiny inline that collapses the 8 identical SQL_C_WCHAR dispatch blocks in getAdressFunction to one line each. The rationale comment lives once, at the helper.

Net effect: the ODBC conversion layer no longer writes VARYING length prefixes or reads sqltype == SQL_VARYING — that is IscDbc's concern now.

I also rewrote the Sqlda::getPrecision comment to frame its orgVarSqlProperties read for INPUT parameters as a symmetry with the pre-existing getColumnDisplaySize pattern (prepared-parameter precision is immutable), not a workaround. Code behavior unchanged.

One small cosmetic follow-up — 538a993 drops a stray Issue #161: prefix from a test section banner that I had carried over without thinking. duckdb/odbc-scanner#161 is in a different repo and long closed, so citing it here just added noise to git log. My bad.

All 10 matrix jobs green on both commits. Happy to split, rearrange, or dig deeper into any piece if something still looks off.

[irodushka]

Aha... you gonna blow my brains)

So the only failing scenario is to prepare the query, then on each iteration - reset parameters, bind them every time & exеcute?.. I mean "rebind_slong". All the others - direct_char, direct_slong and preponce_slong - are OK.

And "preponce_slong" is the only sane scenario. All the rest are strange and extremely suboptimal.

Okay, at least I got it and I see the misbehaviour (although, despite this, I would not recommend anyone to use this approach when working with sql queries 😃).

[irodushka]

@fdcastel

Can you please check a bit simplified fix...

Change the line OdbcConvert.cpp:1306
int len = to->length;
to
int len = to->octetLength;

and nothing more. I mean - instead of all your changes in this PR. Well, and the line 1398 for the Wide version.
Will it blend help?..

[fdcastel]

Unfortunately it's not so simple.

I built your one-liner against master and measured all four shapes. The short version: it recovers rebind_slong (row count 11 → 500) but leaves direct_slong and preponce_slong silently storing 500 NUL-corrupted PKs, same as pure master. Row counts stay at 500 there; the corruption is in the stored bytes.

Shape	master (no fix)	master + your one-liner	this PR
direct_char	count=500 nul=0 OK	count=500 nul=0 OK	count=500 nul=0 OK
direct_slong	count=500 nul=500 FAIL	count=500 nul=500 FAIL	count=500 nul=0 OK
preponce_slong	count=500 nul=500 FAIL	count=500 nul=500 FAIL	count=500 nul=0 OK
rebind_slong	count=11 nul=1 FAIL	count=500 nul=1 FAIL	count=500 nul=0 OK

(FB 5.0.3, CHARSET=UTF8, 500 rows, nul_rows = POSITION(_OCTETS x'00' IN ID) > 0.)

Which lines up with there being two separate bugs the PR addresses:

• Bug A — conv<Numeric>ToString writes ASCII digits at offset 0 of a SQL_VARYING target buffer (where FB expects a uint16 length prefix) and then calls setSqlLen(actual_len). FB master reads the digits as the prefix ('1','\0' = 49,
  '1','0' = 12337, …) and ends up storing bytes from past the copy range — the embedded NULs. Your one-liner doesn't touch either the offset-0 write or the setSqlLen, so this bug survives, and it fires on every shape that routes through conv*ToString, not just the rebind one.

• Bug B — Sqlda::getPrecision was reading the mutated sqlvar, so record->length collapsed to 1 after the first execute and multi-digit IDs got cut to their first character in the rebind path. Your octetLength switch sidesteps this, because octetLength is populated from getColumnDisplaySize, which already reads from orgVarSqlProperties (the immutable prepare-time snapshot). This is exactly why rebind_slong goes 11 → 500 with your change. Nice spot.

The PR addresses both: the VARYING-vs-TEXT write rule now lives in HeadSqlVar::writeStringData (offset 0 = uint16 prefix, data at offset 2, sqltype/sqllen untouched — and as a side-effect Sqlda::checkAndRebuild() no longer fires for these parameters at all), and Sqlda::getPrecision reads from orgVarSqlProperties for INPUT so the rebind path is immune too. Full patch is the e996b2b commit on top of fb356d6.

On "preponce_slong is the only sane scenario" — fair call, but direct_slong is just SQLExecDirect with a numeric parameter and rebind_slong is what odbc_scanner's odbc_copy_from (and a fair number of high-level wrappers) do under the hood. All three are legal ODBC and all three currently corrupt data on master; I'd rather not pick which ones we're willing to leave broken.

[fdcastel]

Pushed e40dfa8 — two additional tests in test_param_conversions.cpp so the PR's regression suite covers both bugs, not just one.

The original two tests (Issue161_SLongToVarcharViaStoredProcedure, Issue161_SLongToVarcharViaDml) both bind once and execute 500 times. That exercises Bug A — conv<Numeric>ToString writing over the SQL_VARYING length prefix — but not Bug B (Sqlda::getPrecision reading from the mutated sqlvar), because the preponce shape never re-enters defFromMetaDataIn and so never re-reads getPrecision after the first execute. A regression that reintroduced the pre-PR record->length = getPrecision(mutated_sqlvar) read would have slipped past both tests.

The two new tests

Test	Shape	Exercises
Issue161_SLongToVarcharViaDmlRebind	rebind_slong	Bug A + Bug B — re-binds per row
Issue161_SLongToVarcharViaDmlDirect	direct_slong	Bug A via SQLExecDirect (no prepare cache)

• Rebind variant: SQLPrepare once, then per row SQLFreeStmt(SQL_RESET_PARAMS) + SQLBindParameter(×2) +
  SQLExecute. This is what odbc-scanner's odbc_copy_from does internally, and the only shape where Bug B actually fires (a per-row SQLBindParameter forces defFromMetaDataIn to re-read getPrecision, which pre-PR returned the shrunken length from the mutated sqlvar).
• Direct variant: SQLExecDirect per row with SQL_C_SLONG, no prepared-statement context carried between iterations. Guards against any state-leak regression where the per-execute reset path diverges from the per-prepare path.

[fdcastel]

P.S.: I attempted to rename the (very poorly named) branch for this PR, but GitHub ended up closing the PR incorrectly.

So I’m leaving it as is for now.

[irodushka]

Oh my Lord... That's not addressed to you, @fdcastel, just an idle talk.

Finally I got it. The misbehavior occures when I set "CHARACTER SET UTF8" for varchar fields. And it works ok (with the 1-line fix of cause) when I use NONE charset.

The reason is - if we use the UTF8 charset, the WIDE int->string conv function is called, and there's an extremely bad place in it:

		{																						\
			char tempBuf [256];																	\
			strcpy( tempBuf, string );															\
			from->MbsToWcs( (wchar_t *)string, tempBuf, len );									\
			((wchar_t *)string)[len] = L'\0';													\
			len *=2;																			\
		}	

Such a familiar code, isn't it?)
It writes fixed 2-byte characters on Windows and 4-byte - on Linux. The lengths if hardcoded-multiplied by 2. All this logic is invalid in the both cases because UTF8 is not the widechar encoding, but the multi-byte, 1-4 bytes for one character, and for we use the pure ASCII, it must be 1 byte for character here.

Your fix works because you've added the gating logic, I mean chooseNumericToStringConv, and it always use the single-byte conv function when converting from the client var to the FB parameter. So the WIDE conv function is never called, and the toxic code above never works)
BUT I doubt this approach is correct... you're copying data as plain bytes from the client side buffer to the UTF8 FB buffer. It should be ok when the client-side is ASCII (as in the test is) - each ASCII byte corresponds exactly to one UTF8 byte. But what if it's not ASCII?

And the last - it does not relate to VARCHAR at all) Please try to create the test table as

RECREATE TABLE ISSUE161_T(
ID CHAR(20) CHARACTER SET UTF8 NOT NULL PRIMARY KEY,
NAME CHAR(100) CHARACTER SET UTF8
)

and run your tests...

You can also create table with CHARACTER SET NONE and check. I ran the test on Linux, not Windows, and I got 4/4 OK with NONE charset (CHAR or VARCHAR) and 3/4 FAIL with UTF8 (CHAR or VARCHAR). I expect you'll get similar results on Windows.

[irodushka]

you can:

1. make the 1-line fix in 2 places (for char & for widechar conv, length->octetLength)
2. add the condition if(from->isIndicatorSqlDa) before the bad code block in the widechar conv (with from->MbsToWcs...)

As for me, this solves the problem completely. Linux.

P.S. And yes, it is true that when converting from any data type (except string) to string, we cannot get anything but ASCII characters... so your control logic (chooseNumericToStringConv) in these cases looks correct... Just fix the naming - it's not always Numeric. Maybe selectToStringConv?..

[fdcastel]

Almost 😅

TL;DR Your minimal fix (3 lines) works for the write direction: my v3 theory was wrong. But unfortunately it breaks 4 pre-existing tests in the read direction.

---

Tested both of your proposals empirically:

CHAR target and CHARACTER SET NONE

• pushed 220eb3c and 9a1a432.
• Three new gtest cases covering CHAR(20) + UTF8, VARCHAR(20) + NONE, CHAR(20) + NONE in the rebind shape;
  • a cherry-picked rdb$get_context fix for GetServerMajorVersion so SKIP_ON_FIREBIRD6 stops firing on FB 5.
• Combined effect on run 24850170760:
  • 7/7 Issue161 tests
  • Passed on ubuntu-22.04 / 5.0.3 AND on windows-2022 / 5.0.3, correctly Skipped on FB 6 master
  • so the coverage is real on both OSes now, not silent skips.

Your 3-line minimal fix (line 1306 + 1387 + from->isIndicatorSqlDagate)

• I applied it on top of clean master, pushed to a scratch branch, and ran the full CI matrix (run 24851385888, branch since deleted).
• On the write-direction matrix it passes exactly as you described on Linux: all 16 (column type × charset × shape) combinations OK, including CHAR + UTF8.
• My earlier claim that the VARYING prefix stomp would persist under your minimal fix was wrong: the OO-API path via checkAndRebuild() handles the sqllen mutation fine. Apologies for the sloppy theoretical argument.

But

• the minimal fix regressed 4 pre-existing read-direction tests on every matrix job: DataTypeTest.NumericPrecision, ResultConversionsTest.{IntegerToChar, NumericToChar, BooleanToChar}.
• The reason is that s/to->length/to->octetLength/ at line 1306 affects both directions
  • and on the read direction (FB column → client SQL_C_CHAR buffer) to->length and to->octetLength are not the same:
  • to->length is the declared capacity (SQL_DESC_LENGTH) while to->octetLength can come from the source column's display size and be smaller.
• Using octetLength as the truncation cap on the read path writes only part of the formatted number and the test fails.

This PR's chooseNumericToStringConv avoids that: it routes only when to->isIndicatorSqlDa is true (write-to-FB), leaving the read path on the wide variant unchanged and to->length semantics untouched.

I can't see a tweak to your 3-line approach that keeps it 3 lines without introducing the same to->isIndicatorSqlDa branching we already have.

On chooseNumericToStringConv naming

Agreed, the helper covers date/time as well, not just numerics. I'll rename to selectToStringConv in a follow-up commit on this PR.

On "what if not ASCII"

• the specific converters this PR touches (conv<Numeric|Date|Time|DateTime>ToString{,W}) emit only ASCII: 0–9, ., -, :, , e/E, *.
• No path in those macros produces a byte > 0x7F, so routing the UTF8 / WVARCHAR target through the byte variant is bit-identical regardless of charset.
• The character-data family (conv<String>ToString*) is a different code path, unchanged by this PR, and continues to flow through transferString*ToAllowedType with charset awareness.

Next steps

I need TWO beers, now.

[irodushka]

No need to use the exact 3-lines patch, it was for illustration only. I think it's ok to separate logic for read&write directions. And yes, that legacy code is very, very, VERY entangled) чт, 23 апр. 2026 г., 21:43 F.D.Castel ***@***.***>:

…

*fdcastel* left a comment (FirebirdSQL/firebird-odbc-driver#292) <#292?email_source=notifications&email_token=AAKZQLWDIDVTKYZDM4QOMOT4XJP4JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGY4TENBRG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4306924170> Almost 😅 *TL;DR* Your minimal fix (3 lines) works for the *write* direction: my v3 theory was wrong. But unfortunately it breaks 4 pre-existing tests in the *read* direction. ------------------------------ Tested both of your proposals empirically: CHAR target and CHARACTER SET NONE - pushed 220eb3c <fdcastel@220eb3c> and 9a1a432 <fdcastel@9a1a432>. - Three new gtest cases covering CHAR(20) + UTF8, VARCHAR(20) + NONE, CHAR(20) + NONE in the rebind shape; - a cherry-picked rdb$get_context fix for GetServerMajorVersion so SKIP_ON_FIREBIRD6 stops firing on FB 5. - Combined effect on run 24850170760 <https://github.com/fdcastel/firebird-odbc-driver/actions/runs/24850170760>: - 7/7 Issue161 tests - Passed on ubuntu-22.04 / 5.0.3 AND on windows-2022 / 5.0.3, correctly Skipped on FB 6 master - so the coverage is real on both OSes now, not silent skips. Your 3-line minimal fix (line 1306 + 1387 + from->isIndicatorSqlDagate) - I applied it on top of clean master, pushed to a scratch branch, and ran the full CI matrix (run 24851385888 <https://github.com/fdcastel/firebird-odbc-driver/actions/runs/24851385888>, branch since deleted). - On the write-direction matrix *it passes exactly as you described on Linux*: all 16 (column type × charset × shape) combinations OK, including CHAR + UTF8. - *My earlier claim that the VARYING prefix stomp would persist under your minimal fix was wrong*: the OO-API path via checkAndRebuild() handles the sqllen mutation fine. Apologies for the sloppy theoretical argument. But - the minimal fix regressed 4 pre-existing read-direction tests on every matrix job: DataTypeTest.NumericPrecision, ResultConversionsTest.{IntegerToChar, NumericToChar, BooleanToChar}. - The reason is that s/to->length/to->octetLength/ at line 1306 *affects both directions* - and on the read direction (FB column → client SQL_C_CHAR buffer) to->length and to->octetLength are not the same: - to->length is the declared capacity (SQL_DESC_LENGTH) while to->octetLength can come from the source column's display size and be smaller. - Using octetLength as the truncation cap on the read path writes only part of the formatted number and the test fails. This PR's chooseNumericToStringConv avoids that: it routes only when to->isIndicatorSqlDa is true (write-to-FB), leaving the read path on the wide variant unchanged and to->length semantics untouched. I can't see a tweak to your 3-line approach that keeps it 3 lines without introducing the same to->isIndicatorSqlDa branching we already have. On chooseNumericToStringConv naming Agreed, the helper covers date/time as well, not just numerics. I'll rename to selectToStringConv in a follow-up commit on this PR. On "what if not ASCII" - the specific converters this PR touches ( conv<Numeric|Date|Time|DateTime>ToString{,W}) emit only ASCII: 0–9, ., -, :, , e/E, *. - No path in those macros produces a byte > 0x7F, so routing the UTF8 / WVARCHAR target through the byte variant is bit-identical regardless of charset. - The character-data family (conv<String>ToString*) is a different code path, unchanged by this PR, and continues to flow through transferString*ToAllowedType with charset awareness. Next steps I need TWO beers, now. — Reply to this email directly, view it on GitHub <#292?email_source=notifications&email_token=AAKZQLWDIDVTKYZDM4QOMOT4XJP4JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGY4TENBRG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4306924170>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKZQLRR2JK3F6F2SIRPWN34XJP4JAVCNFSM6AAAAACYBN4RGGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGMBWHEZDIMJXGA> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAKZQLUBYJAGO37ENWG7Z2L4XJP4JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGY4TENBRG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AAKZQLQDMW3MMGFIQ6EE7BL4XJP4JA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMZQGY4TENBRG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>