Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix struct flattening to add a validity column only when the input column has null element #8374

Merged
merged 4 commits into from
May 27, 2021
Merged

Fix struct flattening to add a validity column only when the input column has null element #8374

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
172f848
Use `has_nulls()` instead of `nullable()` during flattening of struct…
ttnghia May 26, 2021
238d9e2
Merge branch 'branch-21.06' into fix_struct_flattening
ttnghia May 26, 2021
5596785
Fix doc
ttnghia May 26, 2021
224eb13
Add more explanation to the comments
ttnghia May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions cpp/src/structs/utilities.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,22 @@ struct flattened_table {
order col_order,
null_order col_null_order)
{
if (nullability == column_nullability::FORCE || col.nullable()) {
// nullable columns could be required for comparisions such as joins
// Even if it is not required to extract the bitmask to a separate column,
// we should always do that if the structs column has any null element.
//
// In addition, we should check for null by calling to `has_nulls()`, not `nullable()`.
// This is because when comparing structs columns, if one column has bitmask while the other
// does not (and both columns do not have any null element) then flattening them using
// `nullable()` will result in tables with different number of columns.
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
//
// Notice that, for comparing structs columns when one column has null while the other
// doesn't, `nullability` must be passed in with value `column_nullability::FORCE` to make
// sure the flattening results are tables having the same number of columns.

if (nullability == column_nullability::FORCE || col.has_nulls()) {
validity_as_column.push_back(cudf::is_valid(col));
if (col.nullable()) {
// copy bitmask only works if the column is nullable
if (col.has_nulls()) {
// copy bitmask is needed only if the column has null
validity_as_column.back()->set_null_mask(copy_bitmask(col));
}
flat_columns.push_back(validity_as_column.back()->view());
Expand Down