You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2023/06/08 07:39:22 UTC
[arrow] branch main updated: GH-35961: [C++][FlightSQL] Accept Protobuf 3.12.0 or later (#35962)
This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new a2e1f6648f GH-35961: [C++][FlightSQL] Accept Protobuf 3.12.0 or later (#35962)
a2e1f6648f is described below
commit a2e1f6648f807e00fa144094222061c1b3bcb6e1
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Thu Jun 8 16:39:12 2023 +0900
GH-35961: [C++][FlightSQL] Accept Protobuf 3.12.0 or later (#35962)
### Rationale for this change
We can use `optional` with Protobuf 3.12.0 by specifying `--experimental_allow_proto3_optional` explicitly.
If we accept Protobuf 3.12.0 or later for Flight SQL, we can use system Protobuf on Ubuntu 22.04. Because Ubuntu 22.04 ships Protobuf 3.12.4.
### What changes are included in this PR?
Specify `--experimental_allow_proto3_optional` explicitly when Protobuf is < 3.15.0.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #35961
Authored-by: Sutou Kouhei <ko...@clear-code.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
ci/docker/debian-11-cpp.dockerfile | 3 ++-
ci/docker/ubuntu-22.04-cpp.dockerfile | 5 -----
cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 ++++--
cpp/src/arrow/flight/sql/CMakeLists.txt | 12 +++++++++---
cpp/src/arrow/flight/sql/client.cc | 16 ++++++++++++++++
cpp/src/arrow/flight/sql/server.cc | 8 ++++++++
dev/tasks/linux-packages/apache-arrow/Rakefile | 16 +++-------------
7 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/ci/docker/debian-11-cpp.dockerfile b/ci/docker/debian-11-cpp.dockerfile
index e6ac9e6071..00adc6bd6b 100644
--- a/ci/docker/debian-11-cpp.dockerfile
+++ b/ci/docker/debian-11-cpp.dockerfile
@@ -65,6 +65,8 @@ RUN apt-get update -y -q && \
libgoogle-glog-dev \
libgrpc++-dev \
liblz4-dev \
+ libprotobuf-dev \
+ libprotoc-dev \
libre2-dev \
libsnappy-dev \
libssl-dev \
@@ -121,5 +123,4 @@ ENV absl_SOURCE=BUNDLED \
GTest_SOURCE=BUNDLED \
ORC_SOURCE=BUNDLED \
PATH=/usr/lib/ccache/:$PATH \
- Protobuf_SOURCE=BUNDLED \
xsimd_SOURCE=BUNDLED
diff --git a/ci/docker/ubuntu-22.04-cpp.dockerfile b/ci/docker/ubuntu-22.04-cpp.dockerfile
index 03a6bb9c72..e6fd44ff2d 100644
--- a/ci/docker/ubuntu-22.04-cpp.dockerfile
+++ b/ci/docker/ubuntu-22.04-cpp.dockerfile
@@ -160,11 +160,7 @@ RUN /arrow/ci/scripts/install_sccache.sh unknown-linux-musl /usr/local/bin
# provided by the distribution:
# - Abseil is old
# - libc-ares-dev does not install CMake config files
-# - flatbuffer is not packaged
# - libgtest-dev only provide sources
-# - libprotobuf-dev only provide sources
-# ARROW-17051: this build uses static Protobuf, so we must also use
-# static Arrow to run Flight/Flight SQL tests
ENV absl_SOURCE=BUNDLED \
ARROW_ACERO=ON \
ARROW_BUILD_STATIC=ON \
@@ -199,6 +195,5 @@ ENV absl_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
PATH=/usr/lib/ccache/:$PATH \
- Protobuf_SOURCE=BUNDLED \
PYTHON=python3 \
xsimd_SOURCE=BUNDLED
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index d0398d2d0d..9e8ecb5ceb 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -1758,8 +1758,10 @@ endmacro()
if(ARROW_WITH_PROTOBUF)
if(ARROW_FLIGHT_SQL)
- # Flight SQL uses proto3 optionals, which require 3.15 or later.
- set(ARROW_PROTOBUF_REQUIRED_VERSION "3.15.0")
+ # Flight SQL uses proto3 optionals, which require 3.12 or later.
+ # 3.12.0-3.14.0: need --experimental_allow_proto3_optional
+ # 3.15.0-: don't need --experimental_allow_proto3_optional
+ set(ARROW_PROTOBUF_REQUIRED_VERSION "3.12.0")
elseif(ARROW_SUBSTRAIT)
# Substrait protobuf files use proto3 syntax
set(ARROW_PROTOBUF_REQUIRED_VERSION "3.0.0")
diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt
index 628b02b9d2..0f12dbfdf9 100644
--- a/cpp/src/arrow/flight/sql/CMakeLists.txt
+++ b/cpp/src/arrow/flight/sql/CMakeLists.txt
@@ -27,10 +27,16 @@ set(FLIGHT_SQL_GENERATED_PROTO_FILES "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.c
set(PROTO_DEPENDS ${FLIGHT_SQL_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF})
+set(FLIGHT_SQL_PROTOC_COMMAND
+ ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}"
+ "--cpp_out=dllexport_decl=ARROW_FLIGHT_SQL_EXPORT:${CMAKE_CURRENT_BINARY_DIR}")
+if(Protobuf_VERSION VERSION_LESS 3.15)
+ list(APPEND FLIGHT_SQL_PROTOC_COMMAND "--experimental_allow_proto3_optional")
+endif()
+list(APPEND FLIGHT_SQL_PROTOC_COMMAND "${FLIGHT_SQL_PROTO}")
+
add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES}
- COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}"
- "--cpp_out=dllexport_decl=ARROW_FLIGHT_SQL_EXPORT:${CMAKE_CURRENT_BINARY_DIR}"
- "${FLIGHT_SQL_PROTO}"
+ COMMAND ${FLIGHT_SQL_PROTOC_COMMAND}
DEPENDS ${PROTO_DEPENDS})
set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE)
diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc
index 25bf8e384e..b0d77563bc 100644
--- a/cpp/src/arrow/flight/sql/client.cc
+++ b/cpp/src/arrow/flight/sql/client.cc
@@ -41,14 +41,22 @@ namespace {
arrow::Result<FlightDescriptor> GetFlightDescriptorForCommand(
const google::protobuf::Message& command) {
google::protobuf::Any any;
+#if PROTOBUF_VERSION >= 3015000
if (!any.PackFrom(command)) {
return Status::SerializationError("Failed to pack ", command.GetTypeName());
}
+#else
+ any.PackFrom(command);
+#endif
std::string buf;
+#if PROTOBUF_VERSION >= 3015000
if (!any.SerializeToString(&buf)) {
return Status::SerializationError("Failed to serialize ", command.GetTypeName());
}
+#else
+ any.SerializeToString(&buf);
+#endif
return FlightDescriptor::Command(buf);
}
@@ -71,16 +79,24 @@ arrow::Result<std::unique_ptr<SchemaResult>> GetSchemaForCommand(
::arrow::Result<Action> PackAction(const std::string& action_type,
const google::protobuf::Message& message) {
google::protobuf::Any any;
+#if PROTOBUF_VERSION >= 3015000
if (!any.PackFrom(message)) {
return Status::SerializationError("Could not pack ", message.GetTypeName(),
" into Any");
}
+#else
+ any.PackFrom(message);
+#endif
std::string buffer;
+#if PROTOBUF_VERSION >= 3015000
if (!any.SerializeToString(&buffer)) {
return Status::SerializationError("Could not serialize packed ",
message.GetTypeName());
}
+#else
+ any.SerializeToString(&buffer);
+#endif
Action action;
action.type = action_type;
diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc
index 7621711308..3975a01353 100644
--- a/cpp/src/arrow/flight/sql/server.cc
+++ b/cpp/src/arrow/flight/sql/server.cc
@@ -361,14 +361,22 @@ arrow::Result<ActionEndTransactionRequest> ParseActionEndTransactionRequest(
arrow::Result<Result> PackActionResult(const google::protobuf::Message& message) {
google::protobuf::Any any;
+#if PROTOBUF_VERSION >= 3015000
if (!any.PackFrom(message)) {
return Status::IOError("Failed to pack ", message.GetTypeName());
}
+#else
+ any.PackFrom(message);
+#endif
std::string buffer;
+#if PROTOBUF_VERSION >= 3015000
if (!any.SerializeToString(&buffer)) {
return Status::IOError("Failed to serialize packed ", message.GetTypeName());
}
+#else
+ any.SerializeToString(&buffer);
+#endif
return Result{Buffer::FromString(std::move(buffer))};
}
diff --git a/dev/tasks/linux-packages/apache-arrow/Rakefile b/dev/tasks/linux-packages/apache-arrow/Rakefile
index 962d34340a..cdc6d2cf35 100644
--- a/dev/tasks/linux-packages/apache-arrow/Rakefile
+++ b/dev/tasks/linux-packages/apache-arrow/Rakefile
@@ -107,21 +107,11 @@ class ApacheArrowPackageTask < PackageTask
end
def apt_prepare_debian_control_protobuf(control, target)
- # Flight requires Protobuf 3.15.0 or later but Ubuntu 22.04
- # doesn't provide Protobuf 3.15.0 or later yet.
- #
- # See also:
- # * cpp/cmake_modules/ThirdpartyToolchain.cmake
- # * https://packages.debian.org/search?keywords=libprotobuf-dev
- # * https://packages.ubuntu.com/search?keywords=libprotobuf-dev
- #
- # We can use system Protobuf without Flight because we can use
- # Protobuf 3.0.0 or later without Flight.
case target
- when /\Adebian-bookworm/
- use_system_protobuf = ""
- else
+ when /\Aubuntu-focal/
use_system_protobuf = "#"
+ else
+ use_system_protobuf = ""
end
control.gsub(/@USE_SYSTEM_PROTOBUF@/, use_system_protobuf)
end