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.

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

Use make_empty_lists_column instead of make_empty_column(type_id::LIST) #13099

Merged
merged 8 commits into from
Apr 12, 2023

Conversation

davidwendt
Copy link
Contributor

Description

Fixes bug where cudf::make_empty_column(type_id::LIST) is called and adds a gtests to check for this error.
The make_empty_column cannot accept a nested type because it requires a child type. The internal make_empty_lists_column is moved to the lists_column_factories.hpp header which is itself moved to the cpp/include/cudf/lists/detail directory since it only contains detail functions.

Closes #13096

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Apr 10, 2023
@davidwendt davidwendt requested a review from a team as a code owner April 10, 2023 15:09
@davidwendt davidwendt self-assigned this Apr 10, 2023
@davidwendt davidwendt requested a review from a team as a code owner April 10, 2023 15:09
@github-actions github-actions bot added the conda label Apr 10, 2023
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving ops-codeowner file changes

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making sure that I understand, this is a code path that was never taken before? Otherwise we should have hit the CUDF_EXPECTS in make_empty_column that prohibits nested types, correct?

@davidwendt
Copy link
Contributor Author

Just making sure that I understand, this is a code path that was never taken before? Otherwise we should have hit the CUDF_EXPECTS in make_empty_column that prohibits nested types, correct?

That is correct. I thought I had a test for this as well but missed this case.

@davidwendt
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 9a9f718 into rapidsai:branch-23.06 Apr 12, 2023
@davidwendt davidwendt deleted the bug-make-empty-list branch April 12, 2023 12:32
rapids-bot bot pushed a commit that referenced this pull request Apr 13, 2023
Resolves #11754. The `byte_cast` function is creating unsanitized lists from null inputs, which is a bug. [This logic](https://github.com/rapidsai/cudf/blob/9c06330363db4da99803a3728b8bf44f9829f0b9/cpp/src/reshape/byte_cast.cu#L66-L81) copies nonzero bytes even if the input element is null. The input's null mask is copied onto the output parent list column, but the null children are nonempty. This PR fixes the bug by calling `cudf::purge_nonempty_nulls` on the result before returning, if there are any nulls to be purged.

Depends on:
 * #13099

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Nghia Truong (https://github.com/ttnghia)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Mike Wilson (https://github.com/hyperbolic2346)

URL: #11971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] make_empty_column is called on type_id::LIST input
4 participants