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

Add DELTA_BYTE_ARRAY encoder for Parquet #15239

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

etseidl
Copy link
Contributor

@etseidl etseidl commented Mar 6, 2024

Description

Re-submission of #14938. Final (delta) piece of #13501.

Adds the ability to encode Parquet pages as DELTA_BYTE_ARRAY. Python testing wlll be added as a follow-on when per-column encoding selection is added to the python API (ref this comment).

Checklist

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

@etseidl etseidl requested a review from a team as a code owner March 6, 2024 17:04
@etseidl etseidl requested review from vyasr and ttnghia March 6, 2024 17:04
Copy link

copy-pr-bot bot commented Mar 6, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Mar 6, 2024
@etseidl
Copy link
Contributor Author

etseidl commented Mar 6, 2024

cc @vuule @PointKernel @nvdbaranec

@vuule vuule added feature request New feature or request non-breaking Non-breaking change labels Mar 6, 2024
Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

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

Skipped the parts that haven't changed from #14938

cpp/tests/io/parquet_reader_test.cpp Outdated Show resolved Hide resolved
cpp/src/io/parquet/page_enc.cu Show resolved Hide resolved
@vuule
Copy link
Contributor

vuule commented Mar 6, 2024

/ok to test

@vuule vuule requested a review from PointKernel March 7, 2024 18:15
Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

cpp/src/io/parquet/page_enc.cu Show resolved Hide resolved
@PointKernel
Copy link
Member

/ok to test

@PointKernel
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 65fb218 into rapidsai:branch-24.04 Mar 8, 2024
73 checks passed
@etseidl etseidl deleted the delta_ba_deux branch March 8, 2024 07:00
rapids-bot bot pushed a commit that referenced this pull request May 8, 2024
test_parquet.py currently takes around 55s to run on an RTXA6000 system. A large portion of that run time is in two tests of the Parquet DELTA_LENGTH_BYTE_ARRAY and DELTA_BYTE_ARRAY encodings. These tests are parameterized with varying row counts to test certain encoding edge cases, but the final two row counts (10,000, 50,000) are unnecessarily large to provide adequate test coverage. This PR reduces the number of row counts (some were redundant) and decreases the maximum row count to 1,000.  This drops the execution time to just under 26s on the same system.

This PR also corrects an oversight from #15239. DELTA_BYTE_ARRAY encoding should have been added to the tests at that time.

Authors:
  - Ed Seidl (https://github.com/etseidl)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)
  - Matthew Roeschke (https://github.com/mroeschke)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #15693
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants