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.

Sign up for GitHub

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

Jump to bottom

ORC-1551: Use orc-format 1.0.0-beta #1688

Closed
wants to merge 1 commit into from
Closed

ORC-1551: Use orc-format 1.0.0-beta #1688

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Dec 9, 2023 •
edited
Loading

What changes were proposed in this pull request?

This PR aims to use orc-format 1.0.0-beta.

Why are the changes needed?

1.0.0-beta has the following changes.

How was this patch tested?

Pass the CIs.

@dongjoon-hyun dongjoon-hyun changed the title ORC-1551: [C++] Use orc-format 1.0.0-beta ORC-1551: [C++] Use orc-format 1.0.0-beta Dec 9, 2023
@dongjoon-hyun dongjoon-hyun changed the title ORC-1551: [C++] Use orc-format 1.0.0-beta ORC-1551: Use orc-format 1.0.0-beta Dec 9, 2023
@github-actions github-actions bot added the JAVA label Dec 9, 2023
@dongjoon-hyun dongjoon-hyun force-pushed the ORC-1551 branch 2 times, most recently from 5f38535 to f6d48dd Compare December 9, 2023 01:48
@dongjoon-hyun
Copy link
Member Author

cc @williamhyun , @wgtmac , @guiyanakuang, @stiga-huang , @coderex2522 , @deshanxiao

wgtmac
wgtmac approved these changes Dec 9, 2023
View reviewed changes
Copy link
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

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

Thanks @dongjoon-hyun!

java/core/src/test/org/apache/orc/TestVectorOrcFile.java
assertTrue(m.contains("Postscript: footerLength: 19 compression: NONE " +
"compressionBlockSize: 65536 version: 19 version: 99 metadataLength: 0 " +
"writerVersion: 1"));
assertTrue(m.contains("Postscript: footer_length: 19 compression: NONE " +
Copy link
Member

Choose a reason for hiding this comment

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

I assume this file should not be affected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya, the error message contains the protobuf field names.

@dongjoon-hyun
Copy link
Member Author

Thank you, @wgtmac and @guiyanakuang .

To start and maintain Apache ORC Format 1.0.0 in the official ProtocolBuffer way, this is the minimum required change due to the following.

As we know, ProtocolBuffers is a safe protocol for field renaming in terms of binary compatibility. So, Apache ORC 1.x libraries (Java/C++) can read and write ORC Format v1.0 files without any problems. However, the generated C++ code and dump meta info is different like we see in this PR.

The new C++ API and dump meta data follows an official ProtocolBuffers recommendation. So, I believe we can help and persuade the down-streams like Apache Arrow easily without many blockers.

@dongjoon-hyun
Copy link
Member Author

Let me merge this~

@dongjoon-hyun dongjoon-hyun deleted the ORC-1551 branch December 9, 2023 05:48
@dongjoon-hyun dongjoon-hyun added this to the 2.0.0 milestone Dec 9, 2023
cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
@dongjoon-hyun @cxzl25
ORC-1551: Use orc-format 1.0.0-beta
b2101dd 
### What changes were proposed in this pull request?

This PR aims to use `orc-format` `1.0.0-beta`.

### Why are the changes needed?

`1.0.0-beta` has the following changes.
- apache/orc-format#5
- apache/orc-format#4
- apache/orc-format#7
- apache/orc-format#9

### How was this patch tested?

Pass the CIs.

Closes apache#1688 from dongjoon-hyun/ORC-1551.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wgtmac wgtmac wgtmac approved these changes

Assignees
No one assigned
Labels
BUILD CPP JAVA
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dongjoon-hyun @wgtmac