Skip to content

Commit

Permalink
GH-41141: [C++] Reduce string inlining in Substrait serde (#45174)
Browse files Browse the repository at this point in the history
### Rationale for this change

This patch adds a helper function to create Status instances with similarly formatted messages in order to reduce the string literal bloat in the binary.

### What changes are included in this PR?

Helper function UserDefinedLiteralToArrow::FailedToUnpack. Also added ARROW_PREDICT_FALSE to conditions before calling the function.

### Are these changes tested?

No. Normally there are no tests to verify specific Status messages.

### Are there any user-facing changes?

No.

* GitHub Issue: #41141

Authored-by: Attila Jeges <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
attilajeges authored Jan 6, 2025
1 parent b4b46e3 commit 1938582
Showing 1 changed file with 17 additions and 21 deletions.
38 changes: 17 additions & 21 deletions cpp/src/arrow/engine/substrait/expression_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -434,71 +434,67 @@ struct UserDefinedLiteralToArrow {
}
Status Visit(const IntegerType& type) {
google::protobuf::UInt64Value value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined integer literal to UInt64Value");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("integer", "UInt64Value");
}
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
return Status::OK();
}
Status Visit(const Time32Type& type) {
google::protobuf::Int32Value value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined time32 literal to Int32Value");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("time32", "Int32Value");
}
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
return Status::OK();
}
Status Visit(const Time64Type& type) {
google::protobuf::Int64Value value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined time64 literal to Int64Value");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("time64", "Int64Value");
}
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
return Status::OK();
}
Status Visit(const Date64Type& type) {
google::protobuf::Int64Value value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined date64 literal to Int64Value");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("date64", "Int64Value");
}
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
return Status::OK();
}
Status Visit(const HalfFloatType& type) {
google::protobuf::UInt32Value value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined half_float literal to UInt32Value");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("half_float", "UInt32Value");
}
uint16_t half_float_value = value.value();
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), half_float_value));
return Status::OK();
}
Status Visit(const LargeStringType& type) {
google::protobuf::StringValue value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined large_string literal to StringValue");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("large_string", "StringValue");
}
ARROW_ASSIGN_OR_RAISE(scalar_,
MakeScalar(type.GetSharedPtr(), std::string(value.value())));
return Status::OK();
}
Status Visit(const LargeBinaryType& type) {
google::protobuf::BytesValue value;
if (!user_defined_->value().UnpackTo(&value)) {
return Status::Invalid(
"Failed to unpack user defined large_binary literal to BytesValue");
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
return FailedToUnpack("large_binary", "BytesValue");
}
ARROW_ASSIGN_OR_RAISE(scalar_,
MakeScalar(type.GetSharedPtr(), std::string(value.value())));
return Status::OK();
}
Status operator()(const DataType& type) { return VisitTypeInline(type, this); }
Status FailedToUnpack(const char* from, const char* to) {
return Status::Invalid("Failed to unpack user defined ", from, " literal to ", to);
}

std::shared_ptr<Scalar> scalar_;
const substrait::Expression::Literal::UserDefined* user_defined_;
Expand Down

0 comments on commit 1938582

Please sign in to comment.