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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
