You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/06/27 16:21:04 UTC
[arrow] branch master updated: ARROW-5145: [C++] More input
validation in release mode
This is an automated email from the ASF dual-hosted git repository.
wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new f77c342 ARROW-5145: [C++] More input validation in release mode
f77c342 is described below
commit f77c3427ca801597b572fb197b92b0133269049b
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Thu Jun 27 11:17:55 2019 -0500
ARROW-5145: [C++] More input validation in release mode
This will make user experience better when passing invalid arguments.
Author: Antoine Pitrou <an...@python.org>
Closes #4720 from pitrou/ARROW-5145-release-mode-checks and squashes the following commits:
daaeb220f <Antoine Pitrou> ARROW-5145: More input validation in release mode
---
cpp/cmake_modules/SetupCxxFlags.cmake | 2 +-
cpp/src/arrow/array-list-test.cc | 2 +-
cpp/src/arrow/array.cc | 104 +++++++++++++------------
cpp/src/arrow/array/builder_adaptive.cc | 2 -
cpp/src/arrow/array/builder_binary.cc | 6 +-
cpp/src/arrow/array/builder_dict.cc | 4 +-
cpp/src/arrow/array/builder_nested.cc | 9 ++-
cpp/src/arrow/array/builder_primitive.cc | 2 +-
cpp/src/arrow/buffer.cc | 4 +-
cpp/src/arrow/compute/kernels/util-internal.cc | 6 +-
cpp/src/arrow/extension_type.cc | 6 +-
cpp/src/arrow/flight/client.cc | 4 +-
cpp/src/arrow/flight/serialization-internal.cc | 4 +-
cpp/src/arrow/gpu/cuda_memory.cc | 20 +++--
cpp/src/arrow/io/buffered.cc | 16 ++--
cpp/src/arrow/io/compressed.cc | 4 +-
cpp/src/arrow/io/file.cc | 9 +--
cpp/src/arrow/io/memory.cc | 4 +-
cpp/src/arrow/ipc/message.cc | 3 +-
cpp/src/arrow/ipc/reader.cc | 20 +++--
cpp/src/arrow/python/arrow_to_pandas.cc | 2 +-
cpp/src/arrow/python/common.cc | 4 +-
cpp/src/arrow/python/decimal.cc | 8 +-
cpp/src/arrow/python/helpers.cc | 9 +--
cpp/src/arrow/python/helpers.h | 2 +-
cpp/src/arrow/python/inference.cc | 3 +-
cpp/src/arrow/python/python-test.cc | 13 ++--
cpp/src/arrow/python/type_traits.h | 2 +-
cpp/src/arrow/record_batch.cc | 4 +-
cpp/src/arrow/scalar.cc | 13 ++--
cpp/src/arrow/sparse_tensor.cc | 10 +--
cpp/src/arrow/status.cc | 14 +++-
cpp/src/arrow/status.h | 23 +++---
cpp/src/arrow/table.cc | 4 +-
cpp/src/arrow/table_builder.cc | 2 +-
cpp/src/arrow/tensor.cc | 6 +-
cpp/src/arrow/type.cc | 14 ++--
cpp/src/arrow/util/compression_bz2.cc | 4 +-
cpp/src/arrow/util/compression_zlib.cc | 8 +-
cpp/src/arrow/util/decimal.cc | 4 +-
cpp/src/arrow/util/io-util.cc | 7 +-
cpp/src/arrow/util/key_value_metadata.cc | 4 +-
cpp/src/arrow/util/logging.h | 45 ++++++-----
cpp/src/arrow/util/thread-pool.cc | 2 +-
44 files changed, 245 insertions(+), 193 deletions(-)
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
index 0c07a55..496904b 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -143,7 +143,7 @@ if("${BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN")
-Wno-comma -Wno-unused-macros -Wno-unused-parameter -Wno-unused-template -Wno-undef \
-Wno-shadow -Wno-switch-enum -Wno-exit-time-destructors \
-Wno-global-constructors -Wno-weak-template-vtables -Wno-undefined-reinterpret-cast \
--Wno-implicit-fallthrough -Wno-unreachable-code-return \
+-Wno-implicit-fallthrough -Wno-unreachable-code -Wno-unreachable-code-return \
-Wno-float-equal -Wno-missing-prototypes -Wno-documentation-unknown-command \
-Wno-old-style-cast -Wno-covered-switch-default \
-Wno-cast-align -Wno-vla-extension -Wno-shift-sign-overflow \
diff --git a/cpp/src/arrow/array-list-test.cc b/cpp/src/arrow/array-list-test.cc
index 3847b9e..c3118e9 100644
--- a/cpp/src/arrow/array-list-test.cc
+++ b/cpp/src/arrow/array-list-test.cc
@@ -189,7 +189,7 @@ TEST_F(TestListArray, TestFromArrays) {
ListArray::FromArrays(*offsets1->Slice(0, 0), *values, pool_, &tmp));
// Offsets not int32
- ASSERT_RAISES(Invalid, ListArray::FromArrays(*values, *offsets1, pool_, &tmp));
+ ASSERT_RAISES(TypeError, ListArray::FromArrays(*values, *offsets1, pool_, &tmp));
}
TEST_F(TestListArray, TestAppendNull) {
diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc
index b41d6dd..95acc6b 100644
--- a/cpp/src/arrow/array.cc
+++ b/cpp/src/arrow/array.cc
@@ -78,7 +78,7 @@ std::shared_ptr<ArrayData> ArrayData::Make(const std::shared_ptr<DataType>& type
}
ArrayData ArrayData::Slice(int64_t off, int64_t len) const {
- DCHECK_LE(off, length);
+ ARROW_CHECK_LE(off, length) << "Slice offset greater than array length";
len = std::min(length - off, len);
off += offset;
@@ -166,8 +166,7 @@ std::shared_ptr<Array> Array::Slice(int64_t offset) const {
std::string Array::ToString() const {
std::stringstream ss;
- Status s = PrettyPrint(*this, 0, &ss);
- DCHECK_OK(s);
+ ARROW_CHECK_OK(PrettyPrint(*this, 0, &ss));
return ss.str();
}
@@ -190,7 +189,7 @@ PrimitiveArray::PrimitiveArray(const std::shared_ptr<DataType>& type, int64_t le
BooleanArray::BooleanArray(const std::shared_ptr<ArrayData>& data)
: PrimitiveArray(data) {
- DCHECK_EQ(data->type->id(), Type::BOOL);
+ ARROW_CHECK_EQ(data->type->id(), Type::BOOL);
}
BooleanArray::BooleanArray(int64_t length, const std::shared_ptr<Buffer>& data,
@@ -221,7 +220,7 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo
}
if (offsets.type_id() != Type::INT32) {
- return Status::Invalid("List offsets must be signed int32");
+ return Status::TypeError("List offsets must be signed int32");
}
BufferVector buffers = {};
@@ -231,11 +230,15 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo
const int64_t num_offsets = offsets.length();
if (offsets.null_count() > 0) {
- std::shared_ptr<Buffer> clean_offsets, clean_valid_bits;
+ if (!offsets.IsValid(num_offsets - 1)) {
+ return Status::Invalid("Last list offset should be non-null");
+ }
+ std::shared_ptr<Buffer> clean_offsets, clean_valid_bits;
RETURN_NOT_OK(AllocateBuffer(pool, num_offsets * sizeof(int32_t), &clean_offsets));
// Copy valid bits, zero out the bit for the final offset
+ // XXX why?
RETURN_NOT_OK(offsets.null_bitmap()->Copy(0, BitUtil::BytesForBits(num_offsets - 1),
&clean_valid_bits));
BitUtil::ClearBit(clean_valid_bits->mutable_data(), num_offsets);
@@ -245,7 +248,6 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo
auto clean_raw_offsets = reinterpret_cast<int32_t*>(clean_offsets->mutable_data());
// Must work backwards so we can tell how many values were in the last non-null value
- DCHECK(offsets.IsValid(num_offsets - 1));
int32_t current_offset = raw_offsets[num_offsets - 1];
for (int64_t i = num_offsets - 1; i >= 0; --i) {
if (offsets.IsValid(i)) {
@@ -271,8 +273,8 @@ Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPo
void ListArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->Array::SetData(data);
- DCHECK_EQ(data->buffers.size(), 2);
- DCHECK(data->type->id() == Type::LIST);
+ ARROW_CHECK_EQ(data->buffers.size(), 2);
+ ARROW_CHECK(data->type->id() == Type::LIST);
list_type_ = checked_cast<const ListType*>(data->type.get());
auto value_offsets = data->buffers[1];
@@ -280,7 +282,9 @@ void ListArray::SetData(const std::shared_ptr<ArrayData>& data) {
? nullptr
: reinterpret_cast<const int32_t*>(value_offsets->data());
- DCHECK_EQ(data_->child_data.size(), 1);
+ ARROW_CHECK_EQ(data_->child_data.size(), 1);
+ ARROW_CHECK_EQ(list_type_->value_type()->id(), data->child_data[0]->type->id());
+ DCHECK(list_type_->value_type()->Equals(data->child_data[0]->type));
values_ = MakeArray(data_->child_data[0]);
}
@@ -309,12 +313,12 @@ MapArray::MapArray(const std::shared_ptr<DataType>& type, int64_t length,
}
void MapArray::SetData(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::MAP);
+ ARROW_CHECK_EQ(data->type->id(), Type::MAP);
auto pair_data = data->child_data[0];
- DCHECK_EQ(pair_data->type->id(), Type::STRUCT);
- DCHECK_EQ(pair_data->null_count, 0);
- DCHECK_EQ(pair_data->child_data.size(), 2);
- DCHECK_EQ(pair_data->child_data[0]->null_count, 0);
+ ARROW_CHECK_EQ(pair_data->type->id(), Type::STRUCT);
+ ARROW_CHECK_EQ(pair_data->null_count, 0);
+ ARROW_CHECK_EQ(pair_data->child_data.size(), 2);
+ ARROW_CHECK_EQ(pair_data->child_data[0]->null_count, 0);
auto pair_list_data = data->Copy();
pair_list_data->type = list(pair_data->type);
@@ -343,13 +347,14 @@ FixedSizeListArray::FixedSizeListArray(const std::shared_ptr<DataType>& type,
}
void FixedSizeListArray::SetData(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::FIXED_SIZE_LIST);
+ ARROW_CHECK_EQ(data->type->id(), Type::FIXED_SIZE_LIST);
this->Array::SetData(data);
+ ARROW_CHECK_EQ(list_type()->value_type()->id(), data->child_data[0]->type->id());
DCHECK(list_type()->value_type()->Equals(data->child_data[0]->type));
list_size_ = list_type()->list_size();
- DCHECK_EQ(data_->child_data.size(), 1);
+ ARROW_CHECK_EQ(data_->child_data.size(), 1);
values_ = MakeArray(data_->child_data[0]);
}
@@ -367,12 +372,12 @@ std::shared_ptr<Array> FixedSizeListArray::values() const { return values_; }
// String and binary
BinaryArray::BinaryArray(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::BINARY);
+ ARROW_CHECK_EQ(data->type->id(), Type::BINARY);
SetData(data);
}
void BinaryArray::SetData(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->buffers.size(), 3);
+ ARROW_CHECK_EQ(data->buffers.size(), 3);
auto value_offsets = data->buffers[1];
auto value_data = data->buffers[2];
this->Array::SetData(data);
@@ -399,7 +404,7 @@ BinaryArray::BinaryArray(const std::shared_ptr<DataType>& type, int64_t length,
}
StringArray::StringArray(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::STRING);
+ ARROW_CHECK_EQ(data->type->id(), Type::STRING);
SetData(data);
}
@@ -453,7 +458,7 @@ DayTimeIntervalType::DayMilliseconds DayTimeIntervalArray::GetValue(int64_t i) c
Decimal128Array::Decimal128Array(const std::shared_ptr<ArrayData>& data)
: FixedSizeBinaryArray(data) {
- DCHECK_EQ(data->type->id(), Type::DECIMAL);
+ ARROW_CHECK_EQ(data->type->id(), Type::DECIMAL);
}
std::string Decimal128Array::FormatValue(int64_t i) const {
@@ -466,7 +471,7 @@ std::string Decimal128Array::FormatValue(int64_t i) const {
// Struct
StructArray::StructArray(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::STRUCT);
+ ARROW_CHECK_EQ(data->type->id(), Type::STRUCT);
SetData(data);
boxed_fields_.resize(data->child_data.size());
}
@@ -475,6 +480,7 @@ StructArray::StructArray(const std::shared_ptr<DataType>& type, int64_t length,
const std::vector<std::shared_ptr<Array>>& children,
std::shared_ptr<Buffer> null_bitmap, int64_t null_count,
int64_t offset) {
+ ARROW_CHECK_EQ(type->id(), Type::STRUCT);
SetData(ArrayData::Make(type, length, {null_bitmap}, null_count, offset));
for (const auto& child : children) {
data_->child_data.push_back(child->data());
@@ -497,7 +503,6 @@ std::shared_ptr<Array> StructArray::field(int i) const {
}
boxed_fields_[i] = MakeArray(field_data);
}
- DCHECK(boxed_fields_[i]);
return boxed_fields_[i];
}
@@ -559,7 +564,8 @@ Status StructArray::Flatten(MemoryPool* pool, ArrayVector* out) const {
void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->Array::SetData(data);
- DCHECK_EQ(data->buffers.size(), 3);
+ ARROW_CHECK_EQ(data->type->id(), Type::UNION);
+ ARROW_CHECK_EQ(data->buffers.size(), 3);
auto type_ids = data_->buffers[1];
auto value_offsets = data_->buffers[2];
@@ -571,10 +577,7 @@ void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
boxed_fields_.resize(data->child_data.size());
}
-UnionArray::UnionArray(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::UNION);
- SetData(data);
-}
+UnionArray::UnionArray(const std::shared_ptr<ArrayData>& data) { SetData(data); }
UnionArray::UnionArray(const std::shared_ptr<DataType>& type, int64_t length,
const std::vector<std::shared_ptr<Array>>& children,
@@ -600,11 +603,11 @@ Status UnionArray::MakeDense(const Array& type_ids, const Array& value_offsets,
}
if (value_offsets.type_id() != Type::INT32) {
- return Status::Invalid("UnionArray offsets must be signed int32");
+ return Status::TypeError("UnionArray offsets must be signed int32");
}
if (type_ids.type_id() != Type::INT8) {
- return Status::Invalid("UnionArray type_ids must be signed int8");
+ return Status::TypeError("UnionArray type_ids must be signed int8");
}
if (value_offsets.null_count() != 0) {
@@ -640,7 +643,7 @@ Status UnionArray::MakeSparse(const Array& type_ids,
const std::vector<uint8_t>& type_codes,
std::shared_ptr<Array>* out) {
if (type_ids.type_id() != Type::INT8) {
- return Status::Invalid("UnionArray type_ids must be signed int8");
+ return Status::TypeError("UnionArray type_ids must be signed int8");
}
if (field_names.size() > 0 && field_names.size() != children.size()) {
@@ -681,7 +684,6 @@ std::shared_ptr<Array> UnionArray::child(int i) const {
}
boxed_fields_[i] = MakeArray(child_data);
}
- DCHECK(boxed_fields_[i]);
return boxed_fields_[i];
}
@@ -689,7 +691,6 @@ const Array* UnionArray::UnsafeChild(int i) const {
if (!boxed_fields_[i]) {
boxed_fields_[i] = MakeArray(data_->child_data[i]);
}
- DCHECK(boxed_fields_[i]);
return boxed_fields_[i].get();
}
@@ -733,8 +734,8 @@ std::shared_ptr<Array> DictionaryArray::indices() const { return indices_; }
DictionaryArray::DictionaryArray(const std::shared_ptr<ArrayData>& data)
: dict_type_(checked_cast<const DictionaryType*>(data->type.get())) {
- DCHECK_EQ(data->type->id(), Type::DICTIONARY);
- DCHECK(data->dictionary);
+ ARROW_CHECK_EQ(data->type->id(), Type::DICTIONARY);
+ ARROW_CHECK_NE(data->dictionary, nullptr);
SetData(data);
}
@@ -749,10 +750,10 @@ DictionaryArray::DictionaryArray(const std::shared_ptr<DataType>& type,
const std::shared_ptr<Array>& indices,
const std::shared_ptr<Array>& dictionary)
: dict_type_(checked_cast<const DictionaryType*>(type.get())) {
- DCHECK_EQ(type->id(), Type::DICTIONARY);
+ ARROW_CHECK_EQ(type->id(), Type::DICTIONARY);
+ ARROW_CHECK_EQ(indices->type_id(), dict_type_->index_type()->id());
+ ARROW_CHECK_EQ(dict_type_->value_type()->id(), dictionary->type()->id());
DCHECK(dict_type_->value_type()->Equals(*dictionary->type()));
- DCHECK_EQ(indices->type_id(), dict_type_->index_type()->id());
- DCHECK(dictionary);
auto data = indices->data()->Copy();
data->type = type;
data->dictionary = dictionary;
@@ -765,9 +766,11 @@ Status DictionaryArray::FromArrays(const std::shared_ptr<DataType>& type,
const std::shared_ptr<Array>& indices,
const std::shared_ptr<Array>& dictionary,
std::shared_ptr<Array>* out) {
- DCHECK_EQ(type->id(), Type::DICTIONARY);
+ if (type->id() != Type::DICTIONARY) {
+ return Status::TypeError("Expected a dictionary type");
+ }
const auto& dict = checked_cast<const DictionaryType&>(*type);
- DCHECK_EQ(indices->type_id(), dict.index_type()->id());
+ ARROW_CHECK_EQ(indices->type_id(), dict.index_type()->id());
int64_t upper_bound = dictionary->length();
Status is_valid;
@@ -786,16 +789,12 @@ Status DictionaryArray::FromArrays(const std::shared_ptr<DataType>& type,
is_valid = ValidateDictionaryIndices<Int64Type>(indices, upper_bound);
break;
default:
- return Status::NotImplemented("Categorical index type not supported: ",
+ return Status::NotImplemented("Dictionary index type not supported: ",
indices->type()->ToString());
}
-
- if (!is_valid.ok()) {
- return is_valid;
- }
-
+ RETURN_NOT_OK(is_valid);
*out = std::make_shared<DictionaryArray>(type, indices, dictionary);
- return is_valid;
+ return Status::OK();
}
template <typename InType, typename OutType>
@@ -816,7 +815,9 @@ Status DictionaryArray::Transpose(MemoryPool* pool, const std::shared_ptr<DataTy
const std::shared_ptr<Array>& dictionary,
const std::vector<int32_t>& transpose_map,
std::shared_ptr<Array>* out) const {
- DCHECK_EQ(type->id(), Type::DICTIONARY);
+ if (type->id() != Type::DICTIONARY) {
+ return Status::TypeError("Expected dictionary type");
+ }
const auto& out_dict_type = checked_cast<const DictionaryType&>(*type);
const auto& out_index_type =
@@ -1378,7 +1379,8 @@ class NullArrayFactory {
: type_(*type), length_(length), buffer_length_(BitUtil::BytesForBits(length)) {}
operator int64_t() && {
- DCHECK_OK(VisitTypeInline(type_, this));
+ // TODO this should implement proper error propagation
+ ARROW_CHECK_OK(VisitTypeInline(type_, this));
return buffer_length_;
}
@@ -1485,7 +1487,7 @@ class NullArrayFactory {
Status Visit(const StructType& type) {
for (int i = 0; i < type_->num_children(); ++i) {
- DCHECK_OK(CreateChild(i, length_, &(*out_)->child_data[i]));
+ RETURN_NOT_OK(CreateChild(i, length_, &(*out_)->child_data[i]));
}
return Status::OK();
}
@@ -1498,7 +1500,7 @@ class NullArrayFactory {
}
for (int i = 0; i < type_->num_children(); ++i) {
- DCHECK_OK(CreateChild(i, length_, &(*out_)->child_data[i]));
+ RETURN_NOT_OK(CreateChild(i, length_, &(*out_)->child_data[i]));
}
return Status::OK();
}
diff --git a/cpp/src/arrow/array/builder_adaptive.cc b/cpp/src/arrow/array/builder_adaptive.cc
index e96c9a2..b88c9d3 100644
--- a/cpp/src/arrow/array/builder_adaptive.cc
+++ b/cpp/src/arrow/array/builder_adaptive.cc
@@ -86,7 +86,6 @@ Status AdaptiveIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
output_type = int64();
break;
default:
- DCHECK(false);
return Status::NotImplemented("Only ints of size 1,2,4,8 are supported");
}
@@ -272,7 +271,6 @@ Status AdaptiveUIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
output_type = uint64();
break;
default:
- DCHECK(false);
return Status::NotImplemented("Only ints of size 1,2,4,8 are supported");
}
diff --git a/cpp/src/arrow/array/builder_binary.cc b/cpp/src/arrow/array/builder_binary.cc
index ccb79a1..818ad15 100644
--- a/cpp/src/arrow/array/builder_binary.cc
+++ b/cpp/src/arrow/array/builder_binary.cc
@@ -49,7 +49,11 @@ BinaryBuilder::BinaryBuilder(const std::shared_ptr<DataType>& type, MemoryPool*
BinaryBuilder::BinaryBuilder(MemoryPool* pool) : BinaryBuilder(binary(), pool) {}
Status BinaryBuilder::Resize(int64_t capacity) {
- DCHECK_LE(capacity, kListMaximumElements);
+ if (capacity > kListMaximumElements) {
+ return Status::CapacityError(
+ "BinaryBuilder cannot reserve space for more then 2^31 - 1 child elements, got ",
+ capacity);
+ }
RETURN_NOT_OK(CheckCapacity(capacity, capacity_));
// one more then requested for offsets
diff --git a/cpp/src/arrow/array/builder_dict.cc b/cpp/src/arrow/array/builder_dict.cc
index 648b6ff..8d5814a 100644
--- a/cpp/src/arrow/array/builder_dict.cc
+++ b/cpp/src/arrow/array/builder_dict.cc
@@ -107,7 +107,9 @@ Status DictionaryType::Unify(MemoryPool* pool, const std::vector<const DataType*
return Status::Invalid("need at least one input type");
}
- DCHECK_EQ(types.size(), dictionaries.size());
+ if (types.size() != dictionaries.size()) {
+ return Status::Invalid("expecting the same number of types and dictionaries");
+ }
std::vector<const DictionaryType*> dict_types;
dict_types.reserve(types.size());
diff --git a/cpp/src/arrow/array/builder_nested.cc b/cpp/src/arrow/array/builder_nested.cc
index 309cd2a..0bf9171 100644
--- a/cpp/src/arrow/array/builder_nested.cc
+++ b/cpp/src/arrow/array/builder_nested.cc
@@ -89,7 +89,11 @@ Status ListBuilder::AppendNulls(int64_t length) {
}
Status ListBuilder::Resize(int64_t capacity) {
- DCHECK_LE(capacity, kListMaximumElements);
+ if (capacity > kListMaximumElements) {
+ return Status::CapacityError(
+ "ListArray cannot reserve space for more then 2^31 - 1 child elements, got ",
+ capacity);
+ }
RETURN_NOT_OK(CheckCapacity(capacity, capacity_));
// one more then requested for offsets
@@ -169,7 +173,8 @@ void MapBuilder::Reset() {
}
Status MapBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
- DCHECK_EQ(item_builder_->length(), key_builder_->length());
+ ARROW_CHECK_EQ(item_builder_->length(), key_builder_->length())
+ << "keys and items builders don't have the same size in MapBuilder";
// finish list(keys) builder
RETURN_NOT_OK(list_builder_->FinishInternal(out));
// finish values builder
diff --git a/cpp/src/arrow/array/builder_primitive.cc b/cpp/src/arrow/array/builder_primitive.cc
index 3c899c0..34d198e 100644
--- a/cpp/src/arrow/array/builder_primitive.cc
+++ b/cpp/src/arrow/array/builder_primitive.cc
@@ -49,7 +49,7 @@ BooleanBuilder::BooleanBuilder(MemoryPool* pool)
BooleanBuilder::BooleanBuilder(const std::shared_ptr<DataType>& type, MemoryPool* pool)
: BooleanBuilder(pool) {
- DCHECK_EQ(Type::BOOL, type->id());
+ ARROW_CHECK_EQ(Type::BOOL, type->id());
}
void BooleanBuilder::Reset() {
diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc
index 589d93d..510b9cf 100644
--- a/cpp/src/arrow/buffer.cc
+++ b/cpp/src/arrow/buffer.cc
@@ -32,8 +32,8 @@ namespace arrow {
Status Buffer::Copy(const int64_t start, const int64_t nbytes, MemoryPool* pool,
std::shared_ptr<Buffer>* out) const {
// Sanity checks
- DCHECK_LT(start, size_);
- DCHECK_LE(nbytes, size_ - start);
+ ARROW_CHECK_LT(start, size_);
+ ARROW_CHECK_LE(nbytes, size_ - start);
std::shared_ptr<ResizableBuffer> new_buffer;
RETURN_NOT_OK(AllocateResizableBuffer(pool, nbytes, &new_buffer));
diff --git a/cpp/src/arrow/compute/kernels/util-internal.cc b/cpp/src/arrow/compute/kernels/util-internal.cc
index f29badf..8b1f803 100644
--- a/cpp/src/arrow/compute/kernels/util-internal.cc
+++ b/cpp/src/arrow/compute/kernels/util-internal.cc
@@ -58,7 +58,7 @@ Status AllocateValueBuffer(FunctionContext* ctx, const DataType& type, int64_t l
if (bit_width == 1) {
buffer_size = BitUtil::BytesForBits(length);
} else {
- DCHECK_EQ(bit_width % 8, 0)
+ ARROW_CHECK_EQ(bit_width % 8, 0)
<< "Only bit widths with multiple of 8 are currently supported";
buffer_size = length * fw_type.bit_width() / 8;
}
@@ -182,7 +182,7 @@ Datum WrapArraysLike(const Datum& value,
} else if (value.kind() == Datum::CHUNKED_ARRAY) {
return Datum(std::make_shared<ChunkedArray>(arrays));
} else {
- DCHECK(false) << "unhandled datum kind";
+ ARROW_LOG(FATAL) << "unhandled datum kind";
return Datum();
}
}
@@ -200,7 +200,7 @@ Datum WrapDatumsLike(const Datum& value, const std::vector<Datum>& datums) {
}
return Datum(std::make_shared<ChunkedArray>(arrays));
} else {
- DCHECK(false) << "unhandled datum kind";
+ ARROW_LOG(FATAL) << "unhandled datum kind";
return Datum();
}
}
diff --git a/cpp/src/arrow/extension_type.cc b/cpp/src/arrow/extension_type.cc
index 25945f3..b08f974 100644
--- a/cpp/src/arrow/extension_type.cc
+++ b/cpp/src/arrow/extension_type.cc
@@ -49,8 +49,8 @@ ExtensionArray::ExtensionArray(const std::shared_ptr<ArrayData>& data) { SetData
ExtensionArray::ExtensionArray(const std::shared_ptr<DataType>& type,
const std::shared_ptr<Array>& storage) {
- DCHECK_EQ(type->id(), Type::EXTENSION);
- DCHECK(
+ ARROW_CHECK_EQ(type->id(), Type::EXTENSION);
+ ARROW_CHECK(
storage->type()->Equals(*checked_cast<const ExtensionType&>(*type).storage_type()));
auto data = storage->data()->Copy();
// XXX This pointer is reverted below in SetData()...
@@ -59,7 +59,7 @@ ExtensionArray::ExtensionArray(const std::shared_ptr<DataType>& type,
}
void ExtensionArray::SetData(const std::shared_ptr<ArrayData>& data) {
- DCHECK_EQ(data->type->id(), Type::EXTENSION);
+ ARROW_CHECK_EQ(data->type->id(), Type::EXTENSION);
this->Array::SetData(data);
auto storage_data = data->Copy();
diff --git a/cpp/src/arrow/flight/client.cc b/cpp/src/arrow/flight/client.cc
index 0e0d82c..c508dca 100644
--- a/cpp/src/arrow/flight/client.cc
+++ b/cpp/src/arrow/flight/client.cc
@@ -295,7 +295,9 @@ class DoPutPayloadWriter : public ipc::internal::IpcPayloadWriter {
if (first_payload_) {
// First Flight message needs to encore the Flight descriptor
- DCHECK_EQ(ipc_payload.type, ipc::Message::SCHEMA);
+ if (ipc_payload.type != ipc::Message::SCHEMA) {
+ return Status::Invalid("First IPC message should be schema");
+ }
std::string str_descr;
{
pb::FlightDescriptor pb_descr;
diff --git a/cpp/src/arrow/flight/serialization-internal.cc b/cpp/src/arrow/flight/serialization-internal.cc
index 0ff5aba..74b1b20 100644
--- a/cpp/src/arrow/flight/serialization-internal.cc
+++ b/cpp/src/arrow/flight/serialization-internal.cc
@@ -150,7 +150,9 @@ grpc::Status FlightDataSerialize(const FlightPayload& msg, ByteBuffer* out,
// Write the descriptor if present
int32_t descriptor_size = 0;
if (msg.descriptor != nullptr) {
- DCHECK_LT(msg.descriptor->size(), kInt32Max);
+ if (msg.descriptor->size() > kInt32Max) {
+ return ToGrpcStatus(Status::CapacityError("Descriptor size overflow (>= 2**31)"));
+ }
descriptor_size = static_cast<int32_t>(msg.descriptor->size());
header_size += 1 + WireFormatLite::LengthDelimitedSize(descriptor_size);
}
diff --git a/cpp/src/arrow/gpu/cuda_memory.cc b/cpp/src/arrow/gpu/cuda_memory.cc
index c777bf7..522ebe2 100644
--- a/cpp/src/arrow/gpu/cuda_memory.cc
+++ b/cpp/src/arrow/gpu/cuda_memory.cc
@@ -99,7 +99,7 @@ CudaBuffer::CudaBuffer(uint8_t* data, int64_t size,
mutable_data_ = data;
}
-CudaBuffer::~CudaBuffer() { DCHECK_OK(Close()); }
+CudaBuffer::~CudaBuffer() { ARROW_CHECK_OK(Close()); }
Status CudaBuffer::Close() {
if (own_data_) {
@@ -153,20 +153,26 @@ Status CudaBuffer::CopyToHost(const int64_t position, const int64_t nbytes,
Status CudaBuffer::CopyFromHost(const int64_t position, const void* data,
int64_t nbytes) {
- DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer";
+ if (nbytes > size_ - position) {
+ return Status::Invalid("Copy would overflow buffer");
+ }
return context_->CopyHostToDevice(mutable_data_ + position, data, nbytes);
}
Status CudaBuffer::CopyFromDevice(const int64_t position, const void* data,
int64_t nbytes) {
- DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer";
+ if (nbytes > size_ - position) {
+ return Status::Invalid("Copy would overflow buffer");
+ }
return context_->CopyDeviceToDevice(mutable_data_ + position, data, nbytes);
}
Status CudaBuffer::CopyFromAnotherDevice(const std::shared_ptr<CudaContext>& src_ctx,
const int64_t position, const void* data,
int64_t nbytes) {
- DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer";
+ if (nbytes > size_ - position) {
+ return Status::Invalid("Copy would overflow buffer");
+ }
return src_ctx->CopyDeviceToAnotherDevice(context_, mutable_data_ + position, data,
nbytes);
}
@@ -182,8 +188,8 @@ Status CudaBuffer::ExportForIpc(std::shared_ptr<CudaIpcMemHandle>* handle) {
CudaHostBuffer::~CudaHostBuffer() {
CudaDeviceManager* manager = nullptr;
- DCHECK_OK(CudaDeviceManager::GetInstance(&manager));
- DCHECK_OK(manager->FreeHost(mutable_data_, size_));
+ ARROW_CHECK_OK(CudaDeviceManager::GetInstance(&manager));
+ ARROW_CHECK_OK(manager->FreeHost(mutable_data_, size_));
}
// ----------------------------------------------------------------------
@@ -225,7 +231,7 @@ class CudaBufferWriter::CudaBufferWriterImpl {
buffer_size_(0),
buffer_position_(0) {
buffer_ = buffer;
- DCHECK(buffer->is_mutable()) << "Must pass mutable buffer";
+ ARROW_CHECK(buffer->is_mutable()) << "Must pass mutable buffer";
mutable_data_ = buffer->mutable_data();
size_ = buffer->size();
position_ = 0;
diff --git a/cpp/src/arrow/io/buffered.cc b/cpp/src/arrow/io/buffered.cc
index ed3cd4e..ad8260c 100644
--- a/cpp/src/arrow/io/buffered.cc
+++ b/cpp/src/arrow/io/buffered.cc
@@ -160,7 +160,9 @@ class BufferedOutputStream::Impl : public BufferedBase {
Status SetBufferSize(int64_t new_buffer_size) {
std::lock_guard<std::mutex> guard(lock_);
- DCHECK_GT(new_buffer_size, 0);
+ if (new_buffer_size <= 0) {
+ return Status::Invalid("Buffer size should be positive");
+ }
if (buffer_pos_ >= new_buffer_size) {
// If the buffer is shrinking, first flush to the raw OutputStream
RETURN_NOT_OK(FlushUnlocked());
@@ -189,7 +191,7 @@ Status BufferedOutputStream::Create(int64_t buffer_size, MemoryPool* pool,
return Status::OK();
}
-BufferedOutputStream::~BufferedOutputStream() { DCHECK_OK(impl_->Close()); }
+BufferedOutputStream::~BufferedOutputStream() { ARROW_CHECK_OK(impl_->Close()); }
Status BufferedOutputStream::SetBufferSize(int64_t new_buffer_size) {
return impl_->SetBufferSize(new_buffer_size);
@@ -229,7 +231,7 @@ class BufferedInputStream::Impl : public BufferedBase {
raw_read_bound_(raw_total_bytes_bound),
bytes_buffered_(0) {}
- ~Impl() { DCHECK_OK(Close()); }
+ ~Impl() { ARROW_CHECK_OK(Close()); }
Status Close() {
std::lock_guard<std::mutex> guard(lock_);
@@ -253,7 +255,9 @@ class BufferedInputStream::Impl : public BufferedBase {
Status SetBufferSize(int64_t new_buffer_size) {
std::lock_guard<std::mutex> guard(lock_);
- DCHECK_GT(new_buffer_size, 0);
+ if (new_buffer_size <= 0) {
+ return Status::Invalid("Buffer size should be positive");
+ }
if ((buffer_pos_ + bytes_buffered_) >= new_buffer_size) {
return Status::Invalid("Cannot shrink read buffer if buffered data remains");
}
@@ -339,7 +343,7 @@ class BufferedInputStream::Impl : public BufferedBase {
Status Read(int64_t nbytes, int64_t* bytes_read, void* out) {
std::lock_guard<std::mutex> guard(lock_);
- DCHECK_GT(nbytes, 0);
+ ARROW_CHECK_GT(nbytes, 0);
if (nbytes < buffer_size_) {
// Pre-buffer for small reads
@@ -408,7 +412,7 @@ BufferedInputStream::BufferedInputStream(std::shared_ptr<InputStream> raw,
impl_.reset(new Impl(std::move(raw), pool, raw_total_bytes_bound));
}
-BufferedInputStream::~BufferedInputStream() { DCHECK_OK(impl_->Close()); }
+BufferedInputStream::~BufferedInputStream() { ARROW_CHECK_OK(impl_->Close()); }
Status BufferedInputStream::Create(int64_t buffer_size, MemoryPool* pool,
std::shared_ptr<InputStream> raw,
diff --git a/cpp/src/arrow/io/compressed.cc b/cpp/src/arrow/io/compressed.cc
index 301ebc3..43de132 100644
--- a/cpp/src/arrow/io/compressed.cc
+++ b/cpp/src/arrow/io/compressed.cc
@@ -46,7 +46,7 @@ class CompressedOutputStream::Impl {
Impl(MemoryPool* pool, Codec* codec, const std::shared_ptr<OutputStream>& raw)
: pool_(pool), raw_(raw), codec_(codec), is_open_(true), compressed_pos_(0) {}
- ~Impl() { DCHECK_OK(Close()); }
+ ~Impl() { ARROW_CHECK_OK(Close()); }
Status Init() {
RETURN_NOT_OK(codec_->MakeCompressor(&compressor_));
@@ -241,7 +241,7 @@ class CompressedInputStream::Impl {
return Status::OK();
}
- ~Impl() { DCHECK_OK(Close()); }
+ ~Impl() { ARROW_CHECK_OK(Close()); }
Status Close() {
std::lock_guard<std::mutex> guard(lock_);
diff --git a/cpp/src/arrow/io/file.cc b/cpp/src/arrow/io/file.cc
index 2b7d1a5..d5a48df 100644
--- a/cpp/src/arrow/io/file.cc
+++ b/cpp/src/arrow/io/file.cc
@@ -264,7 +264,7 @@ class ReadableFile::ReadableFileImpl : public OSFile {
ReadableFile::ReadableFile(MemoryPool* pool) { impl_.reset(new ReadableFileImpl(pool)); }
-ReadableFile::~ReadableFile() { DCHECK_OK(impl_->Close()); }
+ReadableFile::~ReadableFile() { ARROW_CHECK_OK(impl_->Close()); }
Status ReadableFile::Open(const std::string& path, std::shared_ptr<ReadableFile>* file) {
return Open(path, default_memory_pool(), file);
@@ -337,7 +337,7 @@ FileOutputStream::FileOutputStream() { impl_.reset(new FileOutputStreamImpl());
FileOutputStream::~FileOutputStream() {
// This can fail; better to explicitly call close
- DCHECK_OK(impl_->Close());
+ ARROW_CHECK_OK(impl_->Close());
}
Status FileOutputStream::Open(const std::string& path,
@@ -393,11 +393,10 @@ class MemoryMappedFile::MemoryMap : public MutableBuffer {
MemoryMap() : MutableBuffer(nullptr, 0) {}
~MemoryMap() {
- DCHECK_OK(Close());
+ ARROW_CHECK_OK(Close());
if (mutable_data_ != nullptr) {
int result = munmap(mutable_data_, static_cast<size_t>(size_));
- DCHECK_EQ(result, 0);
- ARROW_UNUSED(result);
+ ARROW_CHECK_EQ(result, 0) << "munmap failed";
}
}
diff --git a/cpp/src/arrow/io/memory.cc b/cpp/src/arrow/io/memory.cc
index 22a9f9b..e0013ea 100644
--- a/cpp/src/arrow/io/memory.cc
+++ b/cpp/src/arrow/io/memory.cc
@@ -65,7 +65,7 @@ Status BufferOutputStream::Reset(int64_t initial_capacity, MemoryPool* pool) {
BufferOutputStream::~BufferOutputStream() {
// This can fail, better to explicitly call close
if (buffer_) {
- DCHECK_OK(Close());
+ ARROW_CHECK_OK(Close());
}
}
@@ -159,7 +159,7 @@ class FixedSizeBufferWriter::FixedSizeBufferWriterImpl {
memcopy_blocksize_(kMemcopyDefaultBlocksize),
memcopy_threshold_(kMemcopyDefaultThreshold) {
buffer_ = buffer;
- DCHECK(buffer->is_mutable()) << "Must pass mutable buffer";
+ ARROW_CHECK(buffer->is_mutable()) << "Must pass mutable buffer";
mutable_data_ = buffer->mutable_data();
size_ = buffer->size();
position_ = 0;
diff --git a/cpp/src/arrow/ipc/message.cc b/cpp/src/arrow/ipc/message.cc
index 1de2a26..612b1a6 100644
--- a/cpp/src/arrow/ipc/message.cc
+++ b/cpp/src/arrow/ipc/message.cc
@@ -226,7 +226,8 @@ std::string FormatMessageType(Message::Type type) {
Status ReadMessage(int64_t offset, int32_t metadata_length, io::RandomAccessFile* file,
std::unique_ptr<Message>* message) {
- DCHECK_GT(static_cast<size_t>(metadata_length), sizeof(int32_t));
+ ARROW_CHECK_GT(static_cast<size_t>(metadata_length), sizeof(int32_t))
+ << "metadata_length should be at least 4";
std::shared_ptr<Buffer> buffer;
RETURN_NOT_OK(file->ReadAt(offset, metadata_length, &buffer));
diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc
index 992c4fc..002379e 100644
--- a/cpp/src/arrow/ipc/reader.cc
+++ b/cpp/src/arrow/ipc/reader.cc
@@ -104,9 +104,11 @@ class IpcComponentSource {
*out = nullptr;
return Status::OK();
} else {
- DCHECK(BitUtil::IsMultipleOf8(buffer->offset()))
- << "Buffer " << buffer_index
- << " did not start on 8-byte aligned offset: " << buffer->offset();
+ if (!BitUtil::IsMultipleOf8(buffer->offset())) {
+ return Status::Invalid(
+ "Buffer ", buffer_index,
+ " did not start on 8-byte aligned offset: ", buffer->offset());
+ }
return file_->ReadAt(buffer->offset(), buffer->length(), out);
}
}
@@ -357,7 +359,9 @@ static Status LoadRecordBatchFromSource(const std::shared_ptr<Schema>& schema,
for (int i = 0; i < schema->num_fields(); ++i) {
auto arr = std::make_shared<ArrayData>();
RETURN_NOT_OK(LoadArray(*schema->field(i), &context, arr.get()));
- DCHECK_EQ(num_rows, arr->length) << "Array length did not match record batch length";
+ if (num_rows != arr->length) {
+ return Status::IOError("Array length did not match record batch length");
+ }
arrays[i] = std::move(arr);
}
@@ -893,9 +897,11 @@ Status ReadSparseTensor(const Buffer& metadata, io::RandomAccessFile* file,
"Header-type of flatbuffer-encoded Message is not SparseTensor.");
}
const flatbuf::Buffer* buffer = sparse_tensor->data();
- DCHECK(BitUtil::IsMultipleOf8(buffer->offset()))
- << "Buffer of sparse index data "
- << "did not start on 8-byte aligned offset: " << buffer->offset();
+ if (!BitUtil::IsMultipleOf8(buffer->offset())) {
+ return Status::Invalid(
+ "Buffer of sparse index data did not start on 8-byte aligned offset: ",
+ buffer->offset());
+ }
std::shared_ptr<Buffer> data;
RETURN_NOT_OK(file->ReadAt(buffer->offset(), buffer->length(), &data));
diff --git a/cpp/src/arrow/python/arrow_to_pandas.cc b/cpp/src/arrow/python/arrow_to_pandas.cc
index d992001..59bdb17 100644
--- a/cpp/src/arrow/python/arrow_to_pandas.cc
+++ b/cpp/src/arrow/python/arrow_to_pandas.cc
@@ -688,7 +688,7 @@ static Status ConvertDecimals(const PandasOptions& options, const ChunkedArray&
OwnedRef decimal;
OwnedRef Decimal;
RETURN_NOT_OK(internal::ImportModule("decimal", &decimal));
- RETURN_NOT_OK(internal::ImportFromModule(decimal, "Decimal", &Decimal));
+ RETURN_NOT_OK(internal::ImportFromModule(decimal.obj(), "Decimal", &Decimal));
PyObject* decimal_constructor = Decimal.obj();
for (int c = 0; c < data.num_chunks(); c++) {
diff --git a/cpp/src/arrow/python/common.cc b/cpp/src/arrow/python/common.cc
index 8f844e6..aa44ec0 100644
--- a/cpp/src/arrow/python/common.cc
+++ b/cpp/src/arrow/python/common.cc
@@ -55,7 +55,7 @@ PyBuffer::PyBuffer() : Buffer(nullptr, 0) {}
Status PyBuffer::Init(PyObject* obj) {
if (!PyObject_GetBuffer(obj, &py_buf_, PyBUF_ANY_CONTIGUOUS)) {
data_ = reinterpret_cast<const uint8_t*>(py_buf_.buf);
- DCHECK(data_ != nullptr);
+ ARROW_CHECK_NE(data_, nullptr) << "Null pointer in Py_buffer";
size_ = py_buf_.len;
capacity_ = py_buf_.len;
is_mutable_ = !py_buf_.readonly;
@@ -94,7 +94,7 @@ Status ConvertPyError(StatusCode code) {
PyErr_Fetch(&exc_type, &exc_value, &traceback);
PyErr_NormalizeException(&exc_type, &exc_value, &traceback);
- DCHECK_NE(exc_type, nullptr);
+ DCHECK_NE(exc_type, nullptr) << "ConvertPyError called without an exception set";
OwnedRef exc_type_ref(exc_type);
OwnedRef exc_value_ref(exc_value);
diff --git a/cpp/src/arrow/python/decimal.cc b/cpp/src/arrow/python/decimal.cc
index 4b6932d..a1ba3d5 100644
--- a/cpp/src/arrow/python/decimal.cc
+++ b/cpp/src/arrow/python/decimal.cc
@@ -33,7 +33,7 @@ namespace internal {
Status ImportDecimalType(OwnedRef* decimal_type) {
OwnedRef decimal_module;
RETURN_NOT_OK(ImportModule("decimal", &decimal_module));
- RETURN_NOT_OK(ImportFromModule(decimal_module, "Decimal", decimal_type));
+ RETURN_NOT_OK(ImportFromModule(decimal_module.obj(), "Decimal", decimal_type));
return Status::OK();
}
@@ -166,14 +166,13 @@ Status DecimalFromPyObject(PyObject* obj, const DecimalType& arrow_type,
bool PyDecimal_Check(PyObject* obj) {
static OwnedRef decimal_type;
if (!decimal_type.obj()) {
- Status status = ImportDecimalType(&decimal_type);
- DCHECK_OK(status);
+ ARROW_CHECK_OK(ImportDecimalType(&decimal_type));
DCHECK(PyType_Check(decimal_type.obj()));
}
// PyObject_IsInstance() is slower as it has to check for virtual subclasses
const int result =
PyType_IsSubtype(Py_TYPE(obj), reinterpret_cast<PyTypeObject*>(decimal_type.obj()));
- DCHECK_NE(result, -1) << " error during PyType_IsSubtype check";
+ ARROW_CHECK_NE(result, -1) << " error during PyType_IsSubtype check";
return result == 1;
}
@@ -209,7 +208,6 @@ Status DecimalMetadata::Update(int32_t suggested_precision, int32_t suggested_sc
Status DecimalMetadata::Update(PyObject* object) {
bool is_decimal = PyDecimal_Check(object);
- DCHECK(is_decimal) << "Object is not a Python Decimal";
if (ARROW_PREDICT_FALSE(!is_decimal || PyDecimal_ISNAN(object))) {
return Status::OK();
diff --git a/cpp/src/arrow/python/helpers.cc b/cpp/src/arrow/python/helpers.cc
index ab36036..e44878f 100644
--- a/cpp/src/arrow/python/helpers.cc
+++ b/cpp/src/arrow/python/helpers.cc
@@ -144,18 +144,13 @@ Status PyObject_StdStringStr(PyObject* obj, std::string* out) {
Status ImportModule(const std::string& module_name, OwnedRef* ref) {
PyObject* module = PyImport_ImportModule(module_name.c_str());
RETURN_IF_PYERROR();
- DCHECK_NE(module, nullptr) << "unable to import the " << module_name << " module";
ref->reset(module);
return Status::OK();
}
-Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRef* ref) {
- /// Assumes that ImportModule was called first
- DCHECK_NE(module.obj(), nullptr) << "Cannot import from nullptr Python module";
-
- PyObject* attr = PyObject_GetAttrString(module.obj(), name.c_str());
+Status ImportFromModule(PyObject* module, const std::string& name, OwnedRef* ref) {
+ PyObject* attr = PyObject_GetAttrString(module, name.c_str());
RETURN_IF_PYERROR();
- DCHECK_NE(attr, nullptr) << "unable to import the " << name << " object";
ref->reset(attr);
return Status::OK();
}
diff --git a/cpp/src/arrow/python/helpers.h b/cpp/src/arrow/python/helpers.h
index 47b7810..4917f99 100644
--- a/cpp/src/arrow/python/helpers.h
+++ b/cpp/src/arrow/python/helpers.h
@@ -62,7 +62,7 @@ Status ImportModule(const std::string& module_name, OwnedRef* ref);
// \param[out] ref The OwnedRef containing the \c name attribute of the Python module \c
// module
ARROW_PYTHON_EXPORT
-Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRef* ref);
+Status ImportFromModule(PyObject* module, const std::string& name, OwnedRef* ref);
// \brief Check whether obj is an integer, independent of Python versions.
inline bool IsPyInteger(PyObject* obj) {
diff --git a/cpp/src/arrow/python/inference.cc b/cpp/src/arrow/python/inference.cc
index 9d2c35f..eddba0e 100644
--- a/cpp/src/arrow/python/inference.cc
+++ b/cpp/src/arrow/python/inference.cc
@@ -309,8 +309,7 @@ class TypeInferrer {
max_decimal_metadata_(std::numeric_limits<int32_t>::min(),
std::numeric_limits<int32_t>::min()),
decimal_type_() {
- Status status = internal::ImportDecimalType(&decimal_type_);
- DCHECK_OK(status);
+ ARROW_CHECK_OK(internal::ImportDecimalType(&decimal_type_));
}
/// \param[in] obj a Python object in the sequence
diff --git a/cpp/src/arrow/python/python-test.cc b/cpp/src/arrow/python/python-test.cc
index 03e4970..a8282a5 100644
--- a/cpp/src/arrow/python/python-test.cc
+++ b/cpp/src/arrow/python/python-test.cc
@@ -134,10 +134,11 @@ class DecimalTest : public ::testing::Test {
OwnedRef decimal_module;
Status status = internal::ImportModule("decimal", &decimal_module);
- DCHECK_OK(status);
+ ARROW_CHECK_OK(status);
- status = internal::ImportFromModule(decimal_module, "Decimal", &decimal_constructor_);
- DCHECK_OK(status);
+ status = internal::ImportFromModule(decimal_module.obj(), "Decimal",
+ &decimal_constructor_);
+ ARROW_CHECK_OK(status);
}
OwnedRef CreatePythonDecimal(const std::string& string_value) {
@@ -239,13 +240,15 @@ TEST(PandasConversionTest, TestObjectBlockWriteFails) {
auto schema = ::arrow::schema(fields);
auto table = Table::Make(schema, cols);
- PyObject* out;
+ Status st;
Py_BEGIN_ALLOW_THREADS;
+ PyObject* out;
PandasOptions options;
options.use_threads = true;
MemoryPool* pool = default_memory_pool();
- ASSERT_RAISES(UnknownError, ConvertTableToPandas(options, table, pool, &out));
+ st = ConvertTableToPandas(options, table, pool, &out);
Py_END_ALLOW_THREADS;
+ ASSERT_RAISES(UnknownError, st);
}
TEST(BuiltinConversionTest, TestMixedTypeFails) {
diff --git a/cpp/src/arrow/python/type_traits.h b/cpp/src/arrow/python/type_traits.h
index bc71ec4..6153802 100644
--- a/cpp/src/arrow/python/type_traits.h
+++ b/cpp/src/arrow/python/type_traits.h
@@ -291,7 +291,7 @@ static inline int NumPyTypeSize(int npy_type) {
case NPY_OBJECT:
return sizeof(void*);
default:
- DCHECK(false) << "unhandled numpy type";
+ ARROW_CHECK(false) << "unhandled numpy type";
break;
}
return -1;
diff --git a/cpp/src/arrow/record_batch.cc b/cpp/src/arrow/record_batch.cc
index 2bc8c22..1f266df 100644
--- a/cpp/src/arrow/record_batch.cc
+++ b/cpp/src/arrow/record_batch.cc
@@ -97,8 +97,8 @@ class SimpleRecordBatch : public RecordBatch {
Status AddColumn(int i, const std::shared_ptr<Field>& field,
const std::shared_ptr<Array>& column,
std::shared_ptr<RecordBatch>* out) const override {
- DCHECK(field != nullptr);
- DCHECK(column != nullptr);
+ ARROW_CHECK(field != nullptr);
+ ARROW_CHECK(column != nullptr);
if (!field->type()->Equals(column->type())) {
return Status::Invalid("Column data type ", field->type()->name(),
diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc
index 4bc9b92..2562862 100644
--- a/cpp/src/arrow/scalar.cc
+++ b/cpp/src/arrow/scalar.cc
@@ -36,26 +36,27 @@ bool Scalar::Equals(const Scalar& other) const { return ScalarEquals(*this, othe
Time32Scalar::Time32Scalar(int32_t value, const std::shared_ptr<DataType>& type,
bool is_valid)
: internal::PrimitiveScalar{type, is_valid}, value(value) {
- DCHECK_EQ(Type::TIME32, type->id());
+ ARROW_CHECK_EQ(Type::TIME32, type->id());
}
Time64Scalar::Time64Scalar(int64_t value, const std::shared_ptr<DataType>& type,
bool is_valid)
: internal::PrimitiveScalar{type, is_valid}, value(value) {
- DCHECK_EQ(Type::TIME64, type->id());
+ ARROW_CHECK_EQ(Type::TIME64, type->id());
}
TimestampScalar::TimestampScalar(int64_t value, const std::shared_ptr<DataType>& type,
bool is_valid)
: internal::PrimitiveScalar{type, is_valid}, value(value) {
- DCHECK_EQ(Type::TIMESTAMP, type->id());
+ ARROW_CHECK_EQ(Type::TIMESTAMP, type->id());
}
FixedSizeBinaryScalar::FixedSizeBinaryScalar(const std::shared_ptr<Buffer>& value,
const std::shared_ptr<DataType>& type,
bool is_valid)
: BinaryScalar(value, type, is_valid) {
- DCHECK_EQ(checked_cast<const FixedSizeBinaryType&>(*type).byte_width(), value->size());
+ ARROW_CHECK_EQ(checked_cast<const FixedSizeBinaryType&>(*type).byte_width(),
+ value->size());
}
Decimal128Scalar::Decimal128Scalar(const Decimal128& value,
@@ -82,8 +83,8 @@ FixedSizeListScalar::FixedSizeListScalar(const std::shared_ptr<Array>& value,
const std::shared_ptr<DataType>& type,
bool is_valid)
: Scalar{type, is_valid}, value(value) {
- DCHECK_EQ(value->length(),
- checked_cast<const FixedSizeListType*>(type.get())->list_size());
+ ARROW_CHECK_EQ(value->length(),
+ checked_cast<const FixedSizeListType*>(type.get())->list_size());
}
FixedSizeListScalar::FixedSizeListScalar(const std::shared_ptr<Array>& value,
diff --git a/cpp/src/arrow/sparse_tensor.cc b/cpp/src/arrow/sparse_tensor.cc
index fc2d538..d90392d 100644
--- a/cpp/src/arrow/sparse_tensor.cc
+++ b/cpp/src/arrow/sparse_tensor.cc
@@ -319,7 +319,7 @@ void MakeSparseTensorFromTensor(const Tensor& tensor,
// Constructor with a column-major NumericTensor
SparseCOOIndex::SparseCOOIndex(const std::shared_ptr<CoordsTensor>& coords)
: SparseIndexBase(coords->shape()[0]), coords_(coords) {
- DCHECK(coords_->is_column_major());
+ ARROW_CHECK(coords_->is_column_major());
}
std::string SparseCOOIndex::ToString() const { return std::string("SparseCOOIndex"); }
@@ -331,8 +331,8 @@ std::string SparseCOOIndex::ToString() const { return std::string("SparseCOOInde
SparseCSRIndex::SparseCSRIndex(const std::shared_ptr<IndexTensor>& indptr,
const std::shared_ptr<IndexTensor>& indices)
: SparseIndexBase(indices->shape()[0]), indptr_(indptr), indices_(indices) {
- DCHECK_EQ(1, indptr_->ndim());
- DCHECK_EQ(1, indices_->ndim());
+ ARROW_CHECK_EQ(1, indptr_->ndim());
+ ARROW_CHECK_EQ(1, indices_->ndim());
}
std::string SparseCSRIndex::ToString() const { return std::string("SparseCSRIndex"); }
@@ -351,7 +351,7 @@ SparseTensor::SparseTensor(const std::shared_ptr<DataType>& type,
shape_(shape),
sparse_index_(sparse_index),
dim_names_(dim_names) {
- DCHECK(is_tensor_supported(type->id()));
+ ARROW_CHECK(is_tensor_supported(type->id()));
}
const std::string& SparseTensor::dim_name(int i) const {
@@ -359,7 +359,7 @@ const std::string& SparseTensor::dim_name(int i) const {
if (dim_names_.size() == 0) {
return kEmpty;
} else {
- DCHECK_LT(i, static_cast<int>(dim_names_.size()));
+ ARROW_CHECK_LT(i, static_cast<int>(dim_names_.size()));
return dim_names_[i];
}
}
diff --git a/cpp/src/arrow/status.cc b/cpp/src/arrow/status.cc
index e97dc8c..cbb2911 100644
--- a/cpp/src/arrow/status.cc
+++ b/cpp/src/arrow/status.cc
@@ -15,11 +15,14 @@
#include <cassert>
#include <cstdlib>
#include <iostream>
+#include <sstream>
+
+#include "arrow/util/logging.h"
namespace arrow {
Status::Status(StatusCode code, const std::string& msg) {
- assert(code != StatusCode::OK);
+ ARROW_CHECK_NE(code, StatusCode::OK) << "Cannot construct ok status with message";
state_ = new State;
state_->code = code;
state_->msg = msg;
@@ -126,4 +129,13 @@ void Status::Abort(const std::string& message) const {
std::abort();
}
+#ifdef ARROW_EXTRA_ERROR_CONTEXT
+void Status::AddContextLine(const char* filename, int line, const char* expr) {
+ ARROW_CHECK(!ok()) << "Cannot add context line to ok status";
+ std::stringstream ss;
+ ss << "\nIn " << filename << ", line " << line << ", code: " << expr;
+ state_->msg += ss.str();
+}
+#endif
+
} // namespace arrow
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 790d9b7..1ed0da6 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -20,10 +20,6 @@
#include <string>
#include <utility>
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
-#include <sstream>
-#endif
-
#include "arrow/util/macros.h"
#include "arrow/util/string_builder.h"
#include "arrow/util/visibility.h"
@@ -31,14 +27,13 @@
#ifdef ARROW_EXTRA_ERROR_CONTEXT
/// \brief Return with given status if condition is met.
-#define ARROW_RETURN_IF_(condition, status, expr) \
- do { \
- if (ARROW_PREDICT_FALSE(condition)) { \
- ::arrow::Status _s = (status); \
- std::stringstream ss; \
- ss << _s.message() << "\n" << __FILE__ << ":" << __LINE__ << " code: " << expr; \
- return ::arrow::Status(_s.code(), ss.str()); \
- } \
+#define ARROW_RETURN_IF_(condition, status, expr) \
+ do { \
+ if (ARROW_PREDICT_FALSE(condition)) { \
+ ::arrow::Status _st = (status); \
+ _st.AddContextLine(__FILE__, __LINE__, expr); \
+ return _st; \
+ } \
} while (0)
#else
@@ -338,6 +333,10 @@ class ARROW_EXPORT Status {
[[noreturn]] void Abort() const;
[[noreturn]] void Abort(const std::string& message) const;
+#ifdef ARROW_EXTRA_ERROR_CONTEXT
+ void AddContextLine(const char* filename, int line, const char* expr);
+#endif
+
private:
struct State {
StatusCode code;
diff --git a/cpp/src/arrow/table.cc b/cpp/src/arrow/table.cc
index addb255..5c58adc 100644
--- a/cpp/src/arrow/table.cc
+++ b/cpp/src/arrow/table.cc
@@ -61,7 +61,7 @@ Status CompactColumn(const std::shared_ptr<Column>& column, MemoryPool* pool,
ChunkedArray::ChunkedArray(const ArrayVector& chunks) : chunks_(chunks) {
length_ = 0;
null_count_ = 0;
- DCHECK_GT(chunks.size(), 0)
+ ARROW_CHECK_GT(chunks.size(), 0)
<< "cannot construct ChunkedArray from empty vector and omitted type";
type_ = chunks[0]->type();
for (const std::shared_ptr<Array>& chunk : chunks) {
@@ -141,7 +141,7 @@ bool ChunkedArray::Equals(const std::shared_ptr<ChunkedArray>& other) const {
}
std::shared_ptr<ChunkedArray> ChunkedArray::Slice(int64_t offset, int64_t length) const {
- DCHECK_LE(offset, length_);
+ ARROW_CHECK_LE(offset, length_) << "Slice offset greater than array length";
int curr_chunk = 0;
while (curr_chunk < num_chunks() && offset >= chunk(curr_chunk)->length()) {
diff --git a/cpp/src/arrow/table_builder.cc b/cpp/src/arrow/table_builder.cc
index 21d158f..1ef8650 100644
--- a/cpp/src/arrow/table_builder.cc
+++ b/cpp/src/arrow/table_builder.cc
@@ -75,7 +75,7 @@ Status RecordBatchBuilder::Flush(std::shared_ptr<RecordBatch>* batch) {
}
void RecordBatchBuilder::SetInitialCapacity(int64_t capacity) {
- DCHECK_GT(capacity, 0) << "Initial capacity must be positive";
+ ARROW_CHECK_GT(capacity, 0) << "Initial capacity must be positive";
initial_capacity_ = capacity;
}
diff --git a/cpp/src/arrow/tensor.cc b/cpp/src/arrow/tensor.cc
index 743a9bc..575f9a0 100644
--- a/cpp/src/arrow/tensor.cc
+++ b/cpp/src/arrow/tensor.cc
@@ -78,7 +78,7 @@ Tensor::Tensor(const std::shared_ptr<DataType>& type, const std::shared_ptr<Buff
const std::vector<int64_t>& shape, const std::vector<int64_t>& strides,
const std::vector<std::string>& dim_names)
: type_(type), data_(data), shape_(shape), strides_(strides), dim_names_(dim_names) {
- DCHECK(is_tensor_supported(type->id()));
+ ARROW_CHECK(is_tensor_supported(type->id()));
if (shape.size() > 0 && strides.size() == 0) {
ComputeRowMajorStrides(checked_cast<const FixedWidthType&>(*type_), shape, &strides_);
}
@@ -97,7 +97,7 @@ const std::string& Tensor::dim_name(int i) const {
if (dim_names_.size() == 0) {
return kEmpty;
} else {
- DCHECK_LT(i, static_cast<int>(dim_names_.size()));
+ ARROW_CHECK_LT(i, static_cast<int>(dim_names_.size()));
return dim_names_[i];
}
}
@@ -172,7 +172,7 @@ struct NonZeroCounter {
template <typename TYPE>
typename std::enable_if<!is_number_type<TYPE>::value, Status>::type Visit(
const TYPE& type) {
- DCHECK(!is_tensor_supported(type.id()));
+ ARROW_CHECK(!is_tensor_supported(type.id()));
return Status::NotImplemented("Tensor of ", type.ToString(), " is not implemented");
}
diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc
index b3fcb0c..2dbd31a 100644
--- a/cpp/src/arrow/type.cc
+++ b/cpp/src/arrow/type.cc
@@ -208,7 +208,7 @@ TimeType::TimeType(Type::type type_id, TimeUnit::type unit)
: TemporalType(type_id), unit_(unit) {}
Time32Type::Time32Type(TimeUnit::type unit) : TimeType(Type::TIME32, unit) {
- DCHECK(unit == TimeUnit::SECOND || unit == TimeUnit::MILLI)
+ ARROW_CHECK(unit == TimeUnit::SECOND || unit == TimeUnit::MILLI)
<< "Must be seconds or milliseconds";
}
@@ -219,7 +219,7 @@ std::string Time32Type::ToString() const {
}
Time64Type::Time64Type(TimeUnit::type unit) : TimeType(Type::TIME64, unit) {
- DCHECK(unit == TimeUnit::MICRO || unit == TimeUnit::NANO)
+ ARROW_CHECK(unit == TimeUnit::MICRO || unit == TimeUnit::NANO)
<< "Must be microseconds or nanoseconds";
}
@@ -407,8 +407,8 @@ int StructType::GetChildIndex(const std::string& name) const {
Decimal128Type::Decimal128Type(int32_t precision, int32_t scale)
: DecimalType(16, precision, scale) {
- DCHECK_GE(precision, 1);
- DCHECK_LE(precision, 38);
+ ARROW_CHECK_GE(precision, 1);
+ ARROW_CHECK_LE(precision, 38);
}
// ----------------------------------------------------------------------
@@ -424,10 +424,10 @@ DictionaryType::DictionaryType(const std::shared_ptr<DataType>& index_type,
index_type_(index_type),
value_type_(value_type),
ordered_(ordered) {
-#ifndef NDEBUG
+ ARROW_CHECK(is_integer(index_type->id()))
+ << "dictionary index type should be signed integer";
const auto& int_type = checked_cast<const IntegerType&>(*index_type);
- DCHECK_EQ(int_type.is_signed(), true) << "dictionary index type should be signed";
-#endif
+ ARROW_CHECK(int_type.is_signed()) << "dictionary index type should be signed integer";
}
DataTypeLayout DictionaryType::layout() const {
diff --git a/cpp/src/arrow/util/compression_bz2.cc b/cpp/src/arrow/util/compression_bz2.cc
index 407fb82..a26bb3e 100644
--- a/cpp/src/arrow/util/compression_bz2.cc
+++ b/cpp/src/arrow/util/compression_bz2.cc
@@ -44,8 +44,8 @@ static constexpr auto kSizeLimit =
static_cast<int64_t>(std::numeric_limits<unsigned int>::max());
Status BZ2Error(const char* prefix_msg, int bz_result) {
- DCHECK(bz_result != BZ_OK && bz_result != BZ_RUN_OK && bz_result != BZ_FLUSH_OK &&
- bz_result != BZ_FINISH_OK && bz_result != BZ_STREAM_END);
+ ARROW_CHECK(bz_result != BZ_OK && bz_result != BZ_RUN_OK && bz_result != BZ_FLUSH_OK &&
+ bz_result != BZ_FINISH_OK && bz_result != BZ_STREAM_END);
StatusCode code;
std::stringstream ss;
ss << prefix_msg;
diff --git a/cpp/src/arrow/util/compression_zlib.cc b/cpp/src/arrow/util/compression_zlib.cc
index 5afd5e3..cd8b912 100644
--- a/cpp/src/arrow/util/compression_zlib.cc
+++ b/cpp/src/arrow/util/compression_zlib.cc
@@ -131,7 +131,7 @@ class GZipDecompressor : public Decompressor {
*bytes_written = 0;
*need_more_output = true;
} else {
- DCHECK(ret == Z_OK || ret == Z_STREAM_END);
+ ARROW_CHECK(ret == Z_OK || ret == Z_STREAM_END);
// Some progress has been made
*bytes_read = input_len - stream_.avail_in;
*bytes_written = output_len - stream_.avail_out;
@@ -224,7 +224,7 @@ Status GZipCompressor::Compress(int64_t input_len, const uint8_t* input,
*bytes_written = output_len - stream_.avail_out;
} else {
// No progress was possible
- DCHECK_EQ(ret, Z_BUF_ERROR);
+ ARROW_CHECK_EQ(ret, Z_BUF_ERROR);
*bytes_read = 0;
*bytes_written = 0;
}
@@ -250,7 +250,7 @@ Status GZipCompressor::Flush(int64_t output_len, uint8_t* output, int64_t* bytes
if (ret == Z_OK) {
*bytes_written = output_len - stream_.avail_out;
} else {
- DCHECK_EQ(ret, Z_BUF_ERROR);
+ ARROW_CHECK_EQ(ret, Z_BUF_ERROR);
*bytes_written = 0;
}
// "If deflate returns with avail_out == 0, this function must be called
@@ -428,7 +428,7 @@ class GZipCodec::GZipCodecImpl {
// Must be in compression mode
if (!compressor_initialized_) {
Status s = InitCompressor();
- DCHECK_OK(s);
+ ARROW_CHECK_OK(s);
}
int64_t max_len = deflateBound(&stream_, static_cast<uLong>(input_length));
// ARROW-3514: return a more pessimistic estimate to account for bugs
diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc
index ec659e3..46928b2 100644
--- a/cpp/src/arrow/util/decimal.cc
+++ b/cpp/src/arrow/util/decimal.cc
@@ -42,7 +42,7 @@ using internal::SafeSignedAdd;
Decimal128::Decimal128(const std::string& str) : Decimal128() {
Status status = Decimal128::FromString(str, this);
- DCHECK_OK(status);
+ ARROW_CHECK_OK(status);
}
static const Decimal128 kTenTo36(static_cast<int64_t>(0xC097CE7BC90715),
@@ -56,7 +56,7 @@ std::string Decimal128::ToIntegerString() const {
// get anything above 10 ** 36 and print it
Decimal128 top;
- DCHECK_OK(Divide(kTenTo36, &top, &remainder));
+ ARROW_CHECK_OK(Divide(kTenTo36, &top, &remainder));
if (top != 0) {
buf << static_cast<int64_t>(top);
diff --git a/cpp/src/arrow/util/io-util.cc b/cpp/src/arrow/util/io-util.cc
index 1879110..87f64b7 100644
--- a/cpp/src/arrow/util/io-util.cc
+++ b/cpp/src/arrow/util/io-util.cc
@@ -866,7 +866,12 @@ Status DelEnvVar(const std::string& name) { return DelEnvVar(name.c_str()); }
TemporaryDir::TemporaryDir(PlatformFilename&& path) : path_(std::move(path)) {}
-TemporaryDir::~TemporaryDir() { DCHECK_OK(DeleteDirTree(path_)); }
+TemporaryDir::~TemporaryDir() {
+ Status st = DeleteDirTree(path_);
+ if (!st.ok()) {
+ ARROW_LOG(WARNING) << "When trying to delete temporary directory: " << st;
+ }
+}
Status TemporaryDir::Make(const std::string& prefix, std::unique_ptr<TemporaryDir>* out) {
bfs::path path;
diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc
index 0b22403..1c73e30 100644
--- a/cpp/src/arrow/util/key_value_metadata.cc
+++ b/cpp/src/arrow/util/key_value_metadata.cc
@@ -57,13 +57,13 @@ KeyValueMetadata::KeyValueMetadata() : keys_(), values_() {}
KeyValueMetadata::KeyValueMetadata(
const std::unordered_map<std::string, std::string>& map)
: keys_(UnorderedMapKeys(map)), values_(UnorderedMapValues(map)) {
- DCHECK_EQ(keys_.size(), values_.size());
+ ARROW_CHECK_EQ(keys_.size(), values_.size());
}
KeyValueMetadata::KeyValueMetadata(const std::vector<std::string>& keys,
const std::vector<std::string>& values)
: keys_(keys), values_(values) {
- DCHECK_EQ(keys.size(), values.size());
+ ARROW_CHECK_EQ(keys.size(), values.size());
}
void KeyValueMetadata::ToUnorderedMap(
diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h
index 999aca6..a920db1 100644
--- a/cpp/src/arrow/util/logging.h
+++ b/cpp/src/arrow/util/logging.h
@@ -59,25 +59,34 @@ enum class ArrowLogLevel : int {
#define ARROW_IGNORE_EXPR(expr) ((void)(expr))
-#define ARROW_CHECK(condition) \
- (condition) ? ARROW_IGNORE_EXPR(0) \
- : ::arrow::util::Voidify() & \
- ::arrow::util::ArrowLog(__FILE__, __LINE__, \
- ::arrow::util::ArrowLogLevel::ARROW_FATAL) \
- << " Check failed: " #condition " "
+#define ARROW_CHECK(condition) \
+ ARROW_PREDICT_TRUE(condition) \
+ ? ARROW_IGNORE_EXPR(0) \
+ : ::arrow::util::Voidify() & \
+ ::arrow::util::ArrowLog(__FILE__, __LINE__, \
+ ::arrow::util::ArrowLogLevel::ARROW_FATAL) \
+ << " Check failed: " #condition " "
// If 'to_call' returns a bad status, CHECK immediately with a logged message
// of 'msg' followed by the status.
-#define ARROW_CHECK_OK_PREPEND(to_call, msg) \
- do { \
- ::arrow::Status _s = (to_call); \
- ARROW_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \
+#define ARROW_CHECK_OK_PREPEND(to_call, msg) \
+ do { \
+ ::arrow::Status _s = (to_call); \
+ ARROW_CHECK(_s.ok()) << "Operation failed: " << ARROW_STRINGIFY(to_call) << "\n" \
+ << (msg) << ": " << _s.ToString(); \
} while (false)
// If the status is bad, CHECK immediately, appending the status to the
// logged message.
#define ARROW_CHECK_OK(s) ARROW_CHECK_OK_PREPEND(s, "Bad status")
+#define ARROW_CHECK_EQ(val1, val2) ARROW_CHECK((val1) == (val2))
+#define ARROW_CHECK_NE(val1, val2) ARROW_CHECK((val1) != (val2))
+#define ARROW_CHECK_LE(val1, val2) ARROW_CHECK((val1) <= (val2))
+#define ARROW_CHECK_LT(val1, val2) ARROW_CHECK((val1) < (val2))
+#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
+#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
+
#ifdef NDEBUG
#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
@@ -118,14 +127,14 @@ enum class ArrowLogLevel : int {
#else
#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_FATAL
-#define DCHECK(condition) ARROW_CHECK(condition)
-#define DCHECK_OK(status) ARROW_CHECK_OK(status)
-#define DCHECK_EQ(val1, val2) ARROW_CHECK((val1) == (val2))
-#define DCHECK_NE(val1, val2) ARROW_CHECK((val1) != (val2))
-#define DCHECK_LE(val1, val2) ARROW_CHECK((val1) <= (val2))
-#define DCHECK_LT(val1, val2) ARROW_CHECK((val1) < (val2))
-#define DCHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
-#define DCHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
+#define DCHECK ARROW_CHECK
+#define DCHECK_OK ARROW_CHECK_OK
+#define DCHECK_EQ ARROW_CHECK_EQ
+#define DCHECK_NE ARROW_CHECK_NE
+#define DCHECK_LE ARROW_CHECK_LE
+#define DCHECK_LT ARROW_CHECK_LT
+#define DCHECK_GE ARROW_CHECK_GE
+#define DCHECK_GT ARROW_CHECK_GT
#endif // NDEBUG
diff --git a/cpp/src/arrow/util/thread-pool.cc b/cpp/src/arrow/util/thread-pool.cc
index 6969f3f..81152bc 100644
--- a/cpp/src/arrow/util/thread-pool.cc
+++ b/cpp/src/arrow/util/thread-pool.cc
@@ -289,7 +289,7 @@ int ThreadPool::DefaultCapacity() {
// Helper for the singleton pattern
std::shared_ptr<ThreadPool> ThreadPool::MakeCpuThreadPool() {
std::shared_ptr<ThreadPool> pool;
- DCHECK_OK(ThreadPool::Make(ThreadPool::DefaultCapacity(), &pool));
+ ARROW_CHECK_OK(ThreadPool::Make(ThreadPool::DefaultCapacity(), &pool));
// On Windows, the global ThreadPool destructor may be called after
// non-main threads have been killed by the OS, and hang in a condition
// variable.