Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix concatenate empty structs #8947

Merged
merged 8 commits into from
Aug 6, 2021
Merged

Fix concatenate empty structs #8947

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fa7b8c4
fix concat struct columns with empty children
sperlingxx Aug 4, 2021
f8b1946
fix clang style
sperlingxx Aug 4, 2021
418fd9f
Update cpp/src/structs/copying/concatenate.cu
sperlingxx Aug 5, 2021
5e616ac
fix
sperlingxx Aug 5, 2021
e11c30f
Update cpp/src/structs/copying/concatenate.cu
sperlingxx Aug 5, 2021
67332e0
Apply suggestions from code review
sperlingxx Aug 5, 2021
1430e3f
Update cpp/src/structs/copying/concatenate.cu
sperlingxx Aug 5, 2021
59015ba
fix
sperlingxx Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cpp/src/structs/copying/concatenate.cu
View file
Open in desktop
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,12 @@ 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 total_length = children[0]->size();
sperlingxx marked this conversation as resolved.
Show resolved Hide resolved
auto accSizeFn = [](size_type s, column_view const& c) { return s + c.size(); };
sperlingxx marked this conversation as resolved.
Show resolved Hide resolved
auto const total_length =
!children.empty() ? children[0]->size()
: std::accumulate(columns.begin(), columns.end(), size_type{0}, accSizeFn);

// 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
View file
Open in desktop
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