Skip to content

Commit b218a7f

Browse files
pitroufsaintjacques
pitrou
authored and
fsaintjacques
committed
ARROW-7240: [C++] Add Result<T> to APIs to arrow/util
Excluding Decimal, which has its own JIRA (ARROW-7274). Closes apache#5920 from pitrou/ARROW-7240-util-apis-result and squashes the following commits: c1bf8cc <Antoine Pitrou> ARROW-7240: Add Result<T> to APIs to arrow/util Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: François Saint-Jacques <fsaintjacques@gmail.com>
1 parent c6bec15 commit b218a7f

74 files changed

Lines changed: 1283 additions & 1511 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cpp/build-support/lint_cpp_cli.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def lint_files():
109109
# Lint file name, except for pkg-config templates
110110
if not filename.endswith('.pc.in'):
111111
if '-' in filename:
112-
why = ("Please user underscores, not hyphens, "
112+
why = ("Please use underscores, not hyphens, "
113113
"in source file names")
114114
yield full_path, why, 0, full_path
115115

‎cpp/src/arrow/array.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -661,18 +661,20 @@ Status StructArray::Flatten(MemoryPool* pool, ArrayVector* out) const {
661661
// The validity of a flattened datum is the logical AND of the struct
662662
// element's validity and the individual field element's validity.
663663
if (null_bitmap && child_null_bitmap) {
664-
RETURN_NOT_OK(BitmapAnd(pool, child_null_bitmap->data(), child_offset,
665-
null_bitmap_data_, data_->offset, data_->length,
666-
child_offset, &flattened_null_bitmap));
664+
ARROW_ASSIGN_OR_RAISE(
665+
flattened_null_bitmap,
666+
BitmapAnd(pool, child_null_bitmap->data(), child_offset, null_bitmap_data_,
667+
data_->offset, data_->length, child_offset));
667668
} else if (child_null_bitmap) {
668669
flattened_null_bitmap = child_null_bitmap;
669670
flattened_null_count = child_data->null_count;
670671
} else if (null_bitmap) {
671672
if (child_offset == data_->offset) {
672673
flattened_null_bitmap = null_bitmap;
673674
} else {
674-
RETURN_NOT_OK(CopyBitmap(pool, null_bitmap_data_, data_->offset, data_->length,
675-
&flattened_null_bitmap));
675+
ARROW_ASSIGN_OR_RAISE(
676+
flattened_null_bitmap,
677+
CopyBitmap(pool, null_bitmap_data_, data_->offset, data_->length));
676678
}
677679
flattened_null_count = data_->null_count;
678680
} else {

‎cpp/src/arrow/array/dict_internal.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ Status DictionaryArray::Transpose(MemoryPool* pool, const std::shared_ptr<DataTy
204204
// Shift null buffer if the original offset is non-zero
205205
std::shared_ptr<Buffer> null_bitmap;
206206
if (data_->offset != 0) {
207-
RETURN_NOT_OK(
208-
CopyBitmap(pool, null_bitmap_data_, data_->offset, data_->length, &null_bitmap));
207+
ARROW_ASSIGN_OR_RAISE(
208+
null_bitmap, CopyBitmap(pool, null_bitmap_data_, data_->offset, data_->length));
209209
} else {
210210
null_bitmap = data_->buffers[0];
211211
}

‎cpp/src/arrow/array_binary_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class TestStringArray : public ::testing::Test {
9191
length_ = static_cast<int64_t>(offsets_.size()) - 1;
9292
value_buf_ = Buffer::Wrap(chars_);
9393
offsets_buf_ = Buffer::Wrap(offsets_);
94-
ASSERT_OK(BitUtil::BytesToBits(valid_bytes_, default_memory_pool(), &null_bitmap_));
94+
ASSERT_OK_AND_ASSIGN(null_bitmap_, BitUtil::BytesToBits(valid_bytes_));
9595
null_count_ = CountNulls(valid_bytes_);
9696

9797
strings_ = std::make_shared<ArrayType>(length_, offsets_buf_, value_buf_,

‎cpp/src/arrow/array_list_test.cc‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,7 @@ TEST_F(TestMapArray, BuildingStringToInt) {
496496
std::vector<int32_t> offsets = {0, 2, 2, 3, 3};
497497
auto expected_keys = ArrayFromJSON(utf8(), R"(["joe", "mark", "cap"])");
498498
auto expected_values = ArrayFromJSON(int32(), "[0, null, 8]");
499-
std::shared_ptr<Buffer> expected_null_bitmap;
500-
ASSERT_OK(
501-
BitUtil::BytesToBits({1, 0, 1, 1}, default_memory_pool(), &expected_null_bitmap));
499+
ASSERT_OK_AND_ASSIGN(auto expected_null_bitmap, BitUtil::BytesToBits({1, 0, 1, 1}));
502500
MapArray expected(type, 4, Buffer::Wrap(offsets), expected_keys, expected_values,
503501
expected_null_bitmap, 1);
504502

‎cpp/src/arrow/array_test.cc‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ Status MakeArrayFromValidBytes(const std::vector<uint8_t>& v, MemoryPool* pool,
9090
std::shared_ptr<Array>* out) {
9191
int64_t null_count = v.size() - std::accumulate(v.begin(), v.end(), 0);
9292

93-
std::shared_ptr<Buffer> null_buf;
94-
RETURN_NOT_OK(BitUtil::BytesToBits(v, default_memory_pool(), &null_buf));
93+
ARROW_ASSIGN_OR_RAISE(auto null_buf, BitUtil::BytesToBits(v));
9594

9695
TypedBufferBuilder<int32_t> value_builder(pool);
9796
for (size_t i = 0; i < v.size(); ++i) {
@@ -195,8 +194,7 @@ TEST_F(TestArray, TestIsNullIsValid) {
195194
}
196195
}
197196

198-
std::shared_ptr<Buffer> null_buf;
199-
ASSERT_OK(BitUtil::BytesToBits(null_bitmap, default_memory_pool(), &null_buf));
197+
ASSERT_OK_AND_ASSIGN(auto null_buf, BitUtil::BytesToBits(null_bitmap));
200198

201199
std::unique_ptr<Array> arr;
202200
arr.reset(new Int32Array(null_bitmap.size(), nullptr, null_buf, null_count));
@@ -388,8 +386,7 @@ class TestPrimitiveBuilder : public TestBuilder {
388386
int64_t ex_null_count = 0;
389387

390388
if (nullable) {
391-
ASSERT_OK(
392-
BitUtil::BytesToBits(valid_bytes_, default_memory_pool(), &ex_null_bitmap));
389+
ASSERT_OK_AND_ASSIGN(ex_null_bitmap, BitUtil::BytesToBits(valid_bytes_));
393390
ex_null_count = CountNulls(valid_bytes_);
394391
} else {
395392
ex_null_bitmap = nullptr;
@@ -511,9 +508,9 @@ void TestPrimitiveBuilder<PBoolean>::Check(const std::unique_ptr<BooleanBuilder>
511508
std::shared_ptr<Buffer> ex_null_bitmap;
512509
int64_t ex_null_count = 0;
513510

514-
ASSERT_OK(BitUtil::BytesToBits(draws_, default_memory_pool(), &ex_data));
511+
ASSERT_OK_AND_ASSIGN(ex_data, BitUtil::BytesToBits(draws_));
515512
if (nullable) {
516-
ASSERT_OK(BitUtil::BytesToBits(valid_bytes_, default_memory_pool(), &ex_null_bitmap));
513+
ASSERT_OK_AND_ASSIGN(ex_null_bitmap, BitUtil::BytesToBits(valid_bytes_));
517514
ex_null_count = CountNulls(valid_bytes_);
518515
} else {
519516
ex_null_bitmap = nullptr;
@@ -1911,8 +1908,7 @@ class DecimalTest : public ::testing::TestWithParam<int> {
19111908

19121909
auto expected_data = std::make_shared<Buffer>(raw_bytes.data(), BYTE_WIDTH);
19131910
std::shared_ptr<Buffer> expected_null_bitmap;
1914-
ASSERT_OK(
1915-
BitUtil::BytesToBits(valid_bytes, default_memory_pool(), &expected_null_bitmap));
1911+
ASSERT_OK_AND_ASSIGN(expected_null_bitmap, BitUtil::BytesToBits(valid_bytes));
19161912

19171913
int64_t expected_null_count = CountNulls(valid_bytes);
19181914
auto expected = std::make_shared<Decimal128Array>(

‎cpp/src/arrow/compute/kernels/hash.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
#include "arrow/compute/kernels/util_internal.h"
3838
#include "arrow/type.h"
3939
#include "arrow/type_traits.h"
40-
#include "arrow/util/bit_util.h"
4140
#include "arrow/util/checked_cast.h"
4241
#include "arrow/util/hashing.h"
4342
#include "arrow/util/logging.h"
@@ -591,6 +590,7 @@ const char kValuesFieldName[] = "values";
591590
const char kCountsFieldName[] = "counts";
592591
const int32_t kValuesFieldIndex = 0;
593592
const int32_t kCountsFieldIndex = 1;
593+
594594
Status ValueCounts(FunctionContext* ctx, const Datum& value,
595595
std::shared_ptr<Array>* counts) {
596596
std::unique_ptr<HashKernel> func;
@@ -612,6 +612,8 @@ Status ValueCounts(FunctionContext* ctx, const Datum& value,
612612
std::vector<std::shared_ptr<Array>>{uniques, MakeArray(value_counts.array())});
613613
return Status::OK();
614614
}
615+
615616
#undef PROCESS_SUPPORTED_HASH_TYPES
617+
616618
} // namespace compute
617619
} // namespace arrow

‎cpp/src/arrow/compute/kernels/util_internal.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ Status AssignNullIntersection(FunctionContext* ctx, const ArrayData& left,
269269
}
270270

271271
if (left.GetNullCount() > 0 && right.GetNullCount() > 0) {
272-
RETURN_NOT_OK(BitmapAnd(ctx->memory_pool(), left.buffers[0]->data(), left.offset,
273-
right.buffers[0]->data(), right.offset, right.length, 0,
274-
&(output->buffers[0])));
272+
ARROW_ASSIGN_OR_RAISE(
273+
output->buffers[0],
274+
BitmapAnd(ctx->memory_pool(), left.buffers[0]->data(), left.offset,
275+
right.buffers[0]->data(), right.offset, right.length, 0));
275276
// Force computation of null count.
276277
output->null_count = kUnknownNullCount;
277278
output->GetNullCount();

‎cpp/src/arrow/csv/reader.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ class BaseTableReader : public csv::TableReader {
8585
}
8686
int64_t offset = 0;
8787
if (first_block) {
88-
const uint8_t* data;
89-
RETURN_NOT_OK(util::SkipUTF8BOM(buf->data(), buf->size(), &data));
88+
ARROW_ASSIGN_OR_RAISE(auto data, util::SkipUTF8BOM(buf->data(), buf->size()));
9089
offset += data - buf->data();
9190
DCHECK_GE(offset, 0);
9291
}

‎cpp/src/arrow/filesystem/localfs.cc‎

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -173,20 +173,19 @@ Result<FileStats> StatFile(const std::string& path) {
173173

174174
Status StatSelector(const PlatformFilename& dir_fn, const Selector& select,
175175
int32_t nesting_depth, std::vector<FileStats>* out) {
176-
std::vector<PlatformFilename> children;
177-
Status status = ListDir(dir_fn, &children);
178-
if (!status.ok()) {
176+
auto result = ListDir(dir_fn);
177+
if (!result.ok()) {
178+
auto status = result.status();
179179
if (select.allow_non_existent && status.IsIOError()) {
180-
bool exists;
181-
RETURN_NOT_OK(FileExists(dir_fn, &exists));
180+
ARROW_ASSIGN_OR_RAISE(bool exists, FileExists(dir_fn));
182181
if (!exists) {
183182
return Status::OK();
184183
}
185184
}
186185
return status;
187186
}
188187

189-
for (const auto& child_fn : children) {
188+
for (const auto& child_fn : *result) {
190189
PlatformFilename full_fn = dir_fn.Join(child_fn);
191190
ARROW_ASSIGN_OR_RAISE(FileStats st, StatFile(full_fn.ToNative()));
192191
if (st.type() != FileType::NonExistent) {
@@ -214,34 +213,29 @@ LocalFileSystem::LocalFileSystem(const LocalFileSystemOptions& options)
214213
LocalFileSystem::~LocalFileSystem() {}
215214

216215
Result<FileStats> LocalFileSystem::GetTargetStats(const std::string& path) {
217-
PlatformFilename fn;
218-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
216+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
219217
return StatFile(fn.ToNative());
220218
}
221219

222220
Result<std::vector<FileStats>> LocalFileSystem::GetTargetStats(const Selector& select) {
223-
PlatformFilename fn;
224-
RETURN_NOT_OK(PlatformFilename::FromString(select.base_dir, &fn));
221+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(select.base_dir));
225222
std::vector<FileStats> results;
226223
RETURN_NOT_OK(StatSelector(fn, select, 0, &results));
227224
return results;
228225
}
229226

230227
Status LocalFileSystem::CreateDir(const std::string& path, bool recursive) {
231-
PlatformFilename fn;
232-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
228+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
233229
if (recursive) {
234-
return ::arrow::internal::CreateDirTree(fn);
230+
return ::arrow::internal::CreateDirTree(fn).status();
235231
} else {
236-
return ::arrow::internal::CreateDir(fn);
232+
return ::arrow::internal::CreateDir(fn).status();
237233
}
238234
}
239235

240236
Status LocalFileSystem::DeleteDir(const std::string& path) {
241-
bool deleted = false;
242-
PlatformFilename fn;
243-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
244-
RETURN_NOT_OK(::arrow::internal::DeleteDirTree(fn, &deleted));
237+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
238+
ARROW_ASSIGN_OR_RAISE(bool deleted, ::arrow::internal::DeleteDirTree(fn));
245239
if (deleted) {
246240
return Status::OK();
247241
} else {
@@ -250,10 +244,8 @@ Status LocalFileSystem::DeleteDir(const std::string& path) {
250244
}
251245

252246
Status LocalFileSystem::DeleteDirContents(const std::string& path) {
253-
bool deleted = false;
254-
PlatformFilename fn;
255-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
256-
RETURN_NOT_OK(::arrow::internal::DeleteDirContents(fn, &deleted));
247+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
248+
ARROW_ASSIGN_OR_RAISE(bool deleted, ::arrow::internal::DeleteDirContents(fn));
257249
if (deleted) {
258250
return Status::OK();
259251
} else {
@@ -262,10 +254,8 @@ Status LocalFileSystem::DeleteDirContents(const std::string& path) {
262254
}
263255

264256
Status LocalFileSystem::DeleteFile(const std::string& path) {
265-
bool deleted = false;
266-
PlatformFilename fn;
267-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
268-
RETURN_NOT_OK(::arrow::internal::DeleteFile(fn, &deleted));
257+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
258+
ARROW_ASSIGN_OR_RAISE(bool deleted, arrow::internal::DeleteFile(fn));
269259
if (deleted) {
270260
return Status::OK();
271261
} else {
@@ -274,9 +264,8 @@ Status LocalFileSystem::DeleteFile(const std::string& path) {
274264
}
275265

276266
Status LocalFileSystem::Move(const std::string& src, const std::string& dest) {
277-
PlatformFilename sfn, dfn;
278-
RETURN_NOT_OK(PlatformFilename::FromString(src, &sfn));
279-
RETURN_NOT_OK(PlatformFilename::FromString(dest, &dfn));
267+
ARROW_ASSIGN_OR_RAISE(auto sfn, PlatformFilename::FromString(src));
268+
ARROW_ASSIGN_OR_RAISE(auto dfn, PlatformFilename::FromString(dest));
280269

281270
#ifdef _WIN32
282271
if (!MoveFileExW(sfn.ToNative().c_str(), dfn.ToNative().c_str(),
@@ -294,9 +283,8 @@ Status LocalFileSystem::Move(const std::string& src, const std::string& dest) {
294283
}
295284

296285
Status LocalFileSystem::CopyFile(const std::string& src, const std::string& dest) {
297-
PlatformFilename sfn, dfn;
298-
RETURN_NOT_OK(PlatformFilename::FromString(src, &sfn));
299-
RETURN_NOT_OK(PlatformFilename::FromString(dest, &dfn));
286+
ARROW_ASSIGN_OR_RAISE(auto sfn, PlatformFilename::FromString(src));
287+
ARROW_ASSIGN_OR_RAISE(auto dfn, PlatformFilename::FromString(dest));
300288
// XXX should we use fstat() to compare inodes?
301289
if (sfn.ToNative() == dfn.ToNative()) {
302290
return Status::OK();
@@ -351,12 +339,11 @@ namespace {
351339
Result<std::shared_ptr<io::OutputStream>> OpenOutputStreamGeneric(const std::string& path,
352340
bool truncate,
353341
bool append) {
354-
PlatformFilename fn;
355342
int fd;
356343
bool write_only = true;
357-
RETURN_NOT_OK(PlatformFilename::FromString(path, &fn));
358-
RETURN_NOT_OK(
359-
::arrow::internal::FileOpenWritable(fn, write_only, truncate, append, &fd));
344+
ARROW_ASSIGN_OR_RAISE(auto fn, PlatformFilename::FromString(path));
345+
ARROW_ASSIGN_OR_RAISE(
346+
fd, ::arrow::internal::FileOpenWritable(fn, write_only, truncate, append));
360347
std::shared_ptr<io::OutputStream> stream;
361348
Status st = io::FileOutputStream::Open(fd, &stream);
362349
if (!st.ok()) {

0 commit comments

Comments
 (0)