Skip to content

Commit

Permalink
ARROW-4800: [C++] Introduce a Result<T> class
Browse files Browse the repository at this point in the history
- Mostly an adaptation of StatusOr from google/asylo (both header and unittests).
- Demonstrate usage in ipc/writer*
- If this PR is accepted I can do a follow-up PR to port over useful testing utilities.

Author: Micah Kornfield <[email protected]>
Author: emkornfield <[email protected]>

Closes apache#4501 from emkornfield/error_or and squashes the following commits:

82e48c4 <Micah Kornfield> fix linter.  Add unittest.
aad79b1 <Micah Kornfield> rename to Return
1d7dbfb <Micah Kornfield> Use bkietz's suggestion.  cleanup test
d8e8043 <Micah Kornfield> fix compile errors
cc62607 <Micah Kornfield> try non anonyous namespace
86e43ac <Micah Kornfield> export before
8a4b3cc <Micah Kornfield> try explicit instantation for msvc
f12f6d0 <Micah Kornfield> Revert "remove ARROW_EXPORT from test and try add link to gtest_main"
9581b05 <Micah Kornfield> remove ARROW_EXPORT from test and try add link to gtest_main
7a21e57 <Micah Kornfield> try exporting private test classes for appveyor
0b44389 <Micah Kornfield> fix format
de9d2d0 <Micah Kornfield> remove duplicate code.  fix format
504fcd7 <emkornfield> Update cpp/src/arrow/error_or.h
31d9906 <Micah Kornfield> use vendored variant
aa540da <Micah Kornfield> fix append
6f459a5 <Micah Kornfield> address review comments
7a1e54d <Micah Kornfield> Add Arrow export
2886733 <Micah Kornfield> use ARROW_RETURN_NOT_OK
f7ed04f <Micah Kornfield> address comments
3e2b369 <Micah Kornfield> follow recommendation of docs for macro
d5e43d0 <Micah Kornfield> ARROW-4800: Introduce an ErrorOr class
  • Loading branch information
emkornfield authored and wesm committed Jun 22, 2019
1 parent 1515fe1 commit 7e16ee4
Show file tree
Hide file tree
Showing 10 changed files with 1,040 additions and 9 deletions.
51 changes: 51 additions & 0 deletions LICENSE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -882,3 +882,54 @@ This project include code from mingw-w64.
Copyright (c) 2009 - 2013 by the mingw-w64 project
Homepage: https://mingw-w64.org
License: Zope Public License (ZPL) Version 2.1.

---------------------------------------------------------------------------------

This project include code from Google's Asylo project.

* cpp/src/arrow/result.h is based on status_or.h

Copyright (c) Copyright 2017 Asylo authors
Homepage: https://asylo.dev/
License: Apache 2.0

---------------------------------------------------------------------------------

This project includes code from Google's protobuf project

* cpp/src/arrow/result.h ARROW_ASSIGN_OR_RAISE is based off ASSIGN_OR_RETURN

Copyright 2008 Google Inc. All rights reserved.
Homepage: https://developers.google.com/protocol-buffers/
License:

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:

* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following disclaimer
in the documentation and/or other materials provided with the
distribution.
* Neither the name of Google Inc. nor the names of its
contributors may be used to endorse or promote products derived from
this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Code generated by the Protocol Buffer compiler is owned by the owner
of the input file used when generating it. This code is not
standalone and requires a support library to be linked with it. This
support library is itself covered by the above license.
9 changes: 8 additions & 1 deletion cpp/build-support/lint_cpp_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
_STRIP_COMMENT_REGEX = re.compile('(.+)?(?=//)')
_NULLPTR_REGEX = re.compile(r'.*\bnullptr\b.*')
_RETURN_NOT_OK_REGEX = re.compile(r'.*\sRETURN_NOT_OK.*')
_ASSIGN_OR_RAISE_REGEX = re.compile(r'.*\sASSIGN_OR_RAISE.*')


def _paths(paths):
Expand All @@ -54,7 +55,13 @@ def lint_file(path):
arrow/status.h
test
arrow/util/hash.h
arrow/python/util'''))
arrow/python/util''')),
(lambda x: re.match(_ASSIGN_OR_RAISE_REGEX, x),
'Use ARROW_ASSIGN_OR_RAISE in header files', _paths('''\
arrow/result_internal.h
test
'''))

]

with open(path) as f:
Expand Down
2 changes: 2 additions & 0 deletions cpp/src/arrow/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ set(ARROW_SRCS
memory_pool.cc
pretty_print.cc
record_batch.cc
result.cc
scalar.cc
sparse_tensor.cc
status.cc
Expand Down Expand Up @@ -360,6 +361,7 @@ endif()
add_arrow_test(memory_pool-test)
add_arrow_test(pretty_print-test)
add_arrow_test(public-api-test)
add_arrow_test(result-test)
add_arrow_test(scalar-test)
add_arrow_test(status-test)
add_arrow_test(stl-test)
Expand Down
33 changes: 25 additions & 8 deletions cpp/src/arrow/ipc/writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "arrow/ipc/util.h"
#include "arrow/memory_pool.h"
#include "arrow/record_batch.h"
#include "arrow/result_internal.h"
#include "arrow/sparse_tensor.h"
#include "arrow/status.h"
#include "arrow/table.h"
Expand Down Expand Up @@ -593,8 +594,8 @@ Status WriteRecordBatch(const RecordBatch& batch, int64_t buffer_start_offset,

Status WriteRecordBatchStream(const std::vector<std::shared_ptr<RecordBatch>>& batches,
io::OutputStream* dst) {
std::shared_ptr<RecordBatchWriter> writer;
RETURN_NOT_OK(RecordBatchStreamWriter::Open(dst, batches[0]->schema(), &writer));
ASSIGN_OR_RAISE(std::shared_ptr<RecordBatchWriter> writer,
RecordBatchStreamWriter::Open(dst, batches[0]->schema()));
for (const auto& batch : batches) {
// allow sizes > INT32_MAX
DCHECK(batch->schema()->Equals(*batches[0]->schema())) << "Schemas unequal";
Expand Down Expand Up @@ -1159,11 +1160,16 @@ void RecordBatchStreamWriter::set_memory_pool(MemoryPool* pool) {
Status RecordBatchStreamWriter::Open(io::OutputStream* sink,
const std::shared_ptr<Schema>& schema,
std::shared_ptr<RecordBatchWriter>* out) {
ASSIGN_OR_RAISE(*out, Open(sink, schema));
return Status::OK();
}

Result<std::shared_ptr<RecordBatchWriter>> RecordBatchStreamWriter::Open(
io::OutputStream* sink, const std::shared_ptr<Schema>& schema) {
// ctor is private
auto result = std::shared_ptr<RecordBatchStreamWriter>(new RecordBatchStreamWriter());
result->impl_.reset(new RecordBatchStreamWriterImpl(sink, schema));
*out = result;
return Status::OK();
return std::move(result);
}

Status RecordBatchStreamWriter::Close() { return impl_->Close(); }
Expand All @@ -1175,11 +1181,16 @@ RecordBatchFileWriter::~RecordBatchFileWriter() {}
Status RecordBatchFileWriter::Open(io::OutputStream* sink,
const std::shared_ptr<Schema>& schema,
std::shared_ptr<RecordBatchWriter>* out) {
ASSIGN_OR_RAISE(*out, Open(sink, schema));
return Status::OK();
}

Result<std::shared_ptr<RecordBatchWriter>> RecordBatchFileWriter::Open(
io::OutputStream* sink, const std::shared_ptr<Schema>& schema) {
// ctor is private
auto result = std::shared_ptr<RecordBatchFileWriter>(new RecordBatchFileWriter());
result->file_impl_.reset(new RecordBatchFileWriterImpl(sink, schema));
*out = result;
return Status::OK();
return std::move(result);
}

Status RecordBatchFileWriter::WriteRecordBatch(const RecordBatch& batch,
Expand All @@ -1194,11 +1205,17 @@ namespace internal {
Status OpenRecordBatchWriter(std::unique_ptr<IpcPayloadWriter> sink,
const std::shared_ptr<Schema>& schema,
std::unique_ptr<RecordBatchWriter>* out) {
out->reset(new RecordBatchPayloadWriter(std::move(sink), schema));
// XXX should we call Start()?
ASSIGN_OR_RAISE(*out, OpenRecordBatchWriter(std::move(sink), schema));
return Status::OK();
}

Result<std::unique_ptr<RecordBatchWriter>> OpenRecordBatchWriter(
std::unique_ptr<IpcPayloadWriter> sink, const std::shared_ptr<Schema>& schema) {
// XXX should we call Start()?
return std::unique_ptr<RecordBatchWriter>(
new RecordBatchPayloadWriter(std::move(sink), schema));
}

} // namespace internal

// ----------------------------------------------------------------------
Expand Down
27 changes: 27 additions & 0 deletions cpp/src/arrow/ipc/writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <vector>

#include "arrow/ipc/message.h"
#include "arrow/result.h"
#include "arrow/util/visibility.h"

namespace arrow {
Expand Down Expand Up @@ -103,6 +104,15 @@ class ARROW_EXPORT RecordBatchStreamWriter : public RecordBatchWriter {
static Status Open(io::OutputStream* sink, const std::shared_ptr<Schema>& schema,
std::shared_ptr<RecordBatchWriter>* out);

/// Create a new writer from stream sink and schema. User is responsible for
/// closing the actual OutputStream.
///
/// \param[in] sink output stream to write to
/// \param[in] schema the schema of the record batches to be written
/// \return Result<std::shared_ptr<RecordBatchWriter>>
static Result<std::shared_ptr<RecordBatchWriter>> Open(
io::OutputStream* sink, const std::shared_ptr<Schema>& schema);

/// \brief Write a record batch to the stream
///
/// \param[in] batch the record batch to write
Expand Down Expand Up @@ -140,6 +150,14 @@ class ARROW_EXPORT RecordBatchFileWriter : public RecordBatchStreamWriter {
static Status Open(io::OutputStream* sink, const std::shared_ptr<Schema>& schema,
std::shared_ptr<RecordBatchWriter>* out);

/// Create a new writer from stream sink and schema
///
/// \param[in] sink output stream to write to
/// \param[in] schema the schema of the record batches to be written
/// \return Status
static Result<std::shared_ptr<RecordBatchWriter>> Open(
io::OutputStream* sink, const std::shared_ptr<Schema>& schema);

/// \brief Write a record batch to the file
///
/// \param[in] batch the record batch to write
Expand Down Expand Up @@ -332,6 +350,15 @@ Status OpenRecordBatchWriter(std::unique_ptr<IpcPayloadWriter> sink,
const std::shared_ptr<Schema>& schema,
std::unique_ptr<RecordBatchWriter>* out);

/// Create a new RecordBatchWriter from IpcPayloadWriter and schema.
///
/// \param[in] sink the IpcPayloadWriter to write to
/// \param[in] schema the schema of the record batches to be written
/// \return Result<std::unique_ptr<RecordBatchWriter>>
ARROW_EXPORT
Result<std::unique_ptr<RecordBatchWriter>> OpenRecordBatchWriter(
std::unique_ptr<IpcPayloadWriter> sink, const std::shared_ptr<Schema>& schema);

/// \brief Compute IpcPayload for the given schema
/// \param[in] schema the Schema that is being serialized
/// \param[in,out] dictionary_memo class to populate with assigned dictionary ids
Expand Down
4 changes: 4 additions & 0 deletions cpp/src/arrow/public-api-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
#error "DCHECK should not be visible from Arrow public headers."
#endif

#ifdef ASSIGN_OR_RAISE
#error "ASSIGN_OR_RAISE should not be visible from Arrow public headers."
#endif

#ifdef ARROW_UTIL_PARALLEL_H
#error "arrow/util/parallel.h is an internal header"
#endif
Expand Down
Loading

0 comments on commit 7e16ee4

Please sign in to comment.