Skip to content

Fix list scatter child indexing for sliced inputs #23821

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

Open
VaggelisGian wants to merge 5 commits into
base: main
Choose a base branch
from
+116 −8
Open

Fix list scatter child indexing for sliced inputs #23821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4b527e
Fix list scatter child indexing for sliced inputs
VaggelisGian Aug 25, 2026
22e4889
Test scattering null rows from sliced list sources
VaggelisGian Aug 27, 2026
4bcaca4
Apply clang-format to scatter list tests
VaggelisGian Aug 27, 2026
ef2f79c
Use empty null rows in sliced structs scatter test
VaggelisGian Aug 28, 2026
952f387
Merge branch 'main' into fix-lists-scatter-view-offset
wence- Aug 28, 2026
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
17 changes: 9 additions & 8 deletions cpp/src/lists/copying/scatter_helper.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,8 @@ struct list_child_constructor {
auto row_index = d_list_vector[list_index].row_index();
auto actual_list_row = d_list_vector[list_index].bind_to_column(source_lists, target_lists);
auto lists_column = actual_list_row.get_column();
auto lists_offsets_ptr = lists_column.offsets().template data<int32_t>();
auto child_strings_column = lists_column.child();
auto strings_offset = lists_offsets_ptr[row_index] + intra_index;
auto strings_offset = lists_column.offset_at(row_index) + intra_index;

if (child_strings_column.is_null(strings_offset)) { return null_string_view; }
auto const d_str = child_strings_column.template element<string_view>(strings_offset);
Expand Down Expand Up @@ -313,11 +312,10 @@ struct list_child_constructor {
auto actual_list_row = d_list_vector[list_index].bind_to_column(source_lists, target_lists);
auto lists_column = actual_list_row.get_column();
auto child_lists_column = lists_column.child();
auto lists_offsets_ptr = lists_column.offsets().template data<int32_t>();
auto child_lists_offsets_ptr =
child_lists_column.child(lists_column_view::offsets_column_index)
.template data<int32_t>();
auto child_row_index = lists_offsets_ptr[row_index] + intra_index;
auto child_row_index = lists_column.offset_at(row_index) + intra_index;
auto size =
child_lists_offsets_ptr[child_row_index + 1] - child_lists_offsets_ptr[child_row_index];
return unbound_list_view{label, child_row_index, size};
Expand Down Expand Up @@ -389,13 +387,14 @@ struct list_child_constructor {
cudf::size_type const& structs_list_num_rows,
column_view const& structs_list_offsets,
bitmask_type const* structs_list_nullmask,
cudf::size_type const& structs_list_null_count) {
cudf::size_type const& structs_list_null_count,
cudf::size_type const& structs_list_offset) {
return lists_column_view(column_view(data_type{type_id::LIST},
structs_list_num_rows,
nullptr,
structs_list_nullmask,
structs_list_null_count,
0,
structs_list_offset,
{structs_list_offsets, structs_member}));
};

Expand All @@ -405,7 +404,8 @@ struct list_child_constructor {
source_lists_column_view.size(),
source_lists_column_view.offsets(),
source_lists_column_view.null_mask(),
source_lists_column_view.null_count());
source_lists_column_view.null_count(),
source_lists_column_view.offset());
});

auto const iter_target_member_as_list =
Expand All @@ -414,7 +414,8 @@ struct list_child_constructor {
target_lists_column_view.size(),
target_lists_column_view.offsets(),
target_lists_column_view.null_mask(),
target_lists_column_view.null_count());
target_lists_column_view.null_count(),
target_lists_column_view.offset());
});

std::transform(iter_source_member_as_list,
Expand Down
107 changes: 107 additions & 0 deletions cpp/tests/copying/scatter_list_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,82 @@ TYPED_TEST(TypedScatterListsTest, SlicedInputLists)
cudf::test::lists_column_wrapper<T, int32_t>{{8, 8, 8}, {2, 2}, {9, 9, 9, 9}, {4, 4}, {5, 5}});
}

TYPED_TEST(TypedScatterListsTest, SlicedInputListsOfLists)

@wence- wence- Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Can we also test the case where the source has a null mask? I think we can do it in this test by slicing from {1, 4} and having one of the rows we scatter be null.

Same, mutatis mutandis, in the other tests.

@VaggelisGian VaggelisGian Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed 22e4889: all three sliced-input tests now scatter one null source row and expect the corresponding output row to stay null.

  • SlicedInputListsOfLists and SlicedInputListsOfStrings keep the {1, ...} slicing pattern with the null list row at source index 2 so it lands inside the sliced view that gets scattered.
  • SlicedInputListsOfStructs carries its null mask through detail::make_null_mask since it builds the column with make_lists_column directly.

Not compiled locally, no Linux toolchain on this machine. Could you /ok to test 22e4889 when convenient?

{
using T = TypeParam;

auto src_list_column =
cudf::test::lists_column_wrapper<T, int32_t>{
{{{0, 0}, {9, 9}}, {{1, 1}, {8, 8}}, {{2, 2}, {7, 7}}, {{3, 3}, {6, 6}}},
cudf::test::iterators::null_at(2)}
.release();
auto src_sliced = cudf::slice(src_list_column->view(), {1, 4}).front();

auto target_list_column = cudf::test::lists_column_wrapper<T, int32_t>{{{9, 9}, {8, 8}, {7, 7}},
{{6, 6}, {5, 5}, {4, 4}},
{{3, 3}, {2, 2}, {1, 1}},
{{9, 9}, {8, 8}, {7, 7}},
{{6, 6}, {5, 5}, {4, 4}},
{{3, 3}, {2, 2}, {1, 1}}};

auto scatter_map = cudf::test::fixed_width_column_wrapper<cudf::size_type>{2, 0};

auto ret = cudf::scatter(
cudf::table_view({src_sliced}), scatter_map, cudf::table_view({target_list_column}));

CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(
cudf::test::lists_column_wrapper<T, int32_t>{{{},
{{6, 6}, {5, 5}, {4, 4}},
{{1, 1}, {8, 8}},
{{9, 9}, {8, 8}, {7, 7}},
{{6, 6}, {5, 5}, {4, 4}},
{{3, 3}, {2, 2}, {1, 1}}},
cudf::test::iterators::null_at(0)},
ret->get_column(0));
}

TYPED_TEST(TypedScatterListsTest, SlicedInputListsOfStructs)
{
using T = TypeParam;
using offsets_column = cudf::test::fixed_width_column_wrapper<cudf::size_type>;
using numerics_column = cudf::test::fixed_width_column_wrapper<T>;

auto src_numerics = numerics_column{0, 1, 2, 3};
auto src_structs = cudf::test::structs_column_wrapper{{src_numerics}};
auto src_validity = cudf::test::iterators::null_at(2);
auto [src_mask, src_null_count] =
cudf::test::detail::make_null_mask(src_validity, src_validity + 3);
auto src_list_column = cudf::make_lists_column(3,
offsets_column{0, 2, 4, 4}.release(),
src_structs.release(),
src_null_count,
std::move(src_mask));
auto src_sliced = cudf::slice(src_list_column->view(), {1, 3}).front();

auto tgt_numerics = numerics_column{0, 1, 2, 3, 4};
auto tgt_structs = cudf::test::structs_column_wrapper{{tgt_numerics}};
auto target_list_column = cudf::make_lists_column(
5, offsets_column{0, 1, 2, 3, 4, 5}.release(), tgt_structs.release(), 0, {});

auto scatter_map = cudf::test::fixed_width_column_wrapper<cudf::size_type>{2, 0};

auto ret = cudf::scatter(
cudf::table_view({src_sliced}), scatter_map, cudf::table_view({target_list_column->view()}));

auto expected_numerics = numerics_column{1, 2, 3, 3, 4};
auto expected_structs = cudf::test::structs_column_wrapper{{expected_numerics}};
auto expected_validity = cudf::test::iterators::null_at(0);
auto [expected_mask, expected_null_count] =
cudf::test::detail::make_null_mask(expected_validity, expected_validity + 5);
auto expected = cudf::make_lists_column(5,
offsets_column{0, 0, 1, 3, 4, 5}.release(),
expected_structs.release(),
expected_null_count,
std::move(expected_mask));

CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(expected->view(), ret->get_column(0));
}

TYPED_TEST(TypedScatterListsTest, EmptyListsOfFixedWidth)
{
using T = TypeParam;
Expand Down Expand Up @@ -217,6 +293,37 @@ TEST_F(ScatterListsTest, ListsOfStrings)
ret->get_column(0));
}

TEST_F(ScatterListsTest, SlicedInputListsOfStrings)
{
auto src_list_column =
cudf::test::lists_column_wrapper<cudf::string_view>{
{{"zero"}, {"one", "one", "one"}, {"two", "two"}, {"three", "three", "three", "three"}},
cudf::test::iterators::null_at(2)}
.release();
auto src_sliced = cudf::slice(src_list_column->view(), {1, 4}).front();

auto target_list_column =
cudf::test::lists_column_wrapper<cudf::string_view>{{"a", "a", "a", "a", "a"},
{"b", "b", "b", "b", "b"},
{"c", "c", "c", "c", "c"},
{"d", "d", "d", "d", "d"},
{"e", "e", "e", "e", "e"}};

auto scatter_map = cudf::test::fixed_width_column_wrapper<int32_t>{2, 0};

auto ret = cudf::scatter(
cudf::table_view({src_sliced}), scatter_map, cudf::table_view({target_list_column}));

CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(
cudf::test::lists_column_wrapper<cudf::string_view>{{{},
{"b", "b", "b", "b", "b"},
{"one", "one", "one"},
{"d", "d", "d", "d", "d"},
{"e", "e", "e", "e", "e"}},
cudf::test::iterators::null_at(0)},
ret->get_column(0));
}

TEST_F(ScatterListsTest, ListsOfNullableStrings)
{
auto src_strings_column =
Expand Down
Loading