Skip to content

Commit 1938582

Browse files
authored
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 <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent b4b46e3 commit 1938582

File tree

1 file changed

+17
-21
lines changed

1 file changed

+17
-21
lines changed

cpp/src/arrow/engine/substrait/expression_internal.cc

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -434,71 +434,67 @@ struct UserDefinedLiteralToArrow {
434434
}
435435
Status Visit(const IntegerType& type) {
436436
google::protobuf::UInt64Value value;
437-
if (!user_defined_->value().UnpackTo(&value)) {
438-
return Status::Invalid(
439-
"Failed to unpack user defined integer literal to UInt64Value");
437+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
438+
return FailedToUnpack("integer", "UInt64Value");
440439
}
441440
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
442441
return Status::OK();
443442
}
444443
Status Visit(const Time32Type& type) {
445444
google::protobuf::Int32Value value;
446-
if (!user_defined_->value().UnpackTo(&value)) {
447-
return Status::Invalid(
448-
"Failed to unpack user defined time32 literal to Int32Value");
445+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
446+
return FailedToUnpack("time32", "Int32Value");
449447
}
450448
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
451449
return Status::OK();
452450
}
453451
Status Visit(const Time64Type& type) {
454452
google::protobuf::Int64Value value;
455-
if (!user_defined_->value().UnpackTo(&value)) {
456-
return Status::Invalid(
457-
"Failed to unpack user defined time64 literal to Int64Value");
453+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
454+
return FailedToUnpack("time64", "Int64Value");
458455
}
459456
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
460457
return Status::OK();
461458
}
462459
Status Visit(const Date64Type& type) {
463460
google::protobuf::Int64Value value;
464-
if (!user_defined_->value().UnpackTo(&value)) {
465-
return Status::Invalid(
466-
"Failed to unpack user defined date64 literal to Int64Value");
461+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
462+
return FailedToUnpack("date64", "Int64Value");
467463
}
468464
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), value.value()));
469465
return Status::OK();
470466
}
471467
Status Visit(const HalfFloatType& type) {
472468
google::protobuf::UInt32Value value;
473-
if (!user_defined_->value().UnpackTo(&value)) {
474-
return Status::Invalid(
475-
"Failed to unpack user defined half_float literal to UInt32Value");
469+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
470+
return FailedToUnpack("half_float", "UInt32Value");
476471
}
477472
uint16_t half_float_value = value.value();
478473
ARROW_ASSIGN_OR_RAISE(scalar_, MakeScalar(type.GetSharedPtr(), half_float_value));
479474
return Status::OK();
480475
}
481476
Status Visit(const LargeStringType& type) {
482477
google::protobuf::StringValue value;
483-
if (!user_defined_->value().UnpackTo(&value)) {
484-
return Status::Invalid(
485-
"Failed to unpack user defined large_string literal to StringValue");
478+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
479+
return FailedToUnpack("large_string", "StringValue");
486480
}
487481
ARROW_ASSIGN_OR_RAISE(scalar_,
488482
MakeScalar(type.GetSharedPtr(), std::string(value.value())));
489483
return Status::OK();
490484
}
491485
Status Visit(const LargeBinaryType& type) {
492486
google::protobuf::BytesValue value;
493-
if (!user_defined_->value().UnpackTo(&value)) {
494-
return Status::Invalid(
495-
"Failed to unpack user defined large_binary literal to BytesValue");
487+
if (ARROW_PREDICT_FALSE(!user_defined_->value().UnpackTo(&value))) {
488+
return FailedToUnpack("large_binary", "BytesValue");
496489
}
497490
ARROW_ASSIGN_OR_RAISE(scalar_,
498491
MakeScalar(type.GetSharedPtr(), std::string(value.value())));
499492
return Status::OK();
500493
}
501494
Status operator()(const DataType& type) { return VisitTypeInline(type, this); }
495+
Status FailedToUnpack(const char* from, const char* to) {
496+
return Status::Invalid("Failed to unpack user defined ", from, " literal to ", to);
497+
}
502498

503499
std::shared_ptr<Scalar> scalar_;
504500
const substrait::Expression::Literal::UserDefined* user_defined_;

0 commit comments

Comments
 (0)