Skip to content

Commit

Permalink
Add column sanitization checks in CUDF_TEST_EXPECT_COLUMN_* macros (#…
Browse files Browse the repository at this point in the history
…14559)

This PR addresses Issue #[12786](#12786)

The listed functions have been modified to incorporate a column sanitization check; otherwise, they will raise a `std::invalid_argument` error.
- `expect_column_properties_equal`
- `expect_column_properties_equivalent`
- `expect_columns_equal`
- `expect_columns_equivalent`

Authors:
  - Suraj Aralihalli (https://github.com/SurajAralihalli)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - MithunR (https://github.com/mythrocks)

URL: #14559
  • Loading branch information
SurajAralihalli authored Dec 18, 2023
1 parent 90cccef commit 3602816
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 37 deletions.
17 changes: 17 additions & 0 deletions cpp/tests/utilities/column_utilities.cu
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <cudf/column/column_view.hpp>
#include <cudf/copying.hpp>
#include <cudf/detail/copy.hpp>
#include <cudf/detail/get_value.cuh>
#include <cudf/detail/iterator.cuh>
#include <cudf/detail/utilities/vector_factories.hpp>
Expand Down Expand Up @@ -801,6 +802,18 @@ struct column_comparator {
}
};

void check_non_empty_nulls(column_view const& lhs, column_view const& rhs)
{
auto check_column_nulls = [](column_view const& col, const char* col_name) {
if (cudf::detail::has_nonempty_nulls(col, cudf::get_default_stream())) {
throw std::invalid_argument(col_name + std::string(" column has non-empty nulls"));
}
};

check_column_nulls(lhs, "lhs");
check_column_nulls(rhs, "rhs");
}

} // namespace

namespace detail {
Expand All @@ -811,6 +824,7 @@ bool expect_column_properties_equal(column_view const& lhs,
column_view const& rhs,
debug_output_level verbosity)
{
check_non_empty_nulls(lhs, rhs);
auto lhs_indices = generate_all_row_indices(lhs.size());
auto rhs_indices = generate_all_row_indices(rhs.size());
return cudf::type_dispatcher(lhs.type(),
Expand All @@ -829,6 +843,7 @@ bool expect_column_properties_equivalent(column_view const& lhs,
column_view const& rhs,
debug_output_level verbosity)
{
check_non_empty_nulls(lhs, rhs);
auto lhs_indices = generate_all_row_indices(lhs.size());
auto rhs_indices = generate_all_row_indices(rhs.size());
return cudf::type_dispatcher(lhs.type(),
Expand All @@ -847,6 +862,7 @@ bool expect_columns_equal(cudf::column_view const& lhs,
cudf::column_view const& rhs,
debug_output_level verbosity)
{
check_non_empty_nulls(lhs, rhs);
auto lhs_indices = generate_all_row_indices(lhs.size());
auto rhs_indices = generate_all_row_indices(rhs.size());
return cudf::type_dispatcher(lhs.type(),
Expand All @@ -867,6 +883,7 @@ bool expect_columns_equivalent(cudf::column_view const& lhs,
debug_output_level verbosity,
size_type fp_ulps)
{
check_non_empty_nulls(lhs, rhs);
auto lhs_indices = generate_all_row_indices(lhs.size());
auto rhs_indices = generate_all_row_indices(rhs.size());
return cudf::type_dispatcher(lhs.type(),
Expand Down
37 changes: 0 additions & 37 deletions cpp/tests/utilities_tests/column_utilities_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,43 +281,6 @@ TEST_F(ColumnUtilitiesListsTest, DifferingRowCounts)
a, b, cudf::test::debug_output_level::QUIET));
}

TEST_F(ColumnUtilitiesListsTest, UnsanitaryLists)
{
// unsanitary
//
// List<int32_t>:
// Length : 1
// Offsets : 0, 3
// Null count: 1
// 0
// 0, 1, 2
std::vector<std::unique_ptr<cudf::column>> children;
children.emplace_back(
std::move(cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 3}.release()));
children.emplace_back(std::move(cudf::test::fixed_width_column_wrapper<int>{0, 1, 2}.release()));

auto l0 = std::make_unique<cudf::column>(cudf::data_type{cudf::type_id::LIST},
1,
rmm::device_buffer{},
cudf::create_null_mask(1, cudf::mask_state::ALL_NULL),
1,
std::move(children));

// sanitary
//
// List<int32_t>:
// Length : 1
// Offsets : 0, 0
// Null count: 1
// 0
auto l1 = cudf::test::lists_column_wrapper<int>::make_one_empty_row_column(false);

// equivalent, but not equal
CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*l0, l1);
EXPECT_FALSE(
cudf::test::detail::expect_columns_equal(*l0, l1, cudf::test::debug_output_level::QUIET));
}

TEST_F(ColumnUtilitiesListsTest, DifferentPhysicalStructureBeforeConstruction)
{
// list<int>
Expand Down

0 comments on commit 3602816

Please sign in to comment.