From 7521c3f2c1acc8161c82a5e4cd1d9f96f58945ec Mon Sep 17 00:00:00 2001 From: nvdbaranec <56695930+nvdbaranec@users.noreply.github.com> Date: Mon, 12 Jul 2021 19:13:00 -0500 Subject: [PATCH] Fix a crash in pack() when being handed tables with no columns. (#8697) 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: https://github.com/rapidsai/cudf/pull/8697 --- cpp/include/cudf/copying.hpp | 10 ++++++++++ cpp/src/copying/pack.cpp | 12 ++++++++---- cpp/tests/copying/pack_tests.cpp | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/cpp/include/cudf/copying.hpp b/cpp/include/cudf/copying.hpp index 6be865ea993..6ab115196d6 100644 --- a/cpp/include/cudf/copying.hpp +++ b/cpp/include/cudf/copying.hpp @@ -529,6 +529,7 @@ struct packed_columns { * @ingroup copy_split */ struct metadata { + metadata() = default; metadata(std::vector&& v) : data_(std::move(v)) {} uint8_t const* data() const { return data_.data(); } size_t size() const { return data_.size(); } @@ -537,6 +538,15 @@ struct packed_columns { std::vector data_; }; + packed_columns() + : metadata_(std::make_unique()), gpu_data(std::make_unique()) + { + } + packed_columns(std::unique_ptr&& md, std::unique_ptr&& gd) + : metadata_(std::move(md)), gpu_data(std::move(gd)) + { + } + std::unique_ptr metadata_; std::unique_ptr gpu_data; }; diff --git a/cpp/src/copying/pack.cpp b/cpp/src/copying/pack.cpp index 182e3ff0584..89e5972f448 100644 --- a/cpp/src/copying/pack.cpp +++ b/cpp/src/copying/pack.cpp @@ -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 @@ -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); } /** @@ -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(input.gpu_data->data())); + return input.metadata_->size() == 0 + ? table_view{} + : detail::unpack(input.metadata_->data(), + reinterpret_cast(input.gpu_data->data())); } /** diff --git a/cpp/tests/copying/pack_tests.cpp b/cpp/tests/copying/pack_tests.cpp index 2e7c41333d5..11aa505d163 100644 --- a/cpp/tests/copying/pack_tests.cpp +++ b/cpp/tests/copying/pack_tests.cpp @@ -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 a; + cudf::test::strings_column_wrapper b; + cudf::test::lists_column_wrapper c; + cudf::table_view t({a, b, c}); + this->run_test(t); + } +} + // clang-format on } // namespace test