You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "deshanxiao (via GitHub)" <gi...@apache.org> on 2023/07/24 08:13:43 UTC
[GitHub] [orc] deshanxiao opened a new pull request, #1565: ORC-1463: [Java] Support Brotli codec
deshanxiao opened a new pull request, #1565:
URL: https://github.com/apache/orc/pull/1565
### What changes were proposed in this pull request?
This PR is aimed to add Brotli codec defined by [RFC7932](https://www.ietf.org/rfc/rfc7932.txt).
### Why are the changes needed?
To support more codec in ORC.
### How was this patch tested?
UT and CI pipeline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1869024723
Option 2 looks best on my side. Thanks @deshanxiao!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1848111954
Thank you @dongjoon-hyun !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1843483822
@wgtmac No~ This PR (Java) implementation is enough to add BROTLI to `orc-format`. C++ implementation can follow later.
> Do we need a PoC implementation on the C++ side before making a decision?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao closed pull request #1565: [DRAFT] ORC-1463: [Java] Support Brotli codec
URL: https://github.com/apache/orc/pull/1565
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1869218266
Thank you @dongjoon-hyun @wgtmac . Please let me re-implement a version based on brotli4j.
Close this PR and will create a new one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1842119191
Do we need a PoC implementation on the C++ side before making a decision? AFAIK, official C++ brotli library is complete and it is used in the Apache Arrow for years.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] wgtmac commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1288564335
##########
java/shims/src/java/org/apache/orc/impl/HadoopShims.java:
##########
@@ -35,6 +35,7 @@ enum DirectCompressionType {
ZLIB_NOHEADER,
ZLIB,
SNAPPY,
+ BROTLI,
Review Comment:
IIUC, shims here are used to workaround codec provided by Hadoop libraries of different versions. It seems that we do not need this line.
##########
proto/orc_proto.proto:
##########
@@ -386,6 +386,7 @@ enum CompressionKind {
LZO = 3;
LZ4 = 4;
ZSTD = 5;
+ BROTLI = 6;
Review Comment:
In order to check this in, we'd better update the website and the C++ code (at least throw an unimplemented exception on the reader side)
##########
java/core/pom.xml:
##########
@@ -72,6 +72,10 @@
<groupId>org.threeten</groupId>
<artifactId>threeten-extra</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
Review Comment:
I found this one but not sure if it is mature enough: https://github.com/hyperxpro/Brotli4j
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1844200451
BTW, please use a title `Apache ORC Format 2.0` because you are referring `ORC v2 (https://orc.apache.org/specification/ORCv2/)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1844199159
Could you send an email to dev@orc mailing list, @wgtmac ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1672964070
> Before diving into the detail implementation, I just want to make sure if brotli is widely used compared to others. Google itself does not provide an encoder implementation in pure Java. If the benefit it brings does not worth the effort of supporting it, I would rather not do this.
From the test results of this [Jira](https://issues.apache.org/jira/browse/HADOOP-13126), I am positive about supporting brotli.
I add an [Issue](https://github.com/google/brotli/issues/1060) to track it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1661518300
Hi @dongjoon-hyun, the pipeline has passed. If you have any comments for the PR. Please ping me, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1664141550
BTW, do you know why [HADOOP-13126](https://issues.apache.org/jira/browse/HADOOP-13126) is still open?
And, Hadoop JIRA seems to say the following. Could you double-check if there is no license issue or not?
> [Brotli is licensed with the MIT license](https://github.com/google/brotli/blob/master/LICENSE), and the [JNI library jbrotli is ALv2](https://github.com/MeteoGroup/jbrotli/blob/master/LICENSE).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1847903400
Okay, let me make a PR for you, @deshanxiao .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1844171368
BTW, as parquet feature parity has been mentioned here, I have a couple of questions:
- Do we need to formally start the action?
- Should we also keep an eye on the lance format?
- Is it a good time to revisit the TODO items on ORC v2 (https://orc.apache.org/specification/ORCv2/)?
@dongjoon-hyun @mystic-lama
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1868606489
Do you think you can resume this, @deshanxiao ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1837942164
> Is there any update from the upstream, @deshanxiao ? BTW, I removed my previous review because it's too old one.
Hi @dongjoon-hyun I check the upstream repo and no update yet. I think we may need a new approach to achieve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1869354073
cc @williamhyun too for the above context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1648935854
It seems that pom file has some foramt errors. Using `mvn spotless:apply` can auto fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1664968517
> BTW, do you know why [HADOOP-13126](https://issues.apache.org/jira/browse/HADOOP-13126) is still open?
>
It seems that the reviewer think this modification is too large to cause the Hadoop PR freeze.
See https://github.com/apache/hadoop/pull/2723.
> And, Hadoop JIRA seems to say the following. Could you double-check if there is no license issue or not?
>
> > [Brotli is licensed with the MIT license](https://github.com/google/brotli/blob/master/LICENSE), and the [JNI library jbrotli is ALv2](https://github.com/MeteoGroup/jbrotli/blob/master/LICENSE).
From the dependencies:
https://github.com/rdblue/brotli-codec/blob/9bbc2bcec1e7307e84552f28e90f20cb3099f189/build.gradle#L58
We are only indirectly referencing `jbrotli`. So the license requirement you mentioned is right.
BTW, `brotli-codec` is just an adapter to meet with hadoop codec framework. We can use jbrotli directly. What do you think? @dongjoon-hyun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1647454532
`com.github.rdblue:brotli-codec` has no binary for aarch64. We can skip it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1271934962
##########
java/core/pom.xml:
##########
@@ -72,6 +72,10 @@
<groupId>org.threeten</groupId>
<artifactId>threeten-extra</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
Review Comment:
Is this the only way?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1672843168
> I found this one but not sure if it is mature enough: https://github.com/hyperxpro/Brotli4j
From
https://issues.apache.org/jira/browse/PARQUET-1975?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel&focusedCommentId=17281798#comment-17281798
Looks like someone else has had a similar idea. It seems that someone has mentioned a similar idea, but for some reason, parquet still uses a temporary maven library.
I checked that netty is using this library in:
https://github.com/netty/netty/blob/1c5053740dccc4ec952b4cfb8c4bcf697b49d9b6/pom.xml#L763
We could try to re-implement with it @wgtmac
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1869353698
Thank you for leading this area, @deshanxiao .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1846592784
To @deshanxiao , could you make a PR to `https://github.com/apache/orc-format` repository for this line?
```
BROTLI = 6;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1271947405
##########
java/core/pom.xml:
##########
@@ -72,6 +72,10 @@
<groupId>org.threeten</groupId>
<artifactId>threeten-extra</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
Review Comment:
I tried looking for more common repositories, but couldn't find them. Here is parquet pom file: https://github.com/apache/parquet-mr/blob/d2c3c6d2e761a17b75d56fd356a37a2f754072f7/parquet-hadoop/pom.xml#L207
I'm glad to update it if we can find a better one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1660629341
I checked but it seems the workflow is triggered already, @deshanxiao .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1868833757
> May I ask if you think you can resume this, @deshanxiao ?
Sure, this work will resume soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1674153622
+1 for pausing. Thank you, @deshanxiao .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1289902354
##########
java/shims/src/java/org/apache/orc/impl/HadoopShims.java:
##########
@@ -35,6 +35,7 @@ enum DirectCompressionType {
ZLIB_NOHEADER,
ZLIB,
SNAPPY,
+ BROTLI,
Review Comment:
Thanks I will remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1837945771
+1 for add this to orc-format repo.
BTW, before implementing the feature, please give me some time to summarize the pros and cons of all third-party libraries.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1847908410
This is the PR.
- https://github.com/apache/orc-format/pull/7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1868849123
The following is a comparison of the various implementation options we can take:
### Option 1: (Parquet mode):
Use the same way with parquet to support brotli codec, which will use a private maven repository with optional and runtime dependency.
#### Pros:
1.This method has been tested in practice and has been adopted by parquet.
#### Cons:
1. aarch64 is not supported.
2. We will have to use private maven repository. The code in this repository has not been updated in many years.
### Option 2: brotli4j (Netty mode):
#### Pros:
1. brotli4j is still under maintenance and supports all platforms.
2. license check is passed.
#### Cons:
1.This solution was mentioned in the parquet community, but in the end, Option 1 was adopted. The specific reason is unknown.
### Option 3: google/brotli:
#### Pros:
1.Official support.
#### Cons:
1.No brotli encoder. We need to wait for official support.
At present, I prefer `Option 2`. What do you think? @dongjoon-hyun @mystic-lama @wgtmac
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1289781696
##########
proto/orc_proto.proto:
##########
@@ -386,6 +386,7 @@ enum CompressionKind {
LZO = 3;
LZ4 = 4;
ZSTD = 5;
+ BROTLI = 6;
Review Comment:
Thanks @wgtmac I will do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1271957153
##########
java/shims/pom.xml:
##########
@@ -42,6 +42,10 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
+ <artifactId>brotli-codec</artifactId>
+ </dependency>
Review Comment:
Got it, let me try to re-implement this part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] ORC-1463: [Java] Support Brotli codec [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1869145475
Thank you, @deshanxiao . I agree with you. +1 for Option 2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1674153216
As mentioned in this https://github.com/google/brotli/issues/1060, we could wait for official support to come out.
I will pause this PR until its dependencies are ready, what do you think? @dongjoon-hyun @wgtmac
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1271936614
##########
java/pom.xml:
##########
@@ -271,6 +272,12 @@
<version>1.1.1</version>
<optional>true</optional>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
+ <artifactId>brotli-codec</artifactId>
+ <version>${brotli-codec.version}</version>
+ <optional>true</optional>
Review Comment:
Oh, got it. Apache ORC community has only an `Optional` dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1565:
URL: https://github.com/apache/orc/pull/1565#discussion_r1271938092
##########
java/shims/pom.xml:
##########
@@ -42,6 +42,10 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.rdblue</groupId>
+ <artifactId>brotli-codec</artifactId>
+ </dependency>
Review Comment:
Sorry, but this looks wrong to me. Please do not add the compression dependency to `shims` module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1659910282
Disable unit test when arch is `aarch64`.
Hi @dongjoon-hyun Could you help me trigger the Workflow? Thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] deshanxiao commented on pull request #1565: ORC-1463: [Java] Support Brotli codec
Posted by "deshanxiao (via GitHub)" <gi...@apache.org>.
deshanxiao commented on PR #1565:
URL: https://github.com/apache/orc/pull/1565#issuecomment-1661380132
Thanks @guiyanakuang for trigger the workflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org