-
Notifications
You must be signed in to change notification settings - Fork 4.1k
GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver #40939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 16 commits
fbb59e1
f795885
b251d50
d85771f
4485696
f7d52d5
0983cb0
bf48db0
0851610
5ec1fdd
e716a08
1691897
dda37df
845131d
31c90bf
faca7ad
c788ba3
f435e15
141e900
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,9 @@ def lint_file(path): | |
|
|
||
| EXCLUSIONS = _paths('''\ | ||
| arrow/arrow-config.cmake | ||
| arrow/flight/sql/odbc/flight_sql/get_info_cache.h | ||
| arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/blocking_queue.h | ||
| arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/odbc_impl/odbc_handle.h | ||
|
Comment on lines
+80
to
+82
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need them?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CLI/C++ support is being removed (#45810) to allow usage of
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| arrow/python/iterators.h | ||
| arrow/util/hashing.h | ||
| arrow/util/macros.h | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| # 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. | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| cmake_minimum_required(VERSION 3.16) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| set(CMAKE_CXX_STANDARD 17) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| add_custom_target(arrow_flight_sql_odbc) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if(CMAKE_BUILD_TYPE STREQUAL "Release") | ||||||||||||||||||||||||||||||||||||||||||||||||
| add_compile_definitions(NDEBUG) | ||||||||||||||||||||||||||||||||||||||||||||||||
| endif() | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Add Boost dependencies. Should be pre-installed (Brew on Mac). | ||||||||||||||||||||||||||||||||||||||||||||||||
| find_package(Boost REQUIRED) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # - Gandiva has a compile-time (header-only) dependency on Boost, not runtime. | |
| # - Tests need Boost at runtime. | |
| # - S3FS and Flight benchmarks need Boost at runtime. | |
| # - arrow_testing uses boost::filesystem. So arrow_testing requires | |
| # Boost library. (boost::filesystem isn't header-only.) But if we | |
| # use arrow_testing as a static library without | |
| # using arrow::util::Process, we don't need boost::filesystem. | |
| if(ARROW_BUILD_INTEGRATION | |
| OR ARROW_BUILD_TESTS | |
| OR (ARROW_FLIGHT AND (ARROW_TESTING OR ARROW_BUILD_BENCHMARKS)) | |
| OR (ARROW_S3 AND ARROW_BUILD_BENCHMARKS) | |
| OR (ARROW_TESTING AND ARROW_BUILD_SHARED)) | |
| set(ARROW_USE_BOOST TRUE) | |
| set(ARROW_BOOST_REQUIRE_LIBRARY TRUE) | |
| elseif(ARROW_GANDIVA | |
| OR ARROW_TESTING | |
| OR ARROW_WITH_THRIFT | |
| OR (NOT ARROW_USE_NATIVE_INT128)) | |
| set(ARROW_USE_BOOST TRUE) | |
| set(ARROW_BOOST_REQUIRE_LIBRARY FALSE) | |
| else() | |
| set(ARROW_USE_BOOST FALSE) | |
| endif() |
BTW, do we really need Boost? For example, boost/optional.h can be replaced with std::optional in C++17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, the flightsql-odbc uses Boost algorithm and Boost copy as well, so would keep the Boost dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed find_package for Boost, updated ThirdpartyToolchain.cmake
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you do this in cpp/cmake_modules/ThirdpartyToolchain.cmake instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ThirdpartyToolchain.cmake
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,176 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 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. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmake_minimum_required(VERSION 3.16) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set(CMAKE_CXX_STANDARD 17) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set(CMAKE_POSITION_INDEPENDENT_CODE ON) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| define_option(ARROW_POSITION_INDEPENDENT_CODE | |
| "Whether to create position-independent target" ON) |
Do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, removed
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use target based CMake API (target_include_directories()) instead of traditional global CMake API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, used target_include_directories instead
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this because this is not a top-level CMakeLists.txt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, removed
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update
arrow/cpp/cmake_modules/ThirdpartyToolchain.cmake
Lines 374 to 434 in 5989387
| # ---------------------------------------------------------------------- | |
| # Some EP's require other EP's | |
| if(ARROW_WITH_OPENTELEMETRY) | |
| set(ARROW_WITH_NLOHMANN_JSON ON) | |
| set(ARROW_WITH_PROTOBUF ON) | |
| endif() | |
| if(ARROW_PARQUET) | |
| set(ARROW_WITH_RAPIDJSON ON) | |
| set(ARROW_WITH_THRIFT ON) | |
| endif() | |
| if(ARROW_WITH_THRIFT) | |
| set(ARROW_WITH_ZLIB ON) | |
| endif() | |
| if(ARROW_FLIGHT) | |
| set(ARROW_WITH_GRPC ON) | |
| endif() | |
| if(ARROW_WITH_GRPC) | |
| set(ARROW_WITH_RE2 ON) | |
| set(ARROW_WITH_ZLIB ON) | |
| endif() | |
| if(ARROW_GCS) | |
| set(ARROW_WITH_GOOGLE_CLOUD_CPP ON) | |
| set(ARROW_WITH_NLOHMANN_JSON ON) | |
| set(ARROW_WITH_ZLIB ON) | |
| endif() | |
| if(ARROW_AZURE) | |
| set(ARROW_WITH_AZURE_SDK ON) | |
| endif() | |
| if(ARROW_JSON) | |
| set(ARROW_WITH_RAPIDJSON ON) | |
| endif() | |
| if(ARROW_ORC OR ARROW_FLIGHT) | |
| set(ARROW_WITH_PROTOBUF ON) | |
| endif() | |
| if(ARROW_SUBSTRAIT) | |
| set(ARROW_WITH_PROTOBUF ON) | |
| endif() | |
| if(ARROW_S3) | |
| set(ARROW_WITH_ZLIB ON) | |
| endif() | |
| if((NOT ARROW_COMPUTE) AND (NOT ARROW_GANDIVA)) | |
| set(ARROW_WITH_UTF8PROC OFF) | |
| endif() | |
| if((NOT ARROW_COMPUTE) | |
| AND (NOT ARROW_GANDIVA) | |
| AND (NOT ARROW_WITH_GRPC)) | |
| set(ARROW_WITH_RE2 OFF) | |
| endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, done
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this because arrow_static/arrow_flight_static CMake targets do this automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, Apache Arrow C++ doesn't use CMake's standard testing mechanism for now...
Could you remove this for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use our add_arrow_lib() instead?
arrow/cpp/cmake_modules/BuildUtils.cmake
Line 190 in 5989387
| function(ADD_ARROW_LIB LIB_NAME) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have an upcoming PR#46099 that uses add_arrow_lib(arrow_flight_sql_odbc) to add an arrow library at the odbc directory. The flight_sql and odbcabstraction directories are subfolders to arrow_flight_sql_odbc, so I suggest we keep add_library for these 2 subfolders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I may misunderstand your comment... What will be focused in #46099? Will #46099 focus on replacing add_library() with add_arrow_lib()?
The
flight_sqlandodbcabstractiondirectories are subfolders toarrow_flight_sql_odbc, so I suggest we keepadd_libraryfor these 2 subfolders.
Could you explain why add_library() is better for them? (I'm not sure why "subfolders" is important here.)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target_sources(arrow_odbc_spi_impl | |
| target_sources(arrow_odbc_spi_impl PRIVATE |
According to cmake docs, target_sources need a INTERFACE, PUBLIC or PRIVATE keyword to be able to build. I feel PRIVATE might be suitable here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the keyword resolved some build issues from my local environment
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, removed
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this?
I think that they are default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this block of code
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you do this in ThirdpartyToolchain.cmake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, moved to ThirdpartyToolchain.cmake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may be able to remove this by add_arrow_lib().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #40939 (comment)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| // 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/flight_sql/accessors/binary_array_accessor.h" | ||
|
|
||
| #include <algorithm> | ||
| #include <cstdint> | ||
| #include "arrow/array.h" | ||
|
|
||
| namespace driver { | ||
| namespace flight_sql { | ||
|
|
||
| using arrow::BinaryArray; | ||
| using odbcabstraction::RowStatus; | ||
|
|
||
| namespace { | ||
|
|
||
| inline RowStatus MoveSingleCellToBinaryBuffer(ColumnBinding* binding, BinaryArray* array, | ||
| int64_t arrow_row, int64_t i, | ||
| int64_t& value_offset, | ||
| bool update_value_offset, | ||
| odbcabstraction::Diagnostics& diagnostics) { | ||
| RowStatus result = odbcabstraction::RowStatus_SUCCESS; | ||
|
|
||
| const char* value = array->Value(arrow_row).data(); | ||
| size_t size_in_bytes = array->value_length(arrow_row); | ||
|
|
||
| size_t remaining_length = static_cast<size_t>(size_in_bytes - value_offset); | ||
| size_t value_length = std::min(remaining_length, binding->buffer_length); | ||
|
|
||
| auto* byte_buffer = | ||
| static_cast<unsigned char*>(binding->buffer) + i * binding->buffer_length; | ||
| memcpy(byte_buffer, ((char*)value) + value_offset, value_length); | ||
|
|
||
| if (remaining_length > binding->buffer_length) { | ||
| result = odbcabstraction::RowStatus_SUCCESS_WITH_INFO; | ||
| diagnostics.AddTruncationWarning(); | ||
| if (update_value_offset) { | ||
| value_offset += value_length; | ||
| } | ||
| } else if (update_value_offset) { | ||
| value_offset = -1; | ||
| } | ||
|
|
||
| if (binding->strlen_buffer) { | ||
| binding->strlen_buffer[i] = static_cast<ssize_t>(remaining_length); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| template <CDataType TARGET_TYPE> | ||
| BinaryArrayFlightSqlAccessor<TARGET_TYPE>::BinaryArrayFlightSqlAccessor(Array* array) | ||
| : FlightSqlAccessor<BinaryArray, TARGET_TYPE, | ||
| BinaryArrayFlightSqlAccessor<TARGET_TYPE>>(array) {} | ||
|
|
||
| template <> | ||
| RowStatus | ||
| BinaryArrayFlightSqlAccessor<odbcabstraction::CDataType_BINARY>::MoveSingleCell_impl( | ||
| ColumnBinding* binding, int64_t arrow_row, int64_t i, int64_t& value_offset, | ||
| bool update_value_offset, odbcabstraction::Diagnostics& diagnostics) { | ||
| return MoveSingleCellToBinaryBuffer(binding, this->GetArray(), arrow_row, i, | ||
| value_offset, update_value_offset, diagnostics); | ||
| } | ||
|
|
||
| template <CDataType TARGET_TYPE> | ||
| size_t BinaryArrayFlightSqlAccessor<TARGET_TYPE>::GetCellLength_impl( | ||
| ColumnBinding* binding) const { | ||
| return binding->buffer_length; | ||
| } | ||
|
|
||
| template class BinaryArrayFlightSqlAccessor<odbcabstraction::CDataType_BINARY>; | ||
|
|
||
| } // namespace flight_sql | ||
| } // namespace driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove this and add only
"ARROW_FLIGHT_SQL_ODBC": "ON"tofeatures-maximalinstead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Kou, I'll look into your comments, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed the code