ODBC: use the length indicator of the current row of a parameter array - #13610
Open
singhpratech wants to merge 1 commit into
Open
singhpratech wants to merge 1 commit into
singhpratech wants to merge 1 commit into
Conversation
Parameter::Write tested buffer.GetInputSize() on the parameter's own buffer, before the byte and element offsets of the row being written were applied, so the NULL indicator of the first row decided the NULL-ness of every row of the array. A NULL below the first row was written as an empty string for a character parameter and made the client pass a length of -1 to memcpy for a binary one, while a NULL in the first row made every row NULL. Apply both offsets to the copy of the buffer first and test that copy, so that the indicator is read at the same offset as the value. Also treat a negative length of a binary parameter as NULL instead of passing it on as an array length.
singhpratech
added a commit
to singhpratech/adbcbridge
that referenced
this pull request
Sep 24, 2026
) (#144) * docs: link omniload's actual release tag, and the install extra The README pointed at omniload's generic releases page. It now links v0.17.0, which is the release that first carried the adbcbridge backend: our merge commit is dated 2026-09-17T13:57:29Z and the v0.17.0 release commit 14:01:26Z, four minutes later, with the extra present in that tag's pyproject and absent from v0.16.0's. Adds the install line, since that is what a reader wants. PyPI's metadata for the published package declares the extra as adbcbridge>=0.1.3,<0.2, and omniload's own full and test extras pull it in. * scripts: a version inside a link belongs to the project the link points at The README's new omniload line names our own backend and cites omniload's release tag on the same line, so looks_like_ours() read v0.17.0 as an adbcbridge version and the version-agreement check failed. A markdown link now decides on its own: a version inside [text](url) is ours only when the url is. A stale version of ours in a link to our own repo is still caught, and another project's version in a link to theirs is not. * docs: Ignite ODBC parameter-array fix sent upstream (apache/ignite#13610) The fix prepared on 2026-09-18 is now a pull request. It had been held for the ASF JIRA ticket Ignite asks for; the ticket still does not exist, since self-signup on issues.apache.org is closed and contributor access has to be requested on dev@ignite.apache.org. Rather than invent an IGNITE-NNNNN number, the pull request states the position openly and offers to be retitled or reopened once the ticket is filed. The issue has had no reply since 2026-08-29.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13537.
With
SQL_ATTR_PARAMSET_SIZE> 1 and column-wise binding, the ODBC driver decides whether a parameter is NULL from the indicator of the first row of the array, so aSQL_NULL_DATAbelow the first row is never sent as NULL.Binding three rows whose indicators are
{SQL_NTS, SQL_NULL_DATA, SQL_NTS}:VARCHARSQL_SUCCESS, every parameter statusSQL_PARAM_SUCCESSBINARYSQLExecuteA NULL in row 0 makes every row NULL, and row-wise binding is unaffected — which is what points at the indicator being read at the wrong index.
Cause
Parameter::Writetestedbuffer.GetInputSize()on the parameter's own buffer, before the byte and element offsets of the row being written were applied. The value was then read from a copy with the offsets applied, so the indicator and the value came from different rows. For theBINARYpath the resulting-1length reachedmemcpyas an array length, which is the crash.Fix
Apply both offsets to the copy first and test that copy, so the indicator is read at the same offset as the value. Separately, treat a negative length on the binary path as NULL rather than passing it on as a length. Two small changes in
modules/platforms/cpp/odbc/src/app/parameter.cpp.Tests
modules/platforms/cpp/odbc-test/src/parameter_test.cpp, new, added to that module'sCMakeLists.txt. It covers the indicator being taken from the current row for character, binary and integer parameters, a NULL in row 0, a NULL below row 0, and the negative-length binary case, by exercisingParameter::Writeagainst a binary writer — no server needed, so it runs in the unit part of the ODBC suite.The self-contained C reproducer from #13537 was re-run against a build with this change: all three rows round-trip, row 2 reads back as NULL for both
VARCHARandBINARY, and theBINARYcase no longer crashes.On the JIRA ticket
CONTRIBUTING.mdasks for an IGNITE ticket and anIGNITE-NNNNNtitle, and I would rather not invent a number. I have asked ondev@ignite.apache.orgfor JIRA contributor access so that I can file it properly; as soon as the ticket exists I will rename the branch and retitle this pull request, or close it and reopen under the ticket if you prefer that. The issue has been open on GitHub since 29 August with the reproducer, and I did not want the fix to sit unavailable in the meantime. Happy to follow whatever process suits you.For context on where it came from: this surfaced while testing Ignite through adbcBridge (https://github.com/singhpratech/adbcbridge), a driver for ADBC — Apache Arrow's database connectivity API — that works over any ODBC driver, where binding a column of values containing NULLs is an everyday operation.