Skip to content

Commit 98f81cc

Browse files
GH-50852: [C++][FlightRPC][ODBC] Decode wide connection/descriptor string attributes with the wide decoder
SetConnectAttr(SQL_ATTR_CURRENT_CATALOG) had its is_unicode decode branches swapped, and SetField(SQL_DESC_NAME) unconditionally used the byte-wise SetAttributeUTF8. In both cases a wide SQLWCHAR buffer was misread, corrupting the stored value. Decode wide buffers with SetAttributeSQLWCHAR, matching the mapping the getters already use (GetStringAttribute / GetAttributeSQLWCHAR). Add a round-trip TYPED_TEST for SQL_ATTR_CURRENT_CATALOG through the wide entry point (connection_attr_test.cc) and a SetField/GetField round-trip unit test for SQL_DESC_NAME (odbc_descriptor_test.cc). Co-authored-by: Isaac
1 parent a7d0bfa commit 98f81cc

5 files changed

Lines changed: 77 additions & 3 deletions

File tree

‎cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ add_arrow_test(odbc_spi_impl_test
186186
json_converter_test.cc
187187
record_batch_transformer_test.cc
188188
util_test.cc
189+
odbc_descriptor_test.cc
189190
EXTRA_LINK_LIBS
190191
arrow_odbc_spi_impl
191192
${ODBC_SPI_IMPL_TEST_LINK_LIBS})

‎cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,9 @@ void ODBCConnection::SetConnectAttr(SQLINTEGER attribute, SQLPOINTER value,
465465
case SQL_ATTR_CURRENT_CATALOG: {
466466
std::string catalog;
467467
if (is_unicode) {
468-
SetAttributeUTF8(value, string_length, catalog);
469-
} else {
470468
SetAttributeSQLWCHAR(value, string_length, catalog);
469+
} else {
470+
SetAttributeUTF8(value, string_length, catalog);
471471
}
472472
if (!spi_connection_->SetAttribute(Connection::CURRENT_CATALOG, catalog)) {
473473
throw DriverException("Option value changed.", "01S02");

‎cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ void ODBCDescriptor::SetField(SQLSMALLINT record_number, SQLSMALLINT field_ident
215215
has_bindings_changed_ = true;
216216
break;
217217
case SQL_DESC_NAME:
218-
SetAttributeUTF8(value, buffer_length, record.name);
218+
SetAttributeSQLWCHAR(value, buffer_length, record.name);
219219
has_bindings_changed_ = true;
220220
break;
221221
case SQL_DESC_OCTET_LENGTH:
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#include "arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.h"
19+
20+
#include "arrow/flight/sql/odbc/odbc_impl/diagnostics.h"
21+
#include "arrow/flight/sql/odbc/odbc_impl/encoding.h"
22+
#include "arrow/flight/sql/odbc/odbc_impl/encoding_utils.h"
23+
24+
#include <sql.h>
25+
#include <sqlext.h>
26+
27+
#include "gtest/gtest.h"
28+
29+
namespace arrow::flight::sql::odbc {
30+
31+
using ODBC::ODBCDescriptor;
32+
33+
TEST(ODBCDescriptorTest, SetGetNameWideRoundTrips) {
34+
arrow::flight::sql::odbc::Diagnostics diagnostics("vendor", "component",
35+
OdbcVersion::V_3);
36+
ODBCDescriptor desc(diagnostics, nullptr, nullptr, /*is_app_descriptor=*/true,
37+
/*is_writable=*/true, /*is_2x_connection=*/false);
38+
39+
SQLSMALLINT count = 1;
40+
desc.SetHeaderField(SQL_DESC_COUNT, reinterpret_cast<SQLPOINTER>(&count), 0);
41+
42+
std::vector<uint8_t> wide;
43+
Utf8ToWcs("my_column", &wide);
44+
desc.SetField(1, SQL_DESC_NAME, wide.data(), static_cast<SQLINTEGER>(wide.size()));
45+
46+
SQLWCHAR out[256];
47+
SQLINTEGER out_len = 0;
48+
desc.GetField(1, SQL_DESC_NAME, out, sizeof(out), &out_len);
49+
std::string name =
50+
ODBC::SqlWcharToString(out, static_cast<SQLSMALLINT>(out_len / GetSqlWCharSize()));
51+
EXPECT_EQ("my_column", name);
52+
}
53+
54+
} // namespace arrow::flight::sql::odbc

‎cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,4 +410,23 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrPacketSizeValid) {
410410
#endif
411411
}
412412

413+
// A multi-character catalog set through the wide entry point must round-trip
414+
// intact.
415+
TYPED_TEST(ConnectionAttributeTest, TestSQLSetGetConnectAttrCurrentCatalogWide) {
416+
ASSIGN_SQLWCHAR_ARR_AND_LEN(catalog, L"my_catalog");
417+
418+
ASSERT_EQ(SQL_SUCCESS, SQLSetConnectAttr(this->conn, SQL_ATTR_CURRENT_CATALOG, catalog,
419+
catalog_len * GetSqlWCharSize()));
420+
421+
SQLWCHAR out_str[kOdbcBufferSize];
422+
SQLINTEGER out_str_len;
423+
ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_CURRENT_CATALOG, out_str,
424+
kOdbcBufferSize, &out_str_len));
425+
// SQLGetConnectAttr returns the length in bytes; convert to characters.
426+
out_str_len /= GetSqlWCharSize();
427+
std::string out_catalog =
428+
ODBC::SqlWcharToString(out_str, static_cast<SQLSMALLINT>(out_str_len));
429+
EXPECT_EQ("my_catalog", out_catalog);
430+
}
431+
413432
} // namespace arrow::flight::sql::odbc

0 commit comments

Comments
 (0)