diff --git a/cpp/src/copying/get_element.cu b/cpp/src/copying/get_element.cu index dc0334bd37b..a4d863d204d 100644 --- a/cpp/src/copying/get_element.cu +++ b/cpp/src/copying/get_element.cu @@ -125,7 +125,8 @@ struct get_element_functor { rmm::cuda_stream_view stream, rmm::mr::device_memory_resource *mr = rmm::mr::get_current_device_resource()) { - bool valid = is_element_valid_sync(input, index, stream); + bool valid = is_element_valid_sync(input, index, stream); + auto const child_col_idx = lists_column_view::child_column_index; if (valid) { lists_column_view lcv(input); @@ -134,9 +135,11 @@ struct get_element_functor { lists::detail::copy_slice(lcv, index, index + 1, stream, mr)->release(); // Construct scalar with row data return std::make_unique( - std::move(*row_slice_contents.children[1]), valid, stream, mr); + std::move(*row_slice_contents.children[child_col_idx]), valid, stream, mr); } else { - return make_default_constructed_scalar(data_type(type_id::LIST)); + auto empty_row_contents = empty_like(input)->release(); + return std::make_unique( + std::move(*empty_row_contents.children[child_col_idx]), valid, stream, mr); } } diff --git a/cpp/src/scalar/scalar_factories.cpp b/cpp/src/scalar/scalar_factories.cpp index e1d71b279d6..af78d84d874 100644 --- a/cpp/src/scalar/scalar_factories.cpp +++ b/cpp/src/scalar/scalar_factories.cpp @@ -146,7 +146,7 @@ template <> std::unique_ptr default_scalar_functor::operator()( rmm::cuda_stream_view stream, rmm::mr::device_memory_resource* mr) { - return std::make_unique(column(), false, stream, mr); + CUDF_FAIL("list_view type not supported"); } template <> diff --git a/cpp/tests/copying/get_value_tests.cpp b/cpp/tests/copying/get_value_tests.cpp index 7d2bc458462..40dc07512eb 100644 --- a/cpp/tests/copying/get_value_tests.cpp +++ b/cpp/tests/copying/get_value_tests.cpp @@ -223,12 +223,17 @@ TYPED_TEST(ListGetFixedWidthValueTest, NonNestedGetNonNullEmpty) TYPED_TEST(ListGetFixedWidthValueTest, NonNestedGetNull) { using LCW = cudf::test::lists_column_wrapper; + using FCW = cudf::test::fixed_width_column_wrapper; + LCW col({LCW{1, 2, 34}, LCW{}, LCW{1}, LCW{}}, this->odds_valid()); size_type index = 2; - auto s = get_element(col, index); + auto s = get_element(col, index); + auto typed_s = static_cast(s.get()); EXPECT_FALSE(s->is_valid()); + // Test preserve column hierarchy + CUDF_TEST_EXPECT_COLUMNS_EQUAL(typed_s->view(), FCW{}); } TYPED_TEST(ListGetFixedWidthValueTest, NestedGetNonNullNonEmpty) @@ -301,7 +306,9 @@ TYPED_TEST(ListGetFixedWidthValueTest, NestedGetNonNullEmpty) TYPED_TEST(ListGetFixedWidthValueTest, NestedGetNull) { - using LCW = cudf::test::lists_column_wrapper; + using LCW = cudf::test::lists_column_wrapper; + using FCW = cudf::test::fixed_width_column_wrapper; + using offset_t = cudf::test::fixed_width_column_wrapper; std::vector valid{1, 0, 1, 0}; // clang-format off @@ -315,9 +322,15 @@ TYPED_TEST(ListGetFixedWidthValueTest, NestedGetNull) // clang-format on size_type index = 1; - auto s = get_element(col, index); + auto s = get_element(col, index); + auto typed_s = static_cast(s.get()); + + auto expected_data = + make_lists_column(0, offset_t{}.release(), FCW{}.release(), 0, rmm::device_buffer{}); EXPECT_FALSE(s->is_valid()); + // Test preserve column hierarchy + CUDF_TEST_EXPECT_COLUMNS_EQUAL(typed_s->view(), *expected_data); } struct ListGetStringValueTest : public BaseFixture { @@ -363,15 +376,18 @@ TEST_F(ListGetStringValueTest, NonNestedGetNonNullEmpty) TEST_F(ListGetStringValueTest, NonNestedGetNull) { - using LCW = cudf::test::lists_column_wrapper; + using LCW = cudf::test::lists_column_wrapper; + using StringCW = strings_column_wrapper; std::vector valid{1, 0, 0, 1}; LCW col({LCW{"aaa", "Héllo"}, LCW{}, LCW{""}, LCW{"42"}}, valid.begin()); size_type index = 2; - auto s = get_element(col, index); + auto s = get_element(col, index); + auto typed_s = static_cast(s.get()); EXPECT_FALSE(s->is_valid()); + CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(typed_s->view(), StringCW{}); } TEST_F(ListGetStringValueTest, NestedGetNonNullNonEmpty) @@ -446,7 +462,9 @@ TEST_F(ListGetStringValueTest, NestedGetNonNullEmpty) TEST_F(ListGetStringValueTest, NestedGetNull) { - using LCW = cudf::test::lists_column_wrapper; + using LCW = cudf::test::lists_column_wrapper; + using offset_t = cudf::test::fixed_width_column_wrapper; + using StringCW = cudf::test::strings_column_wrapper; std::vector valid{0, 0, 1, 1}; // clang-format off @@ -458,11 +476,16 @@ TEST_F(ListGetStringValueTest, NestedGetNull) LCW{} }, valid.begin()); // clang-format on - LCW expected_data{}; size_type index = 0; - auto s = get_element(col, index); + auto s = get_element(col, index); + auto typed_s = static_cast(s.get()); + + auto expected_data = + make_lists_column(0, offset_t{}.release(), StringCW{}.release(), 0, rmm::device_buffer{}); + EXPECT_FALSE(s->is_valid()); + CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*expected_data, typed_s->view()); } /** @@ -480,10 +503,11 @@ struct ListGetStructValueTest : public BaseFixture { * in `initializer_list`. However this is an expensive function because it repeatedly * calls `cudf::set_null_mask` for each row. */ - std::unique_ptr make_test_lists_column(size_type num_lists, - fixed_width_column_wrapper offsets, - std::unique_ptr child, - std::initializer_list null_mask) + std::unique_ptr make_test_lists_column( + size_type num_lists, + fixed_width_column_wrapper offsets, + std::unique_ptr child, + std::initializer_list null_mask) { size_type null_count = num_lists - std::accumulate(null_mask.begin(), null_mask.end(), 0); auto d_null_mask = cudf::create_null_mask( @@ -619,7 +643,7 @@ TYPED_TEST(ListGetStructValueTest, NonNestedGetNonNullEmpty) // 3-rows // [{1, NULL, NULL}, NULL] // [{3, "xyz", [3, 8, 4]}] - // [] <- get_element(0) + // [] <- get_element(2) auto list_column = this->make_test_lists_column(3, {0, 2, 3, 3}, this->leaf_data(), {1, 1, 1}); size_type index = 2; @@ -638,15 +662,21 @@ TYPED_TEST(ListGetStructValueTest, NonNestedGetNonNullEmpty) TYPED_TEST(ListGetStructValueTest, NonNestedGetNull) { // 2-rows - // NULL <- get_element(0) + // NULL <- get_element(0) // [{3, "xyz", [3, 8, 4]}] + using valid_t = std::vector; + auto list_column = this->make_test_lists_column(2, {0, 2, 3}, this->leaf_data(), {0, 1}); size_type index = 0; - auto s = get_element(list_column->view(), index); + auto s = get_element(list_column->view(), index); + auto typed_s = static_cast(s.get()); + + auto expected_data = this->make_test_structs_column({}, {}, {}, valid_t{}.begin()); EXPECT_FALSE(s->is_valid()); + CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(typed_s->view(), expected_data); } TYPED_TEST(ListGetStructValueTest, NestedGetNonNullNonEmpty) @@ -716,7 +746,7 @@ TYPED_TEST(ListGetStructValueTest, NestedGetNonNullEmpty) { // 3-rows // [[{1, NULL, NULL}, NULL]] - // [] <- get_element(1) + // [] <- get_element(1) // [[{3, "xyz", [3, 8, 4]}]] auto list_column = this->make_test_lists_column(2, {0, 2, 3}, this->leaf_data(), {1, 1}); @@ -741,15 +771,25 @@ TYPED_TEST(ListGetStructValueTest, NestedGetNull) // 3-rows // [[{1, NULL, NULL}, NULL]] // [] - // NULL <- get_element(1) + // NULL <- get_element(2) + + using valid_t = std::vector; + using offset_t = cudf::test::fixed_width_column_wrapper; + auto list_column = this->make_test_lists_column(2, {0, 2, 3}, this->leaf_data(), {1, 1}); auto list_column_nested = this->make_test_lists_column(3, {0, 1, 1, 2}, std::move(list_column), {1, 1, 0}); size_type index = 2; auto s = get_element(list_column_nested->view(), index); + auto typed_s = static_cast(s.get()); + + auto nested = this->make_test_structs_column({}, {}, {}, valid_t{}.begin()); + auto expected_data = + make_lists_column(0, offset_t{}.release(), nested.release(), 0, rmm::device_buffer{}); EXPECT_FALSE(s->is_valid()); + CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*expected_data, typed_s->view()); } } // namespace test