You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@parquet.apache.org by we...@apache.org on 2017/07/11 02:03:29 UTC

parquet-cpp git commit: PARQUET-1053: Fix unused result warnings due to unchecked Statuses

Repository: parquet-cpp
Updated Branches:
  refs/heads/master 562651e11 -> 3dedeed1d


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

Author: Phillip Cloud <cp...@gmail.com>

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

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


Project: http://git-wip-us.apache.org/repos/asf/parquet-cpp/repo
Commit: http://git-wip-us.apache.org/repos/asf/parquet-cpp/commit/3dedeed1
Tree: http://git-wip-us.apache.org/repos/asf/parquet-cpp/tree/3dedeed1
Diff: http://git-wip-us.apache.org/repos/asf/parquet-cpp/diff/3dedeed1

Branch: refs/heads/master
Commit: 3dedeed1d574d74bcfafe4359841accc12bd9997
Parents: 562651e
Author: Phillip Cloud <cp...@gmail.com>
Authored: Mon Jul 10 22:03:25 2017 -0400
Committer: Wes McKinney <we...@twosigma.com>
Committed: Mon Jul 10 22:03:25 2017 -0400

----------------------------------------------------------------------
 examples/reader-writer.cc                     |  3 ++-
 src/parquet/arrow/arrow-reader-writer-test.cc |  4 ++--
 src/parquet/arrow/reader.cc                   |  5 +++--
 src/parquet/arrow/test-util.h                 | 26 +++++++++++-----------
 src/parquet/arrow/writer.cc                   |  2 +-
 src/parquet/encoding.h                        | 14 ++++++++----
 src/parquet/exception.h                       |  9 ++++----
 7 files changed, 36 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/examples/reader-writer.cc
----------------------------------------------------------------------
diff --git a/examples/reader-writer.cc b/examples/reader-writer.cc
index 6f21f6c..210968c 100644
--- a/examples/reader-writer.cc
+++ b/examples/reader-writer.cc
@@ -22,6 +22,7 @@
 #include <memory>
 
 #include <arrow/io/file.h>
+#include <arrow/util/logging.h>
 
 #include <parquet/api/reader.h>
 #include <parquet/api/writer.h>
@@ -216,7 +217,7 @@ int main(int argc, char** argv) {
     file_writer->Close();
 
     // Write the bytes to file
-    out_file->Close();
+    DCHECK(out_file->Close().ok());
   } catch (const std::exception& e) {
     std::cerr << "Parquet write error: " << e.what() << std::endl;
     return -1;

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/arrow/arrow-reader-writer-test.cc
----------------------------------------------------------------------
diff --git a/src/parquet/arrow/arrow-reader-writer-test.cc b/src/parquet/arrow/arrow-reader-writer-test.cc
index f2a9651..ddc197c 100644
--- a/src/parquet/arrow/arrow-reader-writer-test.cc
+++ b/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);

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/arrow/reader.cc
----------------------------------------------------------------------
diff --git a/src/parquet/arrow/reader.cc b/src/parquet/arrow/reader.cc
index addfb85..9d7ceed 100644
--- a/src/parquet/arrow/reader.cc
+++ b/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());

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/arrow/test-util.h
----------------------------------------------------------------------
diff --git a/src/parquet/arrow/test-util.h b/src/parquet/arrow/test-util.h
index e44fcb6..191a399 100644
--- a/src/parquet/arrow/test-util.h
+++ b/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));

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/arrow/writer.cc
----------------------------------------------------------------------
diff --git a/src/parquet/arrow/writer.cc b/src/parquet/arrow/writer.cc
index 1e3f6de..a7aebac 100644
--- a/src/parquet/arrow/writer.cc
+++ b/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));

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/encoding.h
----------------------------------------------------------------------
diff --git a/src/parquet/encoding.h b/src/parquet/encoding.h
index 1417e98..ecf3940 100644
--- a/src/parquet/encoding.h
+++ b/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.
   //

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/3dedeed1/src/parquet/exception.h
----------------------------------------------------------------------
diff --git a/src/parquet/exception.h b/src/parquet/exception.h
index 40c24e5..8ec2195 100644
--- a/src/parquet/exception.h
+++ b/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 {