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 null counts for output of row/column conversion: #1170

Merged

Conversation

mythrocks
Copy link
Collaborator

Fixes #1169.

This commit fixes the null counts in row/column conversion, for all output columns.
The prior fix in #1155 only fixed the count for STRING outputs.

There are 2 additional/tangential fixes in this change:

  1. cudf::detail::null_count() is used in place of cudf::null_count(), thus running on the current stream.
  2. The Java RowConversion.convertFromRowsFixedWidthOptimized() now uses the convertFromRowsFixedWidthOptimized() native function instead of convertFromRows(). This should prove faster.

@mythrocks mythrocks self-assigned this May 25, 2023
@mythrocks mythrocks requested a review from hyperbolic2346 May 25, 2023 00:50
@mythrocks mythrocks added the bug Something isn't working label May 25, 2023
Fixes NVIDIA#1169.

This commit fixes the null counts in row/column conversion, for all output
columns.
The prior fix in NVIDIA#1155 only fixed the count for STRING outputs.

There are 2 additional/tangential fixes in this change:
  1. `cudf::detail::null_count()` is used in place of `cudf::null_count()`,
     thus running on the current stream.
  2. The Java `RowConversion.convertFromRowsFixedWidthOptimized()` now uses
     the `convertFromRowsFixedWidthOptimized()` native function instead of
     `convertFromRows()`. This should prove faster.

Signed-off-by: MithunR <[email protected]>
@mythrocks mythrocks force-pushed the column-null-count-followup branch from 3955a2e to 1c0dbc0 Compare May 25, 2023 00:51
@mythrocks
Copy link
Collaborator Author

Build

Copy link
Collaborator

@hyperbolic2346 hyperbolic2346 left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fix

@mythrocks
Copy link
Collaborator Author

mythrocks commented May 25, 2023

Thank you for the quick fix.

All it took was 3 PRs (and counting). :))

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.

[BUG] RowConversionTest fails after UNKNOWN_NULL_COUNT changes
2 participants