[FEA] Verify that no nonempty nulls exist in testing macros #12786
Labels
0 - Backlog
In queue waiting for assignment
feature request
New feature or request
Spark
Functionality that helps Spark RAPIDS
tests
Unit testing for project
Comments
vyasr
added
feature request
|
I can look into this, thanks! |
|
@SurajAralihalli thanks! Let us know if you need any help with this. |
3 tasks
rapids-bot bot
pushed a commit
that referenced
this issue
Dec 18, 2023
…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
|
Amazing work @SurajAralihalli !! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
libcudf functions expect nulls to be sanitized (see "libcudf expects nested types to have sanitized null masks" at https://docs.rapids.ai/api/libcudf/stable/developer_guide). Providing unsanitized inputs to libcudf functions may result in incorrect results. We are willing to accept putting the onus on the user to ensure that any manually constructed inputs are properly sanitized in this way. However, currently this approach can lead to hidden bugs if a libcudf developer implements an API that produces unsanitized columns, something that we do not test for. Adding tests for a particular API to ensure that nulls are sanitized seems helpful on its face, but in practice it requires identifying APIs that need this check a priori, severely reducing the effectiveness of this approach. Therefore, an alternative approach is needed that automatically verifies these invariants for all tests without any additional work on the part of developers.
Describe the solution you'd like
We should modify our
CUDF_TESTS_EXPECTS_*assertion macros to verify that the input columns are properly sanitized prior to performing any comparison. If the inputs columns are unsanitized, a suitable error should be raised even if the intended assertion would otherwise have passed. This approach automatically injects sanitization validation into all tests and ensures that no libcudf APIs produce unsanitized outputs.The text was updated successfully, but these errors were encountered: