PARQUET-1053: Fix unused result warnings due to unchecked Statuses
Author: Phillip Cloud <cpcloud@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
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 @@
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;
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 @@
::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 @@
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);
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 @@
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 @@
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());
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
}
::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 @@
// 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 @@
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 @@
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 @@
}
::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 @@
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));
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 @@
*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));
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 @@
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 @@
// 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.
//
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 {
