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

Fix collect_set on struct type #4996

Merged
merged 2 commits into from
Mar 22, 2022

Conversation

sperlingxx
Copy link
Collaborator

@sperlingxx sperlingxx commented Mar 21, 2022

During the development of collect-like reduction aggregation #4992, I found that CudfCollectSet and CudfMergeSets apply wrong NullEquality, which should be NullEquality.EQUAL. This problem only affects struct types because collect ops apply NullPolicy.EXCLUDE, which means null entries will be excluded during the collection. However, with struct type, nulls wrapped inside outer-level struct will trigger the NullEquality problem.

Since #4992 is targetted to release 22.06 , I create a separate PR to fix this problem.

Signed-off-by: sperlingxx lovedreamf@gmail.com

Signed-off-by: sperlingxx <lovedreamf@gmail.com>
@sperlingxx sperlingxx added the bug Something isn't working label Mar 21, 2022
@sperlingxx
Copy link
Collaborator Author

build

@firestarman
Copy link
Collaborator

I saw #4992 is targeted to release 22.04.

@sperlingxx
Copy link
Collaborator Author

I saw #4992 is targeted to release 22.04.

The branch-22.06 has not been created yet.

@jlowe jlowe added this to the Mar 21 - Apr 1 milestone Mar 21, 2022
@abellina
Copy link
Collaborator

Overall LGTM, just had one question on a test.

Copy link
Collaborator

@abellina abellina left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like a +1 from @revans2 or @jlowe also. I am not an expert at nested types.

@sperlingxx sperlingxx merged commit 7e0e66d into NVIDIA:branch-22.04 Mar 22, 2022
@sperlingxx sperlingxx deleted the fix_collect_set_struct branch March 22, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants