Skip to content

Commit

Permalink
flight: Don't wrap a FlightInfo in an arrow::Result<> in FromProto
Browse files Browse the repository at this point in the history
To make it more consistent with the rest of serialization code.

This reduces code size in ~4KiB.
  • Loading branch information
felipecrv committed Jul 15, 2024
1 parent e55b931 commit 1a292d6
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 26 deletions.
5 changes: 3 additions & 2 deletions cpp/src/arrow/flight/flight_internals_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ void TestRoundtrip(const std::vector<FlightType>& values,
ASSERT_OK(internal::ToProto(values[i], &pb_value));

if constexpr (std::is_same_v<FlightType, FlightInfo>) {
ASSERT_OK_AND_ASSIGN(FlightInfo value, internal::FromProto(pb_value));
EXPECT_EQ(values[i], value);
FlightInfo::Data info_data;
ASSERT_OK(internal::FromProto(pb_value, &info_data));
EXPECT_EQ(values[i], FlightInfo{std::move(info_data)});
} else if constexpr (std::is_same_v<FlightType, SchemaResult>) {
std::string data;
ASSERT_OK(internal::FromProto(pb_value, &data));
Expand Down
31 changes: 16 additions & 15 deletions cpp/src/arrow/flight/serialization_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,22 +251,21 @@ Status ToProto(const FlightDescriptor& descriptor, pb::FlightDescriptor* pb_desc

// FlightInfo

arrow::Result<FlightInfo> FromProto(const pb::FlightInfo& pb_info) {
FlightInfo::Data info;
RETURN_NOT_OK(FromProto(pb_info.flight_descriptor(), &info.descriptor));
Status FromProto(const pb::FlightInfo& pb_info, FlightInfo::Data* info) {
RETURN_NOT_OK(FromProto(pb_info.flight_descriptor(), &info->descriptor));

info.schema = pb_info.schema();
info->schema = pb_info.schema();

info.endpoints.resize(pb_info.endpoint_size());
info->endpoints.resize(pb_info.endpoint_size());
for (int i = 0; i < pb_info.endpoint_size(); ++i) {
RETURN_NOT_OK(FromProto(pb_info.endpoint(i), &info.endpoints[i]));
RETURN_NOT_OK(FromProto(pb_info.endpoint(i), &info->endpoints[i]));
}

info.total_records = pb_info.total_records();
info.total_bytes = pb_info.total_bytes();
info.ordered = pb_info.ordered();
info.app_metadata = pb_info.app_metadata();
return FlightInfo(std::move(info));
info->total_records = pb_info.total_records();
info->total_bytes = pb_info.total_bytes();
info->ordered = pb_info.ordered();
info->app_metadata = pb_info.app_metadata();
return Status::OK();
}

Status FromProto(const pb::BasicAuth& pb_basic_auth, BasicAuth* basic_auth) {
Expand Down Expand Up @@ -315,8 +314,9 @@ Status ToProto(const FlightInfo& info, pb::FlightInfo* pb_info) {

Status FromProto(const pb::PollInfo& pb_info, PollInfo* info) {
if (pb_info.has_info()) {
ARROW_ASSIGN_OR_RAISE(auto flight_info, FromProto(pb_info.info()));
info->info = std::make_unique<FlightInfo>(std::move(flight_info));
FlightInfo::Data info_data;
RETURN_NOT_OK(FromProto(pb_info.info(), &info_data));
info->info = std::make_unique<FlightInfo>(std::move(info_data));
}
if (pb_info.has_flight_descriptor()) {
FlightDescriptor descriptor;
Expand Down Expand Up @@ -360,8 +360,9 @@ Status ToProto(const PollInfo& info, pb::PollInfo* pb_info) {

Status FromProto(const pb::CancelFlightInfoRequest& pb_request,
CancelFlightInfoRequest* request) {
ARROW_ASSIGN_OR_RAISE(FlightInfo info, FromProto(pb_request.info()));
request->info = std::make_unique<FlightInfo>(std::move(info));
FlightInfo::Data info_data;
RETURN_NOT_OK(FromProto(pb_request.info(), &info_data));
request->info = std::make_unique<FlightInfo>(std::move(info_data));
return Status::OK();
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/serialization_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Status FromProto(const pb::FlightDescriptor& pb_descr, FlightDescriptor* descr);
Status FromProto(const pb::FlightEndpoint& pb_endpoint, FlightEndpoint* endpoint);
Status FromProto(const pb::RenewFlightEndpointRequest& pb_request,
RenewFlightEndpointRequest* request);
arrow::Result<FlightInfo> FromProto(const pb::FlightInfo& pb_info);
Status FromProto(const pb::FlightInfo& pb_info, FlightInfo::Data* info);
Status FromProto(const pb::PollInfo& pb_info, PollInfo* info);
Status FromProto(const pb::CancelFlightInfoRequest& pb_request,
CancelFlightInfoRequest* request);
Expand Down
12 changes: 7 additions & 5 deletions cpp/src/arrow/flight/transport/grpc/grpc_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -648,10 +648,10 @@ class UnaryUnaryAsyncCall : public ::grpc::ClientUnaryReactor, public internal::

void OnDone(const ::grpc::Status& status) override {
if (status.ok()) {
auto result = internal::FromProto(pb_response);
client_status = result.status();
FlightInfo::Data info_data;
client_status = internal::FromProto(pb_response, &info_data);
if (client_status.ok()) {
listener->OnNext(std::move(result).MoveValueUnsafe());
listener->OnNext(FlightInfo{std::move(info_data)});
}
}
Finish(status);
Expand Down Expand Up @@ -889,7 +889,8 @@ class GrpcClientImpl : public internal::ClientTransport {

pb::FlightInfo pb_info;
while (!options.stop_token.IsStopRequested() && stream->Read(&pb_info)) {
ARROW_ASSIGN_OR_RAISE(FlightInfo info_data, internal::FromProto(pb_info));
FlightInfo::Data info_data;
RETURN_NOT_OK(internal::FromProto(pb_info, &info_data));
flights.emplace_back(std::move(info_data));
}
if (options.stop_token.IsStopRequested()) rpc.context.TryCancel();
Expand Down Expand Up @@ -939,7 +940,8 @@ class GrpcClientImpl : public internal::ClientTransport {
stub_->GetFlightInfo(&rpc.context, pb_descriptor, &pb_response), &rpc.context);
RETURN_NOT_OK(s);

ARROW_ASSIGN_OR_RAISE(auto info_data, internal::FromProto(pb_response));
FlightInfo::Data info_data;
RETURN_NOT_OK(internal::FromProto(pb_response, &info_data));
*info = std::make_unique<FlightInfo>(std::move(info_data));
return Status::OK();
}
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/arrow/flight/types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ arrow::Result<FlightInfo> FlightInfo::Make(const Schema& schema,
data.ordered = ordered;
data.app_metadata = std::move(app_metadata);
RETURN_NOT_OK(internal::SchemaToString(schema, &data.schema));
return FlightInfo(data);
return FlightInfo(std::move(data));
}

arrow::Result<std::shared_ptr<Schema>> FlightInfo::GetSchema(
Expand All @@ -300,8 +300,9 @@ arrow::Result<std::unique_ptr<FlightInfo>> FlightInfo::Deserialize(
std::string_view serialized) {
pb::FlightInfo pb_info;
RETURN_NOT_OK(ParseFromString("FlightInfo", serialized, &pb_info));
ARROW_ASSIGN_OR_RAISE(FlightInfo info, internal::FromProto(pb_info));
return std::make_unique<FlightInfo>(std::move(info));
FlightInfo::Data info_data;
RETURN_NOT_OK(internal::FromProto(pb_info, &info_data));
return std::make_unique<FlightInfo>(std::move(info_data));
}

std::string FlightInfo::ToString() const {
Expand Down

0 comments on commit 1a292d6

Please sign in to comment.