(arrow) branch main updated: GH-41141: [C++] Reduce string inlining in Substrait serde (#45174)

[ko...@apache.org]

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 1938582783 GH-41141: [C++] Reduce string inlining in Substrait serde (#45174)
1938582783 is described below

commit 1938582783da9975e9272e6cfcff17c01bf71d68
Author: Jeges Attila <at...@gmail.com>
AuthorDate: Mon Jan 6 04:09:41 2025 +0100

    GH-41141: [C++] Reduce string inlining in Substrait serde (#45174)
    
    ### 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 <at...@gmail.com>
    Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
 .../arrow/engine/substrait/expression_internal.cc  | 38 ++++++++++------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/cpp/src/arrow/engine/substrait/expression_internal.cc b/cpp/src/arrow/engine/substrait/expression_internal.cc
index 4cc93589cd..16dcba8944 100644
--- a/cpp/src/arrow/engine/substrait/expression_internal.cc
+++ b/cpp/src/arrow/engine/substrait/expression_internal.cc
@@ -434,45 +434,40 @@ 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));
@@ -480,9 +475,8 @@ struct UserDefinedLiteralToArrow {
   }
   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())));
@@ -490,15 +484,17 @@ struct UserDefinedLiteralToArrow {
   }
   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_;