Skip to content

Commit

Permalink
apacheGH-37851: [C++] IPC: ArrayLoader style enhancement (apache#37872)
Browse files Browse the repository at this point in the history
### Rationale for this change

Enhance the style of `arrow::ipc::ArrayLoader`'s `SkipField`

### What changes are included in this PR?

Set `out_` to `nullptr` when `SkipField` is called.

### Are these changes tested?

No

### Are there any user-facing changes?

No

* Closes: apache#37851

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Benjamin Kietzman <bengilgit@gmail.com>
  • Loading branch information
mapleFU authored and Jeremy Aguilon committed Oct 23, 2023
1 parent 2d3f3df commit 1a08847
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions cpp/src/arrow/ipc/reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ class ArrayLoader {
}
}

Status LoadType(const DataType& type) { return VisitTypeInline(type, this); }
Status LoadType(const DataType& type) {
DCHECK_NE(out_, nullptr);
return VisitTypeInline(type, this);
}

Status Load(const Field* field, ArrayData* out) {
if (max_recursion_depth_ <= 0) {
Expand All @@ -223,6 +226,9 @@ class ArrayLoader {
skip_io_ = true;
Status status = Load(field, &dummy);
skip_io_ = false;
// GH-37851: Reset state. Load will set `out_` to `&dummy`, which would
// be a dangling pointer.
out_ = nullptr;
return status;
}

Expand Down Expand Up @@ -258,6 +264,7 @@ class ArrayLoader {
}

Status LoadCommon(Type::type type_id) {
DCHECK_NE(out_, nullptr);
// This only contains the length and null count, which we need to figure
// out what to do with the buffers. For example, if null_count == 0, then
// we can skip that buffer without reading from shared memory
Expand All @@ -276,6 +283,7 @@ class ArrayLoader {

template <typename TYPE>
Status LoadPrimitive(Type::type type_id) {
DCHECK_NE(out_, nullptr);
out_->buffers.resize(2);

RETURN_NOT_OK(LoadCommon(type_id));
Expand All @@ -290,6 +298,7 @@ class ArrayLoader {

template <typename TYPE>
Status LoadBinary(Type::type type_id) {
DCHECK_NE(out_, nullptr);
out_->buffers.resize(3);

RETURN_NOT_OK(LoadCommon(type_id));
Expand All @@ -299,6 +308,7 @@ class ArrayLoader {

template <typename TYPE>
Status LoadList(const TYPE& type) {
DCHECK_NE(out_, nullptr);
out_->buffers.resize(2);

RETURN_NOT_OK(LoadCommon(type.id()));
Expand All @@ -313,6 +323,7 @@ class ArrayLoader {
}

Status LoadChildren(const std::vector<std::shared_ptr<Field>>& child_fields) {
DCHECK_NE(out_, nullptr);
ArrayData* parent = out_;

parent->child_data.resize(child_fields.size());
Expand Down Expand Up @@ -2010,7 +2021,7 @@ class StreamDecoder::StreamDecoderImpl : public StreamDecoderInternal {
};

StreamDecoder::StreamDecoder(std::shared_ptr<Listener> listener, IpcReadOptions options) {
impl_.reset(new StreamDecoderImpl(std::move(listener), options));
impl_ = std::make_unique<StreamDecoderImpl>(std::move(listener), options);
}

StreamDecoder::~StreamDecoder() {}
Expand Down

0 comments on commit 1a08847

Please sign in to comment.