diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 7c64dee21f5b..cbfef0bae802 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -1581,8 +1581,14 @@ SQLRETURN SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR* co case SQL_REAL: case SQL_FLOAT: case SQL_DOUBLE: { - ird->GetField(column_number, SQL_DESC_PRECISION, column_size_ptr, - sizeof(SQLULEN), nullptr); + // SQL_DESC_PRECISION is stored as a SQLSMALLINT. Read it into a + // correctly-sized local and widen into the SQLULEN output. Passing the + // SQLULEN* directly would only write the low 2 bytes and leave the + // upper 6 bytes uninitialized. + SQLSMALLINT precision = 0; + ird->GetField(column_number, SQL_DESC_PRECISION, &precision, sizeof(precision), + nullptr); + *column_size_ptr = static_cast(precision); break; } @@ -1603,8 +1609,10 @@ SQLRETURN SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR* co case SQL_BIGINT: case SQL_DECIMAL: case SQL_NUMERIC: { + // decimal_digits_ptr is a SQLSMALLINT*; SQL_DESC_SCALE is stored as a + // SQLSMALLINT. Pass the matching buffer size. ird->GetField(column_number, SQL_DESC_SCALE, decimal_digits_ptr, - sizeof(SQLULEN), nullptr); + sizeof(SQLSMALLINT), nullptr); break; } @@ -1621,8 +1629,10 @@ SQLRETURN SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR* co case SQL_INTERVAL_MINUTE_TO_SECOND: case SQL_INTERVAL_HOUR_TO_SECOND: case SQL_INTERVAL_DAY_TO_SECOND: { + // decimal_digits_ptr is a SQLSMALLINT*; SQL_DESC_PRECISION is stored as + // a SQLSMALLINT. Pass the matching buffer size. ird->GetField(column_number, SQL_DESC_PRECISION, decimal_digits_ptr, - sizeof(SQLULEN), nullptr); + sizeof(SQLSMALLINT), nullptr); break; } diff --git a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt index 0f45bfd0c811..d5489b0f8806 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt @@ -32,6 +32,7 @@ set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS connection_attr_test.cc connection_info_test.cc connection_test.cc + describe_col_test.cc errors_test.cc get_functions_test.cc statement_attr_test.cc diff --git a/cpp/src/arrow/flight/sql/odbc/tests/describe_col_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/describe_col_test.cc new file mode 100644 index 000000000000..68288fafc6e3 --- /dev/null +++ b/cpp/src/arrow/flight/sql/odbc/tests/describe_col_test.cc @@ -0,0 +1,125 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +#include "arrow/flight/sql/odbc/tests/odbc_test_suite.h" + +#include "arrow/flight/sql/odbc/odbc_impl/platform.h" + +#include +#include +#include + +#include + +namespace arrow::flight::sql::odbc { + +// These tests run against the remote server only. The mock SQLite server +// reports SQL_WVARCHAR metadata for `SELECT ... AS` result columns (see +// ColumnsMockTest in columns_test.cc), so it cannot exercise the numeric or +// datetime branches of SQLDescribeCol that this fix touches. +class DescribeColRemoteTest : public FlightSQLODBCRemoteTestBase {}; + +namespace { +// Sentinel used to pre-fill the SQLULEN column_size output. If SQLDescribeCol +// only writes the low bytes (the width bug this test guards against), the upper +// bytes remain set to this pattern and the value is far outside any legal +// column size. +constexpr SQLULEN kColumnSizeSentinel = static_cast(0xFFFFFFFFFFFFFFFFULL); + +// Column ordinals in the all-data-types query (see +// ODBCTestBase::GetQueryAllDataTypes). Decimal and timestamp exercise the +// numeric-precision and datetime-precision code paths in SQLDescribeCol. +constexpr SQLUSMALLINT kDecimalColumn = 17; +constexpr SQLUSMALLINT kTimestampColumn = 31; +} // namespace + +// Verify that SQLDescribeCol fully initializes the SQLULEN column_size output +// for a DECIMAL column. Prior to GH-50560 the numeric path read +// SQL_DESC_PRECISION (a SQLSMALLINT) straight into the SQLULEN* output, writing +// only 2 of the 8 bytes and leaving the upper 6 bytes as uninitialized garbage. +TEST_F(DescribeColRemoteTest, TestSQLDescribeColDecimalColumnSizeIsFullyWritten) { + std::wstring wsql = this->GetQueryAllDataTypes(); + std::vector sql0(wsql.begin(), wsql.end()); + + ASSERT_EQ(SQL_SUCCESS, + SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size()))); + + SQLWCHAR column_name[kOdbcBufferSize]; + SQLSMALLINT name_length = 0; + SQLSMALLINT data_type = 0; + SQLULEN column_size = kColumnSizeSentinel; + SQLSMALLINT decimal_digits = -1; + SQLSMALLINT nullable = 0; + + ASSERT_EQ(SQL_SUCCESS, + SQLDescribeCol(this->stmt, kDecimalColumn, column_name, + sizeof(column_name) / sizeof(SQLWCHAR), &name_length, + &data_type, &column_size, &decimal_digits, &nullable)); + + EXPECT_EQ(SQL_DECIMAL, data_type); + + // The precision (column size) is a small positive integer. If only the low + // bytes were written, the sentinel's upper bytes would remain set and the + // full 8-byte value would be enormous. Checking the whole SQLULEN (not just a + // truncated view of it) is what catches the short write. + EXPECT_NE(kColumnSizeSentinel, column_size); + EXPECT_GT(column_size, 0u); + EXPECT_LT(column_size, 100u) << "column_size upper bytes look uninitialized: 0x" + << std::hex << column_size; + + // decimal_digits (scale) is a SQLSMALLINT output; it must be a sane scale. + EXPECT_GE(decimal_digits, 0); + EXPECT_LE(static_cast(decimal_digits), column_size); +} + +// Verify SQLDescribeCol reports a fully-written column_size for a TIMESTAMP +// column. This exercises the datetime branch, which reports the display size +// via SQL_DESC_LENGTH (a SQLULEN) and the seconds precision via +// SQL_DESC_PRECISION into the SQLSMALLINT decimal_digits output. +TEST_F(DescribeColRemoteTest, TestSQLDescribeColTimestampColumnSizeIsFullyWritten) { + std::wstring wsql = this->GetQueryAllDataTypes(); + std::vector sql0(wsql.begin(), wsql.end()); + + ASSERT_EQ(SQL_SUCCESS, + SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size()))); + + SQLWCHAR column_name[kOdbcBufferSize]; + SQLSMALLINT name_length = 0; + SQLSMALLINT data_type = 0; + SQLULEN column_size = kColumnSizeSentinel; + SQLSMALLINT decimal_digits = -1; + SQLSMALLINT nullable = 0; + + ASSERT_EQ(SQL_SUCCESS, + SQLDescribeCol(this->stmt, kTimestampColumn, column_name, + sizeof(column_name) / sizeof(SQLWCHAR), &name_length, + &data_type, &column_size, &decimal_digits, &nullable)); + + EXPECT_EQ(SQL_TYPE_TIMESTAMP, data_type); + + // The timestamp display size is a small integer (e.g. 19 or 23). A short + // write would leave the sentinel's upper bytes intact. + EXPECT_NE(kColumnSizeSentinel, column_size); + EXPECT_GT(column_size, 0u); + EXPECT_LT(column_size, 100u) << "column_size upper bytes look uninitialized: 0x" + << std::hex << column_size; + + // Seconds precision is a small non-negative scale. + EXPECT_GE(decimal_digits, 0); + EXPECT_LE(decimal_digits, 9); +} + +} // namespace arrow::flight::sql::odbc