You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alinaliBQ (via GitHub)" <gi...@apache.org> on 2025/10/09 22:06:10 UTC
[PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
alinaliBQ opened a new pull request, #47772:
URL: https://github.com/apache/arrow/pull/47772
### Rationale for this change
Add connection attribute support for ODBC driver.
### What changes are included in this PR?
- Implementation of `SQLGetConnectAttr` and `SQLSetConnectAttr` to get and set connection attributes
- Tests
### Are these changes tested?
Will be tested in CI when PR is ready for review
### Are there any user-facing changes?
No
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #47772:
URL: https://github.com/apache/arrow/pull/47772
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3590624027
@lidavidm Thank you for the ping, rebased and resolved conflict
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3387915845
Could you create new draft PRs after you complete other draft PRs to reflect comments in other draft PRs?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2418267446
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,565 @@
+// 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"
+
+#ifdef _WIN32
+# include <windows.h>
+#endif
Review Comment:
Can we use `platform.h` as I commented in other PR?
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,565 @@
+// 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"
+
+#ifdef _WIN32
+# include <windows.h>
+#endif
+
+#include <sql.h>
+#include <sqltypes.h>
+#include <sqlucode.h>
+
+#include "gtest/gtest.h"
+
+namespace arrow::flight::sql::odbc {
+
+#ifdef SQL_ATTR_ASYNC_DBC_EVENT
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLSetConnectAttrAsyncDbcEventUnsupported) {
+ this->Connect();
Review Comment:
`SetUp()` as David commented in other PR?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3592596183
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 95cdc0c11d20fab563416afeba055fe43113b75b.
There was 1 benchmark result indicating a performance regression:
- Commit Run on `amd64-m5-4xlarge-linux` at [2025-11-30 12:56:29Z](https://conbench.ursa.dev/compare/runs/e6f4855e49ca421588de0c8a83b5eb64...68c154981f4343abb3e9d50477120220/)
- [`dataset-read` (Python) with async=True, dataset=nyctaxi_multi_ipc_s3](https://conbench.ursa.dev/compare/benchmarks/0692b1f61d3c7a878000ad6e28d97475...0692c078cb77770080007fbcac6b0343)
The [full Conbench report](https://github.com/apache/arrow/runs/56726483013) has more details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3391402237
@lidavidm @kou Sure, will check the general comments first before creating new PRs. I will put the PRs to a pause for now since the comments are similar. Thanks for letting me know
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2418267887
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
Review Comment:
Same comments as other PRs:
- We can create a separate test fixture
- We can put Connect/Disconnect in SetUp/TearDown
- We should probably be using ASSERT_ not EXPECT_
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2449829481
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
Review Comment:
Changed to use `ASSERT_` where applicable. Other items are in-progress. `Connect/Disconnect` have been removed from tests and we will move it to `SetUp/TearDown`
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,565 @@
+// 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"
+
+#ifdef _WIN32
+# include <windows.h>
+#endif
+
+#include <sql.h>
+#include <sqltypes.h>
+#include <sqlucode.h>
+
+#include "gtest/gtest.h"
+
+namespace arrow::flight::sql::odbc {
+
+#ifdef SQL_ATTR_ASYNC_DBC_EVENT
+TYPED_TEST(FlightSQLODBCTestBase, TestSQLSetConnectAttrAsyncDbcEventUnsupported) {
+ this->Connect();
Review Comment:
removed `this->Connect()` from tests. we will move it to `SetUp`
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,565 @@
+// 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"
+
+#ifdef _WIN32
+# include <windows.h>
+#endif
Review Comment:
yes, fixed
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3387624199
:warning: GitHub issue #47708 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2456791454
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,362 @@
+// 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 {
+
+template <typename T>
+class ConnectionAttributeTest : public T {};
+
+using TestTypes =
+ ::testing::Types<FlightSQLODBCMockTestBase, FlightSQLODBCRemoteTestBase>;
+TYPED_TEST_SUITE(ConnectionAttributeTest, TestTypes);
+
+#ifdef SQL_ATTR_ASYNC_DBC_EVENT
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAsyncDbcEventUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_EVENT, 0, 0));
+ // Driver Manager on Windows returns error code HY118
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY118);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_ENABLE
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncEnableUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_ENABLE, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_DBC_PCALLBACK
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncDbcPcCallbackUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCALLBACK, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_DBC_PCONTEXT
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncDbcPcContextUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCONTEXT, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAutoIpdReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_AUTO_IPD, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrConnectionDeadReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_CONNECTION_DEAD, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+#ifdef SQL_ATTR_DBC_INFO_TOKEN
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrDbcInfoTokenUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrEnlistInDtcUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ENLIST_IN_DTC, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrOdbcCursorsDMOnly) {
+ this->AllocEnvConnHandles();
+
+ // Verify DM-only attribute is settable via Driver Manager
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS,
+ reinterpret_cast<SQLPOINTER>(SQL_CUR_USE_DRIVER), 0));
+
+ std::string connect_str = this->GetConnectionString();
+ this->ConnectWithString(connect_str);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrQuietModeReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_QUIET_MODE, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTraceDMOnly) {
+ // Verify DM-only attribute is settable via Driver Manager
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_TRACE,
+ reinterpret_cast<SQLPOINTER>(SQL_OPT_TRACE_OFF), 0));
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTracefileDMOnly) {
+ // Verify DM-only attribute is handled by Driver Manager
+
+ // Use placeholder value as we want the call to fail, or else
+ // the driver manager will produce a trace file.
+ std::wstring trace_file = L"invalid/file/path";
+ std::vector<SQLWCHAR> trace_file0(trace_file.begin(), trace_file.end());
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRACEFILE, &trace_file0[0],
+ static_cast<SQLINTEGER>(trace_file0.size())));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY000);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTranslateLabDMOnly) {
+ // Verify DM-only attribute is handled by Driver Manager
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_LIB, 0, 0));
+ // Checks for invalid argument return error
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY024);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTranslateOptionUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_OPTION, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTxnIsolationUnsupported) {
+ ASSERT_EQ(SQL_ERROR,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_TXN_ISOLATION,
+ reinterpret_cast<SQLPOINTER>(SQL_TXN_READ_UNCOMMITTED), 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+#ifdef SQL_ATTR_DBC_INFO_TOKEN
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrDbcInfoTokenSetOnly) {
+ // Verify that set-only attribute cannot be read
+ SQLPOINTER ptr = NULL;
+ ASSERT_EQ(SQL_ERROR, SQLGetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, ptr, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrOdbcCursorsDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLULEN cursor_attr;
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS, &cursor_attr, 0, 0));
+ EXPECT_EQ(SQL_CUR_USE_DRIVER, cursor_attr);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTraceDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLUINTEGER trace;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_TRACE, &trace, 0, 0));
+ EXPECT_EQ(SQL_OPT_TRACE_OFF, trace);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTraceFileDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLWCHAR out_str[kOdbcBufferSize];
+ SQLINTEGER out_str_len;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_TRACEFILE, out_str,
+ kOdbcBufferSize, &out_str_len));
+ // Length is returned in bytes for SQLGetConnectAttr,
+ // we want the number of characters
+ out_str_len /= arrow::flight::sql::odbc::GetSqlWCharSize();
+ std::string out_connection_string =
+ ODBC::SqlWcharToString(out_str, static_cast<SQLSMALLINT>(out_str_len));
+ EXPECT_TRUE(!out_connection_string.empty());
Review Comment:
fixed
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3387624575
@lidavidm @kou Please review this draft ODBC API PR, thanks. The testing folder structure will be in a separate PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #47772:
URL: https://github.com/apache/arrow/pull/47772#issuecomment-3587931836
@alinaliBQ can you resolve conflicts?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2453537483
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
Review Comment:
Updated to use separate test fixture
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-47708: [C++][FlightRPC] Connection Attribute Support for ODBC [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #47772:
URL: https://github.com/apache/arrow/pull/47772#discussion_r2454494162
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc:
##########
@@ -0,0 +1,362 @@
+// 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 {
+
+template <typename T>
+class ConnectionAttributeTest : public T {};
+
+using TestTypes =
+ ::testing::Types<FlightSQLODBCMockTestBase, FlightSQLODBCRemoteTestBase>;
+TYPED_TEST_SUITE(ConnectionAttributeTest, TestTypes);
+
+#ifdef SQL_ATTR_ASYNC_DBC_EVENT
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAsyncDbcEventUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_EVENT, 0, 0));
+ // Driver Manager on Windows returns error code HY118
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY118);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_ENABLE
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncEnableUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_ENABLE, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_DBC_PCALLBACK
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncDbcPcCallbackUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCALLBACK, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+#ifdef SQL_ATTR_ASYNC_DBC_PCONTEXT
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAyncDbcPcContextUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCONTEXT, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAutoIpdReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_AUTO_IPD, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrConnectionDeadReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_CONNECTION_DEAD, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+#ifdef SQL_ATTR_DBC_INFO_TOKEN
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrDbcInfoTokenUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrEnlistInDtcUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_ENLIST_IN_DTC, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrOdbcCursorsDMOnly) {
+ this->AllocEnvConnHandles();
+
+ // Verify DM-only attribute is settable via Driver Manager
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS,
+ reinterpret_cast<SQLPOINTER>(SQL_CUR_USE_DRIVER), 0));
+
+ std::string connect_str = this->GetConnectionString();
+ this->ConnectWithString(connect_str);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrQuietModeReadOnly) {
+ // Verify read-only attribute cannot be set
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_QUIET_MODE, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTraceDMOnly) {
+ // Verify DM-only attribute is settable via Driver Manager
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_TRACE,
+ reinterpret_cast<SQLPOINTER>(SQL_OPT_TRACE_OFF), 0));
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTracefileDMOnly) {
+ // Verify DM-only attribute is handled by Driver Manager
+
+ // Use placeholder value as we want the call to fail, or else
+ // the driver manager will produce a trace file.
+ std::wstring trace_file = L"invalid/file/path";
+ std::vector<SQLWCHAR> trace_file0(trace_file.begin(), trace_file.end());
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRACEFILE, &trace_file0[0],
+ static_cast<SQLINTEGER>(trace_file0.size())));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY000);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTranslateLabDMOnly) {
+ // Verify DM-only attribute is handled by Driver Manager
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_LIB, 0, 0));
+ // Checks for invalid argument return error
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY024);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTranslateOptionUnsupported) {
+ ASSERT_EQ(SQL_ERROR, SQLSetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_OPTION, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTxnIsolationUnsupported) {
+ ASSERT_EQ(SQL_ERROR,
+ SQLSetConnectAttr(this->conn, SQL_ATTR_TXN_ISOLATION,
+ reinterpret_cast<SQLPOINTER>(SQL_TXN_READ_UNCOMMITTED), 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
+}
+
+#ifdef SQL_ATTR_DBC_INFO_TOKEN
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrDbcInfoTokenSetOnly) {
+ // Verify that set-only attribute cannot be read
+ SQLPOINTER ptr = NULL;
+ ASSERT_EQ(SQL_ERROR, SQLGetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, ptr, 0, 0));
+ VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
+}
+#endif
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrOdbcCursorsDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLULEN cursor_attr;
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS, &cursor_attr, 0, 0));
+ EXPECT_EQ(SQL_CUR_USE_DRIVER, cursor_attr);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTraceDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLUINTEGER trace;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_TRACE, &trace, 0, 0));
+ EXPECT_EQ(SQL_OPT_TRACE_OFF, trace);
+}
+
+TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTraceFileDMOnly) {
+ // Verify that DM-only attribute is handled by driver manager
+ SQLWCHAR out_str[kOdbcBufferSize];
+ SQLINTEGER out_str_len;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_TRACEFILE, out_str,
+ kOdbcBufferSize, &out_str_len));
+ // Length is returned in bytes for SQLGetConnectAttr,
+ // we want the number of characters
+ out_str_len /= arrow::flight::sql::odbc::GetSqlWCharSize();
+ std::string out_connection_string =
+ ODBC::SqlWcharToString(out_str, static_cast<SQLSMALLINT>(out_str_len));
+ EXPECT_TRUE(!out_connection_string.empty());
Review Comment:
nit: EXPECT_FALSE
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org