[GitHub] [arrow] lidavidm commented on a diff in pull request #13986: ARROW-17052: [C++][Python][FlightRPC] expose flight structures serialize

[GitBox <gi...@apache.org>]

lidavidm commented on code in PR #13986:
URL: https://github.com/apache/arrow/pull/13986#discussion_r961990778


##########
cpp/src/arrow/flight/flight_internals_test.cc:
##########
@@ -83,6 +83,13 @@ TEST(FlightTypes, LocationUnknownScheme) {
 }
 
 TEST(FlightTypes, RoundTripTypes) {
+  Action action{"action1", Buffer::FromString("action1-content")};
+  ASSERT_OK_AND_ASSIGN(std::string action_serialized, action.SerializeToString());
+  ASSERT_OK_AND_ASSIGN(Action action_deserialized,
+                       Action::Deserialize(action_serialized));
+  ASSERT_EQ(action.type, action_deserialized.type);
+  ASSERT_TRUE(action.body->Equals(*action_deserialized.body));
+

Review Comment:
   The tests should be expanded to cover each of the new types (though I see this is a draft)



##########
cpp/src/arrow/flight/types.cc:
##########
@@ -162,6 +162,33 @@ Status SchemaResult::GetSchema(ipc::DictionaryMemo* dictionary_memo,
   return GetSchema(dictionary_memo).Value(out);
 }
 
+arrow::Result<std::string> SchemaResult::SerializeToString() const {
+  pb::SchemaResult pb_schema_result;
+  RETURN_NOT_OK(internal::ToProto(*this, &pb_schema_result));
+
+  std::string out;
+  if (!pb_schema_result.SerializeToString(&out)) {
+    return Status::IOError("Serialized schema-result exceeded 2 GiB limit");

Review Comment:
   nit 
   ```suggestion
       return Status::IOError("Serialized SchemaResult exceeded 2 GiB limit");
   ```
   
   & ditto below



##########
cpp/src/arrow/flight/types.cc:
##########
@@ -370,10 +397,141 @@ bool FlightEndpoint::Equals(const FlightEndpoint& other) const {
   return ticket == other.ticket && locations == other.locations;
 }
 
+arrow::Result<std::string> FlightEndpoint::SerializeToString() const {
+  pb::FlightEndpoint pb_flight_endpoint;
+  RETURN_NOT_OK(internal::ToProto(*this, &pb_flight_endpoint));

Review Comment:
   This doesn't build either (the function is defined, but isn't declared in the header)



##########
cpp/src/arrow/flight/types.cc:
##########
@@ -162,6 +162,33 @@ Status SchemaResult::GetSchema(ipc::DictionaryMemo* dictionary_memo,
   return GetSchema(dictionary_memo).Value(out);
 }
 
+arrow::Result<std::string> SchemaResult::SerializeToString() const {
+  pb::SchemaResult pb_schema_result;
+  RETURN_NOT_OK(internal::ToProto(*this, &pb_schema_result));
+
+  std::string out;
+  if (!pb_schema_result.SerializeToString(&out)) {
+    return Status::IOError("Serialized schema-result exceeded 2 GiB limit");
+  }
+  return out;
+}
+
+arrow::Result<SchemaResult> SchemaResult::Deserialize(
+    arrow::util::string_view serialized) {
+  pb::SchemaResult pb_schema_result;
+  if (serialized.size() > static_cast<size_t>(std::numeric_limits<int>::max())) {
+    return Status::Invalid("Serialized SchemaResult size should not exceed 2 GiB");
+  }
+  google::protobuf::io::ArrayInputStream input(serialized.data(),
+                                               static_cast<int>(serialized.size()));
+  if (!pb_schema_result.ParseFromZeroCopyStream(&input)) {
+    return Status::Invalid("Not a valid schema-result");
+  }
+  SchemaResult out;
+  RETURN_NOT_OK(internal::FromProto(pb_schema_result, &out));

Review Comment:
   This doesn't actually build
   
   ```
    /arrow/cpp/src/arrow/flight/types.cc:188:59: error: no matching function for call to 'FromProto(arrow::flight::protocol::SchemaResult&, arrow::flight::SchemaResult*)'
     188 |   RETURN_NOT_OK(internal::FromProto(pb_schema_result, &out));
         |
   ...
   // This is the correct overload
   /arrow/cpp/src/arrow/flight/serialization_internal.h:59:8: note: candidate: 'arrow::Status arrow::flight::internal::FromProto(const arrow::flight::protocol::SchemaResult&, std::string*)'
      59 | Status FromProto(const pb::SchemaResult& pb_result, std::string* result);
         |        ^~~~~~~~~
   ```



-- 
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