Skip to content

ARROW-17798: [C++][Parquet] Add DELTA_BINARY_PACKED encoder to Parquet writer#14191

Merged
pitrou merged 26 commits into
apache:masterfrom
rok:ARROW-17798
Dec 14, 2022
Merged

ARROW-17798: [C++][Parquet] Add DELTA_BINARY_PACKED encoder to Parquet writer#14191
pitrou merged 26 commits into
apache:masterfrom
rok:ARROW-17798

Conversation

@rok

@rok rok commented Sep 21, 2022

Copy link
Copy Markdown
Member

This is to add DELTA_BINARY_PACKED encoder.

@rok

rok commented Sep 21, 2022

Copy link
Copy Markdown
Member Author

This is not really review ready yet.

@github-actions

github-actions Bot commented Sep 21, 2022

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-17798

winval
winval reviewed Sep 22, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
@rok rok force-pushed the ARROW-17798 branch 5 times, most recently from bd0f693 to 641dfd6 Compare September 30, 2022 04:16
@rok rok force-pushed the ARROW-17798 branch 3 times, most recently from b67d895 to 4217379 Compare October 2, 2022 21:44
@rok rok force-pushed the ARROW-17798 branch 3 times, most recently from 36f33cb to 786e689 Compare October 3, 2022 03:19
@rok rok marked this pull request as ready for review October 3, 2022 11:50
@rok rok mentioned this pull request Oct 3, 2022
Merged
@pitrou

pitrou commented Oct 3, 2022

Copy link
Copy Markdown
Member

@shanhuuang Do you want to take a look here?

pitrou
pitrou reviewed Oct 3, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
pitrou
pitrou reviewed Oct 3, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
pitrou
pitrou reviewed Oct 3, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
pitrou
pitrou reviewed Oct 3, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
pitrou
pitrou reviewed Oct 3, 2022
View reviewed changes
Comment thread cpp/src/parquet/encoding.cc Outdated
@rok rok force-pushed the ARROW-17798 branch 2 times, most recently from 277dee6 to 4d4c10f Compare October 3, 2022 16:03
@rok rok mentioned this pull request Oct 7, 2022
Merged
rok and others added 17 commits December 14, 2022 17:01
@rok @wgtmac @pitrou
Update cpp/src/parquet/encoding.cc
8064b7f 
Co-authored-by: Gang Wu <ustcwg@gmail.com>
@rok @pitrou
Review feedback
b29ab91
@rok @wjones127 @pitrou
Apply suggestions from code review
09b8a32 
Co-authored-by: Will Jones <willjones127@gmail.com>
@rok @pitrou
Review feedback
a8c3d28
@rok @pitrou
Linting
66b04ff
@rok @pitrou
Apply suggestions from code review
a7f1cea 
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@rok @pitrou
Review feeback
068ea81
@rok @pitrou
Review feedback
87eb662
@rok @pitrou
Enable int64
ed68e54
@rok @pitrou
Review feedback
586c324
@rok @pitrou
Apply suggestions from code review
c25ae52 
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@rok @pitrou
Review feedback.
62ae225
@rok @pitrou
Adding bitwidths for unpopulated blocks.
90999e1
@rok @pitrou
Review feedback
e1000af
@rok @pitrou
Apply suggestions from code review
702b7a8 
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@rok @pitrou
Review feedback
d46251c
@rok @pitrou
Apply suggestions from code review
01271e7 
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@pitrou

pitrou commented Dec 14, 2022

Copy link
Copy Markdown
Member

Github isn't showing the CI jobs for some reason but they are running:

@pitrou pitrou merged commit 1b3d4af into apache:master Dec 14, 2022
@pitrou

pitrou commented Dec 14, 2022

Copy link
Copy Markdown
Member

I'm glad this is in! Congrats!

@rok

rok commented Dec 14, 2022

Copy link
Copy Markdown
Member Author

Thanks for all the reviews @pitrou & others :)

@mapleFU

mapleFU commented Dec 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

@rok @pitrou @wgtmac
Hi, all, I found a bug here. When calling flushValues, it didn't:

  • clearing the total_value_count_
  • Re-advancing buffer for kMaxPageHeaderWriterSize

I have a bug fixing, and I submit a fixing here: #14959

@ursabot

ursabot commented Dec 15, 2022

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 36824d1 and contender = 1b3d4af. 1b3d4af is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.43% ⬆️0.0%] test-mac-arm
[Finished ⬇️1.9% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️1.1% ⬆️0.34%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 1b3d4afc ec2-t3-xlarge-us-east-2
[Finished] 1b3d4afc test-mac-arm
[Finished] 1b3d4afc ursa-i9-9960x
[Finished] 1b3d4afc ursa-thinkcentre-m75q
[Finished] 36824d1b ec2-t3-xlarge-us-east-2
[Finished] 36824d1b test-mac-arm
[Finished] 36824d1b ursa-i9-9960x
[Finished] 36824d1b ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@asfimport asfimport mentioned this pull request Dec 15, 2022
Closed
@anjakefala anjakefala mentioned this pull request Feb 6, 2023
Closed
@asfimport asfimport mentioned this pull request Nov 18, 2022
Open
@asfimport asfimport mentioned this pull request Jun 23, 2024
Closed
@rok rok mentioned this pull request Sep 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zeroshade zeroshade zeroshade left review comments
@wgtmac wgtmac wgtmac left review comments
@pitrou pitrou pitrou approved these changes
@wjones127 wjones127 wjones127 approved these changes
+1 more reviewer
@winval winval winval left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++ Component: Documentation Component: Parquet Component: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@rok @pitrou @wjones127 @wgtmac @mapleFU @ursabot @zeroshade @winval