From 3db45d84cb2956c5992c037471fff7e9eae9cc06 Mon Sep 17 00:00:00 2001
From: Nghia Truong <nghiatruong.vn@gmail.com>
Date: Fri, 7 Jan 2022 07:32:41 -0700
Subject: [PATCH 1/2] Fix null check and add unit tests

---
 cpp/src/reductions/struct_minmax_util.cuh |  3 ++-
 cpp/tests/groupby/max_tests.cpp           | 21 +++++++++++++++++++++
 cpp/tests/groupby/min_tests.cpp           | 21 +++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/cpp/src/reductions/struct_minmax_util.cuh b/cpp/src/reductions/struct_minmax_util.cuh
index 8a7e94ea4ca..bdce582b28d 100644
--- a/cpp/src/reductions/struct_minmax_util.cuh
+++ b/cpp/src/reductions/struct_minmax_util.cuh
@@ -23,6 +23,7 @@
 #include <cudf/detail/utilities/vector_factories.hpp>
 #include <cudf/table/row_operators.cuh>
 #include <cudf/table/table_device_view.cuh>
+#include <cudf/table/table_view.hpp>
 
 namespace cudf {
 namespace reduction {
@@ -97,7 +98,7 @@ class comparison_binop_generator {
         table_view{{input}}, {}, std::vector<null_order>{DEFAULT_NULL_ORDER})},
       d_flattened_input_ptr{table_device_view::create(flattened_input, stream)},
       is_min_op(is_min_op),
-      has_nulls{input.has_nulls()},
+      has_nulls{has_nested_nulls(table_view{{input}})},
       null_orders_dvec(0, stream)
   {
     if (is_min_op) {
diff --git a/cpp/tests/groupby/max_tests.cpp b/cpp/tests/groupby/max_tests.cpp
index 983802cb9a2..29713bd923b 100644
--- a/cpp/tests/groupby/max_tests.cpp
+++ b/cpp/tests/groupby/max_tests.cpp
@@ -388,5 +388,26 @@ TEST_F(groupby_max_struct_test, null_keys_and_values)
   test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg));
 }
 
+TEST_F(groupby_max_struct_test, values_with_null_child)
+{
+  constexpr int32_t null{0};
+  auto const keys = fixed_width_column_wrapper<int32_t>{1, 1};
+  auto const vals = [] {
+    auto child1 = fixed_width_column_wrapper<int32_t>{1, 1};
+    auto child2 = fixed_width_column_wrapper<int32_t>{{-1, null}, null_at(1)};
+    return structs_column_wrapper{child1, child2};
+  }();
+
+  auto const expect_keys = fixed_width_column_wrapper<int32_t>{1};
+  auto const expect_vals = [] {
+    auto child1 = fixed_width_column_wrapper<int32_t>{1};
+    auto child2 = fixed_width_column_wrapper<int32_t>{-1};
+    return structs_column_wrapper{child1, child2};
+  }();
+
+  auto agg = cudf::make_max_aggregation<groupby_aggregation>();
+  test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg));
+}
+
 }  // namespace test
 }  // namespace cudf
diff --git a/cpp/tests/groupby/min_tests.cpp b/cpp/tests/groupby/min_tests.cpp
index aca3384768c..dad2ee3d621 100644
--- a/cpp/tests/groupby/min_tests.cpp
+++ b/cpp/tests/groupby/min_tests.cpp
@@ -387,5 +387,26 @@ TEST_F(groupby_min_struct_test, null_keys_and_values)
   test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg));
 }
 
+TEST_F(groupby_min_struct_test, values_with_null_child)
+{
+  constexpr int32_t null{0};
+  auto const keys = fixed_width_column_wrapper<int32_t>{1, 1};
+  auto const vals = [] {
+    auto child1 = fixed_width_column_wrapper<int32_t>{1, 1};
+    auto child2 = fixed_width_column_wrapper<int32_t>{{-1, null}, null_at(1)};
+    return structs_column_wrapper{child1, child2};
+  }();
+
+  auto const expect_keys = fixed_width_column_wrapper<int32_t>{1};
+  auto const expect_vals = [] {
+    auto child1 = fixed_width_column_wrapper<int32_t>{1};
+    auto child2 = fixed_width_column_wrapper<int32_t>{{null}, null_at(0)};
+    return structs_column_wrapper{child1, child2};
+  }();
+
+  auto agg = cudf::make_min_aggregation<groupby_aggregation>();
+  test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg));
+}
+
 }  // namespace test
 }  // namespace cudf

From d61bf52187ec4b3fd9bcff3e729987fd766fc37f Mon Sep 17 00:00:00 2001
From: Nghia Truong <nghiatruong.vn@gmail.com>
Date: Mon, 10 Jan 2022 17:10:11 -0700
Subject: [PATCH 2/2] Update copyright year

---
 cpp/src/reductions/struct_minmax_util.cuh | 2 +-
 cpp/tests/groupby/max_tests.cpp           | 2 +-
 cpp/tests/groupby/min_tests.cpp           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpp/src/reductions/struct_minmax_util.cuh b/cpp/src/reductions/struct_minmax_util.cuh
index bdce582b28d..e5832b849bd 100644
--- a/cpp/src/reductions/struct_minmax_util.cuh
+++ b/cpp/src/reductions/struct_minmax_util.cuh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, NVIDIA CORPORATION.
+ * Copyright (c) 2022, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/cpp/tests/groupby/max_tests.cpp b/cpp/tests/groupby/max_tests.cpp
index 29713bd923b..47bed11df30 100644
--- a/cpp/tests/groupby/max_tests.cpp
+++ b/cpp/tests/groupby/max_tests.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019-2021, NVIDIA CORPORATION.
+ * Copyright (c) 2019-2022, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/cpp/tests/groupby/min_tests.cpp b/cpp/tests/groupby/min_tests.cpp
index dad2ee3d621..64bffe1c883 100644
--- a/cpp/tests/groupby/min_tests.cpp
+++ b/cpp/tests/groupby/min_tests.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019-2021, NVIDIA CORPORATION.
+ * Copyright (c) 2019-2022, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.