singhpratech opened a new pull request, #13610:
URL: https://github.com/apache/ignite/pull/13610
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 a `SQL_NULL_DATA` below the first row is never sent as NULL.
Binding three rows whose indicators are `{SQL_NTS, SQL_NULL_DATA, SQL_NTS}`:
| column type | expected | 2.17 actual |
| --- | --- | --- |
| `VARCHAR` | row 2 is NULL | row 2 is a non-NULL empty string,
`SQL_SUCCESS`, every parameter status `SQL_PARAM_SUCCESS` |
| `BINARY` | row 2 is NULL | client segfaults inside `SQLExecute` |
A 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::Write` tested `buffer.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 the `BINARY` path the
resulting `-1` length reached `memcpy` as 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's `CMakeLists.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 exercising `Parameter::Write`
against 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
`VARCHAR` and `BINARY`, and the `BINARY` case no longer crashes.
## On the JIRA ticket
`CONTRIBUTING.md` asks for an IGNITE ticket and an `IGNITE-NNNNN` title, and
I would rather not invent a number. I have asked on `[email protected]`
for 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
