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 ORC writer crash with empty input columns #9808

Merged
merged 5 commits into from
Dec 2, 2021

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Dec 1, 2021

Fixes #9783

Skip some parts of writing when the input table was zero rows.
Add is_empty to hostdevice_2dvector.
Add Python test with empty columns.

@vuule vuule added the cuIO cuIO issue label Dec 1, 2021
@vuule vuule self-assigned this Dec 1, 2021
@github-actions github-actions bot added Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Dec 1, 2021
@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #9808 (c9f73ad) into branch-22.02 (967a333) will decrease coverage by 0.01%.
The diff coverage is 12.35%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.02    #9808      +/-   ##
================================================
- Coverage         10.49%   10.47%   -0.02%     
================================================
  Files               119      119              
  Lines             20305    20368      +63     
================================================
+ Hits               2130     2133       +3     
- Misses            18175    18235      +60     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <ø> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <ø> (ø)
python/cudf/cudf/core/indexed_frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/multiindex.py 0.00% <0.00%> (ø)
python/cudf/cudf/utils/utils.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/csv.py 96.00% <100.00%> (+0.10%) ⬆️
python/dask_cudf/dask_cudf/io/tests/test_csv.py 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20d6723...c9f73ad. Read the comment docs.

@vuule vuule added bug Something isn't working non-breaking Non-breaking change labels Dec 1, 2021
@vuule vuule marked this pull request as ready for review December 1, 2021 19:34
@vuule vuule requested review from a team as code owners December 1, 2021 19:34
@devavret
Copy link
Contributor

devavret commented Dec 1, 2021

That's a lot of lines changed. Can you point me to the core of the changes which fix the issue?

@vuule
Copy link
Contributor Author

vuule commented Dec 1, 2021

That's a lot of lines changed. Can you point me to the core of the changes which fix the issue?

It's actually a small change, and all of it is relevant for the fix. The line count comes from added code blocks. Please try reviewing with "Hide whitespace" on.

Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

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

Looks good overall. Couple small suggestions

cpp/src/io/orc/writer_impl.cu Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Show resolved Hide resolved
cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved
@vuule vuule requested a review from devavret December 1, 2021 23:53
@vuule vuule requested a review from codereport December 1, 2021 23:53
Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

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

🔥

@vuule
Copy link
Contributor Author

vuule commented Dec 2, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b848dd5 into rapidsai:branch-22.02 Dec 2, 2021
rapids-bot bot pushed a commit that referenced this pull request Dec 14, 2021
Follow up of #9808
Skips some kernels when input columns are empty to avoid OOB memory access.

Authors:
  - Vukasin Milovanovic (https://github.com/vuule)

Approvers:
  - Devavret Makkar (https://github.com/devavret)
  - Nghia Truong (https://github.com/ttnghia)
  - David Wendt (https://github.com/davidwendt)

URL: #9896
@vuule vuule deleted the bug-orc-write-empty-columns branch April 20, 2022 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ORC writing failed when table is empty
6 participants