Skip to content

Commit

Permalink
Fix concatenate empty structs (#8947)
Browse files Browse the repository at this point in the history
Closes #8929

Current PR is to fix the illegal memory access problem occurred when concatenating structs with empty children.

Authors:
  - Alfred Xu (https://github.com/sperlingxx)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Mike Wilson (https://github.com/hyperbolic2346)

URL: #8947
  • Loading branch information
sperlingxx authored Aug 6, 2021
1 parent ed3a601 commit ffb37c9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
7 changes: 6 additions & 1 deletion cpp/src/structs/copying/concatenate.cu
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include <algorithm>
#include <memory>
#include <numeric>

namespace cudf {
namespace structs {
Expand All @@ -53,7 +54,11 @@ std::unique_ptr<column> concatenate(host_span<column_view const> columns,
return cudf::detail::concatenate(cols, stream, mr);
});

size_type const total_length = children[0]->size();
// get total length from concatenated children; if no child exists, we would compute it
auto const acc_size_fn = [](size_type s, column_view const& c) { return s + c.size(); };
auto const total_length =
!children.empty() ? children[0]->size()
: std::accumulate(columns.begin(), columns.end(), size_type{0}, acc_size_fn);

// if any of the input columns have nulls, construct the output mask
bool const has_nulls =
Expand Down
16 changes: 16 additions & 0 deletions cpp/tests/copying/concatenate_tests.cu
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,22 @@ TEST_F(StructsColumnTest, ConcatenateStructs)
cudf::test::expect_columns_equivalent(*result, *expected);
}

TEST_F(StructsColumnTest, ConcatenateEmptyStructs)
{
using namespace cudf::test;

auto expected = cudf::make_structs_column(10, {}, 0, rmm::device_buffer());
auto first = cudf::make_structs_column(5, {}, 0, rmm::device_buffer());
auto second = cudf::make_structs_column(2, {}, 0, rmm::device_buffer());
auto third = cudf::make_structs_column(0, {}, 0, rmm::device_buffer());
auto fourth = cudf::make_structs_column(3, {}, 0, rmm::device_buffer());

// concatenate
auto result = cudf::concatenate(std::vector<column_view>({*first, *second, *third, *fourth}));
CUDF_EXPECTS(result->size() == expected->size(), "column size changed after concat");
cudf::test::expect_columns_equivalent(*result, *expected);
}

TEST_F(StructsColumnTest, ConcatenateSplitStructs)
{
using namespace cudf::test;
Expand Down

0 comments on commit ffb37c9

Please sign in to comment.