vikrantpuppala opened a new pull request, #50562:
URL: https://github.com/apache/arrow/pull/50562
Closes #50560.
### Rationale for this change
`SQLDescribeCol` returned uninitialized garbage in the upper bytes of its
`column_size` (`ColumnSizePtr`) output for numeric / decimal / integer /
float
columns.
The numeric path read `SQL_DESC_PRECISION` — stored internally as a
`SQLSMALLINT` (2 bytes) — directly into the caller's `SQLULEN*
column_size_ptr`
(8 bytes). The `GetAttribute` template that ultimately performs the write
copies
exactly `sizeof(T)` bytes (here `T = SQLSMALLINT`, so 2 bytes) and ignores
the
claimed `buffer_length`. The result: only the low 2 bytes of the 8-byte
`SQLULEN` were written and the upper 6 bytes were left uninitialized. Any
application reading the full `column_size` saw a wildly incorrect value.
Two adjacent `decimal_digits` sites (`SQL_DESC_SCALE` for exact-numeric
columns,
`SQL_DESC_PRECISION` for datetime/interval columns) passed `sizeof(SQLULEN)`
as
the buffer size even though `decimal_digits_ptr` is a `SQLSMALLINT*`. Those
did
not corrupt memory (the write width matched the 2-byte target), but the size
argument was wrong and misleading.
Introduced by GH-47724 (#48052).
### What changes are included in this PR?
- Numeric `column_size` path: read `SQL_DESC_PRECISION` into a local
`SQLSMALLINT` and widen-assign it into `*column_size_ptr`, so all 8 bytes
of
the `SQLULEN` output are written.
- `decimal_digits` scale and datetime-precision paths: pass
`sizeof(SQLSMALLINT)` (the true target width) instead of `sizeof(SQLULEN)`.
- Added `describe_col_test.cc` with parameterized (mock + remote) tests that
pre-fill `column_size` with an all-ones sentinel, call `SQLDescribeCol` on
a
`DECIMAL` column and a `TIMESTAMP` column, and assert the full 8-byte
`column_size` is a sane small value (i.e. the upper bytes were
overwritten),
plus sane `decimal_digits`. Without the fix these fail because the
sentinel's
upper bytes survive the short write.
### Are these changes tested?
Yes — new gtest cases in
`cpp/src/arrow/flight/sql/odbc/tests/describe_col_test.cc`, wired into the
ODBC
test `CMakeLists.txt`.
### Are there any user-facing changes?
Yes. `SQLDescribeCol` now returns a correct `column_size` for numeric/decimal
columns instead of a value with uninitialized high bytes. This is a bug fix
in
externally-visible ODBC behavior; no API signatures change.
**This PR contains a "Critical Fix".** A short write of uninitialized memory
into a caller-provided buffer is a correctness bug that produces
nondeterministic wrong results for every numeric/decimal column described via
`SQLDescribeCol`.
--
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]
