Skip to content

Commit

Permalink
Fix a crash in pack() when being handed tables with no columns. (#8697)
Browse files Browse the repository at this point in the history
Also changes the behavior of pack() such that when returning empty data, the `metadata_` and `gpu_data` unique_ptrs are not null, but instead point to empty `metadata` and `rmm::device_buffer` objects, respectively.  Very mild breakage of the interface.

Authors:
  - https://github.com/nvdbaranec

Approvers:
  - Ram (Ramakrishna Prabhu) (https://github.com/rgsl888prabhu)
  - Mark Harris (https://github.com/harrism)

URL: #8697
  • Loading branch information
nvdbaranec authored Jul 13, 2021
1 parent 05fd176 commit 7521c3f
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
10 changes: 10 additions & 0 deletions cpp/include/cudf/copying.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ struct packed_columns {
* @ingroup copy_split
*/
struct metadata {
metadata() = default;
metadata(std::vector<uint8_t>&& v) : data_(std::move(v)) {}
uint8_t const* data() const { return data_.data(); }
size_t size() const { return data_.size(); }
Expand All @@ -537,6 +538,15 @@ struct packed_columns {
std::vector<uint8_t> data_;
};

packed_columns()
: metadata_(std::make_unique<metadata>()), gpu_data(std::make_unique<rmm::device_buffer>())
{
}
packed_columns(std::unique_ptr<metadata>&& md, std::unique_ptr<rmm::device_buffer>&& gd)
: metadata_(std::move(md)), gpu_data(std::move(gd))
{
}

std::unique_ptr<metadata> metadata_;
std::unique_ptr<rmm::device_buffer> gpu_data;
};
Expand Down
12 changes: 8 additions & 4 deletions cpp/src/copying/pack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ packed_columns pack(cudf::table_view const& input,
// do a contiguous_split with no splits to get the memory for the table
// arranged as we want it
auto contig_split_result = cudf::detail::contiguous_split(input, {}, stream, mr);
return std::move(contig_split_result[0].data);
return contig_split_result.empty() ? packed_columns{} : std::move(contig_split_result[0].data);
}

template <typename ColumnIter>
Expand Down Expand Up @@ -229,7 +229,9 @@ packed_columns::metadata pack_metadata(table_view const& table,
size_t buffer_size)
{
CUDF_FUNC_RANGE();
return detail::pack_metadata(table.begin(), table.end(), contiguous_buffer, buffer_size);
return table.is_empty()
? packed_columns::metadata{}
: detail::pack_metadata(table.begin(), table.end(), contiguous_buffer, buffer_size);
}

/**
Expand All @@ -238,8 +240,10 @@ packed_columns::metadata pack_metadata(table_view const& table,
table_view unpack(packed_columns const& input)
{
CUDF_FUNC_RANGE();
return detail::unpack(input.metadata_->data(),
reinterpret_cast<uint8_t const*>(input.gpu_data->data()));
return input.metadata_->size() == 0
? table_view{}
: detail::unpack(input.metadata_->data(),
reinterpret_cast<uint8_t const*>(input.gpu_data->data()));
}

/**
Expand Down
18 changes: 18 additions & 0 deletions cpp/tests/copying/pack_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,24 @@ TEST_F(PackUnpackTest, NestedSliced)
this->run_test(t);
}

TEST_F(PackUnpackTest, EmptyTable)
{
// no columns
{
cudf::table_view t;
this->run_test(t);
}

// no rows
{
cudf::test::fixed_width_column_wrapper<int> a;
cudf::test::strings_column_wrapper b;
cudf::test::lists_column_wrapper<float> c;
cudf::table_view t({a, b, c});
this->run_test(t);
}
}

// clang-format on

} // namespace test
Expand Down

0 comments on commit 7521c3f

Please sign in to comment.