Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions cpp/src/arrow/flight/sql/odbc/odbc_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<SQLULEN>(precision);
break;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
125 changes: 125 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/tests/describe_col_test.cc
Original file line number Diff line number Diff line change
@@ -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 <sql.h>
#include <sqltypes.h>
#include <sqlucode.h>

#include <gtest/gtest.h>

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<SQLULEN>(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<SQLWCHAR> sql0(wsql.begin(), wsql.end());

ASSERT_EQ(SQL_SUCCESS,
SQLExecDirect(this->stmt, &sql0[0], static_cast<SQLINTEGER>(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<SQLULEN>(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<SQLWCHAR> sql0(wsql.begin(), wsql.end());

ASSERT_EQ(SQL_SUCCESS,
SQLExecDirect(this->stmt, &sql0[0], static_cast<SQLINTEGER>(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
Loading