PARQUET-1053: Fix unused result warnings due to unchecked Statuses

Author: Phillip Cloud <[email protected]>

Closes apache#369 from cpcloud/PARQUET-1053 and squashes the following commits:

e0598b4 [Phillip Cloud] PARQUET-1053: Fix unused result warnings due to unchecked Statuses

Change-Id: I4e37258ede5a94049a55965777c92af05cccf276

cpp/src/parquet/arrow/arrow-reader-writer-test.cc

@@ -709,7 +709,7 @@ TEST_F(TestInt96ParquetIO, ReadIntoTimestamp) {
 
   ::arrow::TimestampBuilder builder(
       default_memory_pool(), ::arrow::timestamp(TimeUnit::NANO));
-  builder.Append(val);
+  ASSERT_OK(builder.Append(val));
   std::shared_ptr<Array> values;
   ASSERT_OK(builder.Finish(&values));
   this->ReadAndCheckSingleColumnFile(values.get());
@@ -782,7 +782,7 @@ TEST_F(TestStringParquetIO, EmptyStringColumnRequiredWrite) {
   std::shared_ptr<Array> values;
   ::arrow::StringBuilder builder(::arrow::default_memory_pool());
   for (size_t i = 0; i < SMALL_SIZE; i++) {
-    builder.Append("");
+    ASSERT_OK(builder.Append(""));
   }
   ASSERT_OK(builder.Finish(&values));
   std::shared_ptr<Table> table = MakeSimpleTable(values, false);

cpp/src/parquet/arrow/reader.cc

@@ -28,6 +28,7 @@
 
 #include "arrow/api.h"
 #include "arrow/util/bit-util.h"
+#include "arrow/util/logging.h"
 
 #include "parquet/arrow/schema.h"
 #include "parquet/util/schema-util.h"
@@ -235,7 +236,7 @@ class PARQUET_NO_EXPORT PrimitiveImpl : public ColumnReader::Impl {
         values_buffer_(pool),
         def_levels_buffer_(pool),
         rep_levels_buffer_(pool) {
-    NodeToField(input_->descr()->schema_node(), &field_);
+    DCHECK(NodeToField(input_->descr()->schema_node(), &field_).ok());
     NextRowGroup();
   }
 
@@ -1368,7 +1369,7 @@ Status StructImpl::GetDefLevels(ValueLevelsPtr* data, size_t* length) {
   size_t child_length;
   RETURN_NOT_OK(children_[0]->GetDefLevels(&child_def_levels, &child_length));
   auto size = child_length * sizeof(int16_t);
-  def_levels_buffer_.Resize(size);
+  RETURN_NOT_OK(def_levels_buffer_.Resize(size));
   // Initialize with the minimal def level
   std::memset(def_levels_buffer_.mutable_data(), -1, size);
   auto result_levels = reinterpret_cast<int16_t*>(def_levels_buffer_.mutable_data());

cpp/src/parquet/arrow/test-util.h

@@ -55,7 +55,7 @@ typename std::enable_if<is_arrow_float<ArrowType>::value, Status>::type NonNullA
   std::vector<typename ArrowType::c_type> values;
   ::arrow::test::random_real<typename ArrowType::c_type>(size, 0, 0, 1, &values);
   ::arrow::NumericBuilder<ArrowType> builder(::arrow::default_memory_pool());
-  builder.Append(values.data(), values.size());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size()));
   return builder.Finish(out);
 }
 
@@ -69,7 +69,7 @@ NonNullArray(size_t size, std::shared_ptr<Array>* out) {
   // Passing data type so this will work with TimestampType too
   ::arrow::NumericBuilder<ArrowType> builder(
       ::arrow::default_memory_pool(), std::make_shared<ArrowType>());
-  builder.Append(values.data(), values.size());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size()));
   return builder.Finish(out);
 }
 
@@ -96,7 +96,7 @@ NonNullArray(size_t size, std::shared_ptr<Array>* out) {
   using BuilderType = typename ::arrow::TypeTraits<ArrowType>::BuilderType;
   BuilderType builder(::arrow::default_memory_pool());
   for (size_t i = 0; i < size; i++) {
-    builder.Append("test-string");
+    RETURN_NOT_OK(builder.Append("test-string"));
   }
   return builder.Finish(out);
 }
@@ -109,7 +109,7 @@ NonNullArray(size_t size, std::shared_ptr<Array>* out) {
   // todo: find a way to generate test data with more diversity.
   BuilderType builder(::arrow::default_memory_pool(), ::arrow::fixed_size_binary(5));
   for (size_t i = 0; i < size; i++) {
-    builder.Append("fixed");
+    RETURN_NOT_OK(builder.Append("fixed"));
   }
   return builder.Finish(out);
 }
@@ -120,7 +120,7 @@ typename std::enable_if<is_arrow_bool<ArrowType>::value, Status>::type NonNullAr
   std::vector<uint8_t> values;
   ::arrow::test::randint<uint8_t>(size, 0, 1, &values);
   ::arrow::BooleanBuilder builder(::arrow::default_memory_pool());
-  builder.Append(values.data(), values.size());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size()));
   return builder.Finish(out);
 }
 
@@ -138,7 +138,7 @@ typename std::enable_if<is_arrow_float<ArrowType>::value, Status>::type Nullable
   }
 
   ::arrow::NumericBuilder<ArrowType> builder(::arrow::default_memory_pool());
-  builder.Append(values.data(), values.size(), valid_bytes.data());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size(), valid_bytes.data()));
   return builder.Finish(out);
 }
 
@@ -161,7 +161,7 @@ NullableArray(size_t size, size_t num_nulls, uint32_t seed, std::shared_ptr<Arra
   // Passing data type so this will work with TimestampType too
   ::arrow::NumericBuilder<ArrowType> builder(
       ::arrow::default_memory_pool(), std::make_shared<ArrowType>());
-  builder.Append(values.data(), values.size(), valid_bytes.data());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size(), valid_bytes.data()));
   return builder.Finish(out);
 }
 
@@ -208,10 +208,10 @@ NullableArray(
   uint8_t buffer[kBufferSize];
   for (size_t i = 0; i < size; i++) {
     if (!valid_bytes[i]) {
-      builder.AppendNull();
+      RETURN_NOT_OK(builder.AppendNull());
     } else {
       ::arrow::test::random_bytes(kBufferSize, seed + static_cast<uint32_t>(i), buffer);
-      builder.Append(buffer, kBufferSize);
+      RETURN_NOT_OK(builder.Append(buffer, kBufferSize));
     }
   }
   return builder.Finish(out);
@@ -238,10 +238,10 @@ NullableArray(
   uint8_t buffer[kBufferSize];
   for (size_t i = 0; i < size; i++) {
     if (!valid_bytes[i]) {
-      builder.AppendNull();
+      RETURN_NOT_OK(builder.AppendNull());
     } else {
       ::arrow::test::random_bytes(kBufferSize, seed + static_cast<uint32_t>(i), buffer);
-      builder.Append(buffer);
+      RETURN_NOT_OK(builder.Append(buffer));
     }
   }
   return builder.Finish(out);
@@ -264,7 +264,7 @@ typename std::enable_if<is_arrow_bool<ArrowType>::value, Status>::type NullableA
   }
 
   ::arrow::BooleanBuilder builder(::arrow::default_memory_pool());
-  builder.Append(values.data(), values.size(), valid_bytes.data());
+  RETURN_NOT_OK(builder.Append(values.data(), values.size(), valid_bytes.data()));
   return builder.Finish(out);
 }
 
@@ -349,7 +349,7 @@ template <>
 void ExpectArrayT<::arrow::BooleanType>(void* expected, Array* result) {
   ::arrow::BooleanBuilder builder(
       ::arrow::default_memory_pool(), std::make_shared<::arrow::BooleanType>());
-  builder.Append(reinterpret_cast<uint8_t*>(expected), result->length());
+  EXPECT_OK(builder.Append(reinterpret_cast<uint8_t*>(expected), result->length()));
 
   std::shared_ptr<Array> expected_array;
   EXPECT_OK(builder.Finish(&expected_array));

cpp/src/parquet/arrow/writer.cc

@@ -169,7 +169,7 @@ class LevelBuilder {
       *num_levels = array.length();
     } else {
       RETURN_NOT_OK(rep_levels_.Append(0));
-      HandleListEntries(0, 0, 0, array.length());
+      RETURN_NOT_OK(HandleListEntries(0, 0, 0, array.length()));
 
       std::shared_ptr<Array> def_levels_array;
       RETURN_NOT_OK(def_levels_.Finish(&def_levels_array));

cpp/src/parquet/encoding.h

@@ -20,8 +20,10 @@
 
 #include <cstdint>
 #include <memory>
+#include <sstream>
 
 #include "arrow/util/bit-util.h"
+#include "arrow/status.h"
 
 #include "parquet/exception.h"
 #include "parquet/schema.h"
@@ -49,7 +51,13 @@ class Encoder {
   virtual void PutSpaced(const T* src, int num_values, const uint8_t* valid_bits,
       int64_t valid_bits_offset) {
     PoolBuffer buffer(pool_);
-    buffer.Resize(num_values * sizeof(T));
+    ::arrow::Status status = buffer.Resize(num_values * sizeof(T));
+    if (!status.ok()) {
+      std::ostringstream ss;
+      ss << "buffer.Resize failed in Encoder.PutSpaced in " <<
+         __FILE__ << ", on line " << __LINE__;
+      throw ParquetException(ss.str());
+    }
     int32_t num_valid_values = 0;
     INIT_BITSET(valid_bits, static_cast<int>(valid_bits_offset));
     T* data = reinterpret_cast<T*>(buffer.mutable_data());
@@ -91,9 +99,7 @@ class Decoder {
   // the decoder would decode put to 'max_values', storing the result in 'buffer'.
   // The function returns the number of values decoded, which should be max_values
   // except for end of the current data page.
-  virtual int Decode(T* /* buffer */, int /* max_values */) {
-    throw ParquetException("Decoder does not implement this type.");
-  }
+  virtual int Decode(T* buffer, int max_values) = 0;
 
   // Decode the values in this data page but leave spaces for null entries.
   //

cpp/src/parquet/exception.h

@@ -34,9 +34,10 @@
   }
 
 #define PARQUET_IGNORE_NOT_OK(s) \
-  try {                          \
-    (s);                         \
-  } catch (const ::parquet::ParquetException& e) { UNUSED(e); }
+  do {                           \
+    ::arrow::Status _s = (s);    \
+    UNUSED(_s);                  \
+  } while (0)
 
 #define PARQUET_THROW_NOT_OK(s)                     \
   do {                                              \
@@ -46,7 +47,7 @@
       ss << "Arrow error: " << _s.ToString();       \
       ::parquet::ParquetException::Throw(ss.str()); \
     }                                               \
-  } while (0);
+  } while (0)
 
 namespace parquet {