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/11/04 21:54:35 UTC
[PR] GH-47720: [C++][FlightRPC] WIP: ODBC Columns Metadata [arrow]
alinaliBQ opened a new pull request, #48049:
URL: https://github.com/apache/arrow/pull/48049
### Rationale for this change
Add support for returning columns metadata from the connected data source.
### What changes are included in this PR?
- SQLColumns & Test
### Are these changes tested?
Will be tested in CI when PR is ready for review.
### Are there any user-facing changes?
N/A
--
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-47720: [C++][FlightRPC] ODBC Columns Metadata [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #48049:
URL: https://github.com/apache/arrow/pull/48049#issuecomment-3658418983
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d25f387fdb53895d5143deeabde14bf757c40a77.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/58152749371) has more details. It also includes information about 27 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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-47720: [C++][FlightRPC] ODBC Columns Metadata [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo merged PR #48049:
URL: https://github.com/apache/arrow/pull/48049
--
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-47720: [C++][FlightRPC] ODBC Columns Metadata [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on code in PR #48049:
URL: https://github.com/apache/arrow/pull/48049#discussion_r2611550606
##########
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc:
##########
@@ -180,9 +180,20 @@ void FlightSQLOdbcV2RemoteTestBase::SetUp() {
connected_ = true;
}
-void FlightSQLOdbcEnvConnHandleRemoteTestBase::SetUp() { AllocEnvConnHandles(); }
+void FlightSQLOdbcEnvConnHandleRemoteTestBase::SetUp() {
+ ODBCRemoteTestBase::SetUp();
+ if (skipping_test_) {
+ return;
+ }
Review Comment:
Used for checking if the environment variable `ARROW_FLIGHT_SQL_ODBC_CONN` exists. If it doesn't exist, `skipping_test_` is set to `true` and the test is skipped.
--
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-47720: [C++][FlightRPC] WIP: ODBC Columns Metadata [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48049:
URL: https://github.com/apache/arrow/pull/48049#issuecomment-3488145792
:warning: GitHub issue #47720 **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-47720: [C++][FlightRPC] WIP: ODBC Columns Metadata [arrow]
Posted by "alinaliBQ (via GitHub)" <gi...@apache.org>.
alinaliBQ commented on code in PR #48049:
URL: https://github.com/apache/arrow/pull/48049#discussion_r2492165763
##########
cpp/src/arrow/flight/sql/odbc/tests/columns_test.cc:
##########
@@ -0,0 +1,962 @@
+// 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 ColumnsTest : public T {};
+
+class ColumnsMockTest : public FlightSQLODBCMockTestBase {};
+class ColumnsRemoteTest : public FlightSQLODBCRemoteTestBase {};
+using TestTypes = ::testing::Types<ColumnsMockTest, ColumnsRemoteTest>;
+TYPED_TEST_SUITE(ColumnsTest, TestTypes);
+
+template <typename T>
+class ColumnsOdbcV2Test : public T {};
+
+class ColumnsOdbcV2MockTest : public FlightSQLOdbcV2MockTestBase {};
+class ColumnsOdbcV2RemoteTest : public FlightSQLOdbcV2RemoteTestBase {};
+using TestTypesOdbcV2 = ::testing::Types<ColumnsOdbcV2MockTest, ColumnsOdbcV2RemoteTest>;
+TYPED_TEST_SUITE(ColumnsOdbcV2Test, TestTypesOdbcV2);
+
+namespace {
+// Helper functions
+void CheckSQLColumns(
+ SQLHSTMT stmt, const std::wstring& expected_table,
+ const std::wstring& expected_column, const SQLINTEGER& expected_data_type,
+ const std::wstring& expected_type_name, const SQLINTEGER& expected_column_size,
+ const SQLINTEGER& expected_buffer_length, const SQLSMALLINT& expected_decimal_digits,
+ const SQLSMALLINT& expected_num_prec_radix, const SQLSMALLINT& expected_nullable,
+ const SQLSMALLINT& expected_sql_data_type, const SQLSMALLINT& expected_date_time_sub,
+ const SQLINTEGER& expected_octet_char_length,
+ const SQLINTEGER& expected_ordinal_position,
+ const std::wstring& expected_is_nullable) {
+ CheckStringColumnW(stmt, 3, expected_table); // table name
+ CheckStringColumnW(stmt, 4, expected_column); // column name
+
+ CheckIntColumn(stmt, 5, expected_data_type); // data type
+
+ CheckStringColumnW(stmt, 6, expected_type_name); // type name
+
+ CheckIntColumn(stmt, 7, expected_column_size); // column size
+ CheckIntColumn(stmt, 8, expected_buffer_length); // buffer length
+
+ CheckSmallIntColumn(stmt, 9, expected_decimal_digits); // decimal digits
+ CheckSmallIntColumn(stmt, 10, expected_num_prec_radix); // num prec radix
+ CheckSmallIntColumn(stmt, 11,
+ expected_nullable); // nullable
+
+ CheckNullColumnW(stmt, 12); // remarks
+ CheckNullColumnW(stmt, 13); // column def
+
+ CheckSmallIntColumn(stmt, 14, expected_sql_data_type); // sql data type
+ CheckSmallIntColumn(stmt, 15, expected_date_time_sub); // sql date type sub
+ CheckIntColumn(stmt, 16, expected_octet_char_length); // char octet length
+ CheckIntColumn(stmt, 17,
+ expected_ordinal_position); // oridinal position
+
+ CheckStringColumnW(stmt, 18, expected_is_nullable); // is nullable
+}
Review Comment:
Hi @lidavidm do you have any feedback on this type of helper functions `CheckSQLColumns` that check results from multiple columns? This type is different from the getter functions like `ValidateGreaterThan(...)`
--
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-47720: [C++][FlightRPC] WIP: ODBC Columns Metadata [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #48049:
URL: https://github.com/apache/arrow/pull/48049#discussion_r2496724320
##########
cpp/src/arrow/flight/sql/odbc/tests/columns_test.cc:
##########
@@ -0,0 +1,962 @@
+// 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 ColumnsTest : public T {};
+
+class ColumnsMockTest : public FlightSQLODBCMockTestBase {};
+class ColumnsRemoteTest : public FlightSQLODBCRemoteTestBase {};
+using TestTypes = ::testing::Types<ColumnsMockTest, ColumnsRemoteTest>;
+TYPED_TEST_SUITE(ColumnsTest, TestTypes);
+
+template <typename T>
+class ColumnsOdbcV2Test : public T {};
+
+class ColumnsOdbcV2MockTest : public FlightSQLOdbcV2MockTestBase {};
+class ColumnsOdbcV2RemoteTest : public FlightSQLOdbcV2RemoteTestBase {};
+using TestTypesOdbcV2 = ::testing::Types<ColumnsOdbcV2MockTest, ColumnsOdbcV2RemoteTest>;
+TYPED_TEST_SUITE(ColumnsOdbcV2Test, TestTypesOdbcV2);
+
+namespace {
+// Helper functions
+void CheckSQLColumns(
+ SQLHSTMT stmt, const std::wstring& expected_table,
+ const std::wstring& expected_column, const SQLINTEGER& expected_data_type,
+ const std::wstring& expected_type_name, const SQLINTEGER& expected_column_size,
+ const SQLINTEGER& expected_buffer_length, const SQLSMALLINT& expected_decimal_digits,
+ const SQLSMALLINT& expected_num_prec_radix, const SQLSMALLINT& expected_nullable,
+ const SQLSMALLINT& expected_sql_data_type, const SQLSMALLINT& expected_date_time_sub,
+ const SQLINTEGER& expected_octet_char_length,
+ const SQLINTEGER& expected_ordinal_position,
+ const std::wstring& expected_is_nullable) {
+ CheckStringColumnW(stmt, 3, expected_table); // table name
+ CheckStringColumnW(stmt, 4, expected_column); // column name
+
+ CheckIntColumn(stmt, 5, expected_data_type); // data type
+
+ CheckStringColumnW(stmt, 6, expected_type_name); // type name
+
+ CheckIntColumn(stmt, 7, expected_column_size); // column size
+ CheckIntColumn(stmt, 8, expected_buffer_length); // buffer length
+
+ CheckSmallIntColumn(stmt, 9, expected_decimal_digits); // decimal digits
+ CheckSmallIntColumn(stmt, 10, expected_num_prec_radix); // num prec radix
+ CheckSmallIntColumn(stmt, 11,
+ expected_nullable); // nullable
+
+ CheckNullColumnW(stmt, 12); // remarks
+ CheckNullColumnW(stmt, 13); // column def
+
+ CheckSmallIntColumn(stmt, 14, expected_sql_data_type); // sql data type
+ CheckSmallIntColumn(stmt, 15, expected_date_time_sub); // sql date type sub
+ CheckIntColumn(stmt, 16, expected_octet_char_length); // char octet length
+ CheckIntColumn(stmt, 17,
+ expected_ordinal_position); // oridinal position
+
+ CheckStringColumnW(stmt, 18, expected_is_nullable); // is nullable
+}
Review Comment:
I suppose this is fine given the structure of the test
--
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-47720: [C++][FlightRPC] ODBC Columns Metadata [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #48049:
URL: https://github.com/apache/arrow/pull/48049#discussion_r2611201714
##########
cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc:
##########
@@ -180,9 +180,20 @@ void FlightSQLOdbcV2RemoteTestBase::SetUp() {
connected_ = true;
}
-void FlightSQLOdbcEnvConnHandleRemoteTestBase::SetUp() { AllocEnvConnHandles(); }
+void FlightSQLOdbcEnvConnHandleRemoteTestBase::SetUp() {
+ ODBCRemoteTestBase::SetUp();
+ if (skipping_test_) {
+ return;
+ }
Review Comment:
What is this used for?
--
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