[GitHub] [arrow] cyb70289 commented on a diff in pull request #13286: ARROW-16711: [C++] Remove deprecated ORC APIs

[GitBox <gi...@apache.org>]

cyb70289 commented on code in PR #13286:
URL: https://github.com/apache/arrow/pull/13286#discussion_r887521932


##########
cpp/src/arrow/adapters/orc/util.cc:
##########
@@ -945,123 +945,98 @@ Status WriteBatch(const ChunkedArray& chunked_array, int64_t length,
   return Status::OK();
 }
 
-Status GetArrowType(const liborc::Type* type, std::shared_ptr<DataType>* out) {
+Result<std::shared_ptr<DataType>> GetArrowType(const liborc::Type* type) {
   // When subselecting fields on read, liborc will set some nodes to nullptr,
   // so we need to check for nullptr before progressing
   if (type == nullptr) {
-    *out = null();
-    return Status::OK();
+    return null();
   }
   liborc::TypeKind kind = type->getKind();
   const int subtype_count = static_cast<int>(type->getSubtypeCount());
 
   switch (kind) {
     case liborc::BOOLEAN:
-      *out = boolean();
-      break;
+      return boolean();
     case liborc::BYTE:
-      *out = int8();
-      break;
+      return int8();
     case liborc::SHORT:
-      *out = int16();
-      break;
+      return int16();
     case liborc::INT:
-      *out = int32();
-      break;
+      return int32();
     case liborc::LONG:
-      *out = int64();
-      break;
+      return int64();
     case liborc::FLOAT:
-      *out = float32();
-      break;
+      return float32();
     case liborc::DOUBLE:
-      *out = float64();
-      break;
+      return float64();
     case liborc::VARCHAR:
     case liborc::STRING:
-      *out = utf8();
-      break;
+      return utf8();
     case liborc::BINARY:
-      *out = binary();
-      break;
+      return binary();
     case liborc::CHAR:
-      *out = fixed_size_binary(static_cast<int>(type->getMaximumLength()));
-      break;
+      return fixed_size_binary(static_cast<int>(type->getMaximumLength()));
     case liborc::TIMESTAMP:
-      *out = timestamp(TimeUnit::NANO);
-      break;
+      return timestamp(TimeUnit::NANO);
     case liborc::DATE:
-      *out = date32();
-      break;
+      return date32();
     case liborc::DECIMAL: {
       const int precision = static_cast<int>(type->getPrecision());
       const int scale = static_cast<int>(type->getScale());
       if (precision == 0) {
         // In HIVE 0.11/0.12 precision is set as 0, but means max precision
-        *out = decimal128(38, 6);
+        return decimal128(38, 6);
       } else {
-        *out = decimal128(precision, scale);
+        return decimal128(precision, scale);
       }
       break;
     }
     case liborc::LIST: {
       if (subtype_count != 1) {
         return Status::TypeError("Invalid Orc List type");
       }
-      std::shared_ptr<DataType> elemtype;
-      RETURN_NOT_OK(GetArrowType(type->getSubtype(0), &elemtype));
-      *out = list(elemtype);
-      break;
+      ARROW_ASSIGN_OR_RAISE(auto elemtype, GetArrowType(type->getSubtype(0)));
+      return list(elemtype);
     }
     case liborc::MAP: {
       if (subtype_count != 2) {
         return Status::TypeError("Invalid Orc Map type");
       }
-      std::shared_ptr<DataType> key_type, item_type;
-      RETURN_NOT_OK(GetArrowType(type->getSubtype(0), &key_type));
-      RETURN_NOT_OK(GetArrowType(type->getSubtype(1), &item_type));
-      *out = map(key_type, item_type);
-      break;
+      ARROW_ASSIGN_OR_RAISE(auto key_type, GetArrowType(type->getSubtype(0)));
+      ARROW_ASSIGN_OR_RAISE(auto item_type, GetArrowType(type->getSubtype(1)));
+      return map(key_type, item_type);

Review Comment:
   Apply `std::move` to arguments?
   https://github.com/apache/arrow/blob/master/cpp/src/arrow/type.cc#L2231
   
   `list` passes arguments by const reference, but `map` passes by value. Looks not very consistent.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org