[BUG] mismatched columns on struct lower bound

[revans2]

Describe the bug
In lower_bound and upper_bound for structs it calls structs::detail::flatten_nested_columns for both t and values but it does this independently for both of them. That means if we end up with a t that has null structs but a values that does not, or vise versa we can get errors.

Steps/Code to reproduce bug

diff --git a/cpp/tests/search/search_struct_test.cpp b/cpp/tests/search/search_struct_test.cpp
index 50c326269a..c127f6c8b6 100644
--- a/cpp/tests/search/search_struct_test.cpp
+++ b/cpp/tests/search/search_struct_test.cpp
@@ -152,8 +152,8 @@ TYPED_TEST(TypedStructSearchTest, SimpleInputWithNullsTests)
   auto const structs_values = structs_col{{child_col_values}, null_at(3)}.release();
 
   // Sorted asc, nulls first
-  auto child_col_t = col_wrapper{{XXX, null, 0, 1, 2, 2, 2, 2, 3, 3, 4}, null_at(1)};
-  auto structs_t   = structs_col{{child_col_t}, null_at(0)}.release();
+  auto child_col_t = col_wrapper{{0, 0, 0, 1, 2, 2, 2, 2, 3, 3, 4}};
+  auto structs_t   = structs_col{{child_col_t}}.release();
 
   auto results =
     search_bounds(structs_t, structs_values, {cudf::order::ASCENDING}, {cudf::null_order::BEFORE});

This causes the error to happen. I have not updated the expected result to what they should be, but it does expose the error.

[ RUN      ] TypedStructSearchTest/17.SimpleInputWithNullsTests
unknown file: Failure
C++ exception with description "cuDF failure at: ../include/cudf/table/row_operators.cuh:361: Mismatched number of columns." thrown in the test body.
[  FAILED  ] TypedStructSearchTest/17.SimpleInputWithNullsTests, where TypeParam = cudf::detail::timestamp<cuda::std::__3::chrono::duration<long, cuda::std::__3::ratio<1l, 1000l> > > (0 ms)

[ttnghia]

Hi there! Please verify that my fix (#8188) actually fixes this.

=============
PS: Ooop, I accidentally merged the PR. If the bug is still there, I'll try again with a new PR.

[abellina]

We had some tests that were failing on this exception that are now fixed with @ttnghia's fix. Closing this and thanks!

[abellina]

Heads up, we are seeing this again, now with inputs that are not null at all. I am trying to write the test in c++, but wanted to reopen this.

[revans2]

I got a repro case for the new failure.

diff --git a/cpp/tests/search/search_struct_test.cpp b/cpp/tests/search/search_struct_test.cpp
index 1c2e9b02f0..482853ae21 100644
--- a/cpp/tests/search/search_struct_test.cpp
+++ b/cpp/tests/search/search_struct_test.cpp
@@ -29,7 +29,7 @@ using int32s_col  = cudf::test::fixed_width_column_wrapper<int32_t>;
 using structs_col = cudf::test::structs_column_wrapper;
 using strings_col = cudf::test::strings_column_wrapper;
 
-constexpr bool print_all{false};  // For debugging only
+constexpr bool print_all{true};  // For debugging only
 constexpr int32_t null{0};        // Mark for null child elements
 constexpr int32_t XXX{0};         // Mark for null struct elements
 
@@ -144,6 +144,31 @@ TYPED_TEST(TypedStructSearchTest, SlicedColumnInputTests)
   CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_upper_bound, results.second->view(), print_all);
 }
 
+TYPED_TEST(TypedStructSearchTest, UpperBoundColMismatchTest)
+{
+  using col_wrapper = cudf::test::fixed_width_column_wrapper<int64_t>;
+  auto too_big = cudf::test::iterator_with_null_at(2048);
+
+  auto child_col_t = col_wrapper{{-7858725485978029677L}, too_big};
+  auto const structs_t = structs_col{{child_col_t}, too_big}.release();
+
+  auto child_col_values              = col_wrapper{{-8953497368527767583L}};
+  auto const structs_values = structs_col{{child_col_values}}.release();
+
+  cudf::test::print(*structs_t);
+  cudf::test::print(*structs_values);
+
+  std::cerr << "A" << std::endl;
+  auto results   = search_bounds(structs_t, structs_values);
+  auto expected_lower_bound = int32s_col{1};
+  auto expected_upper_bound = int32s_col{1};
+  std::cerr << "B" << std::endl;
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_lower_bound, results.first->view(), print_all);
+  std::cerr << "C" << std::endl;
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_upper_bound, results.second->view(), print_all);
+  std::cerr << "D" << std::endl;
+}
+
 TYPED_TEST(TypedStructSearchTest, SimpleInputWithNullsTests)
 {
   using col_wrapper = cudf::test::fixed_width_column_wrapper<TypeParam, int32_t>;

The test is not great but it does reproduce the error. It looks to be related to validity only being on one column, and not the other.

[revans2]

This is a better test so at least it covers the types.

diff --git a/cpp/tests/search/search_struct_test.cpp b/cpp/tests/search/search_struct_test.cpp
index 1c2e9b02f0..809d055beb 100644
--- a/cpp/tests/search/search_struct_test.cpp
+++ b/cpp/tests/search/search_struct_test.cpp
@@ -29,7 +29,7 @@ using int32s_col  = cudf::test::fixed_width_column_wrapper<int32_t>;
 using structs_col = cudf::test::structs_column_wrapper;
 using strings_col = cudf::test::strings_column_wrapper;
 
-constexpr bool print_all{false};  // For debugging only
+constexpr bool print_all{true};  // For debugging only
 constexpr int32_t null{0};        // Mark for null child elements
 constexpr int32_t XXX{0};         // Mark for null struct elements
 
@@ -144,6 +144,31 @@ TYPED_TEST(TypedStructSearchTest, SlicedColumnInputTests)
   CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_upper_bound, results.second->view(), print_all);
 }
 
+TYPED_TEST(TypedStructSearchTest, UpperBoundColMismatchTest)
+{
+  using col_wrapper = cudf::test::fixed_width_column_wrapper<TypeParam, int32_t>;
+  auto too_big = cudf::test::iterator_with_null_at(2048);
+
+  auto child_col_t = col_wrapper{{100}, too_big};
+  auto const structs_t = structs_col{{child_col_t}, too_big}.release();
+
+  auto child_col_values              = col_wrapper{{0}};
+  auto const structs_values = structs_col{{child_col_values}}.release();
+
+  cudf::test::print(*structs_t);
+  cudf::test::print(*structs_values);
+
+  std::cerr << "A" << std::endl;
+  auto results   = search_bounds(structs_t, structs_values);
+  auto expected_lower_bound = int32s_col{1};
+  auto expected_upper_bound = int32s_col{1};
+  std::cerr << "B" << std::endl;
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_lower_bound, results.first->view(), print_all);
+  std::cerr << "C" << std::endl;
+  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected_upper_bound, results.second->view(), print_all);
+  std::cerr << "D" << std::endl;
+}
+
 TYPED_TEST(TypedStructSearchTest, SimpleInputWithNullsTests)
 {
   using col_wrapper = cudf::test::fixed_width_column_wrapper<TypeParam, int32_t>;