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

ORC-1545: [JAVA] Use orc-format 1.0.0-SNAPSHOT #1683

Closed
wants to merge 2 commits into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Dec 6, 2023

What changes were proposed in this pull request?

This PR aims to use orc-format 1.0.0-SNAPSHOT in Java as an interim stage.

  1. We need to change C++ separately later.
  2. After (1), we can delete proto/orc_proto.proto.
  3. After releasing orc-format 1.0.0, we need to use it instead of SNAPSHOT.

Why are the changes needed?

To verify orc-format repository and snapshot.

How was this patch tested?

Pass the CIs.

@dongjoon-hyun
Copy link
Member Author

cc @williamhyun , @wgtmac , @guiyanakuang, @mystic-lama

@dongjoon-hyun dongjoon-hyun added this to the 2.0.0 milestone Dec 6, 2023
@dongjoon-hyun dongjoon-hyun changed the title ORC-1545: [JAVA] Use orc-format 1.0.0-SNAPSHOT ORC-1545: [JAVA] Use orc-format 1.0.0-SNAPSHOT Dec 6, 2023
@dongjoon-hyun
Copy link
Member Author

Merged to main for Apache ORC 2.0.0

@dongjoon-hyun dongjoon-hyun deleted the ORC-1545 branch December 6, 2023 19:10
@paliwalashish
Copy link
Contributor

Thanks @dongjoon-hyun

@paliwalashish
Copy link
Contributor

paliwalashish commented Dec 7, 2023

@dongjoon-hyun Do we need the shading section on ORC side? We are creating same files on orc-format build too. I am not too sure, so thought of asking

orc/java/pom.xml

Lines 489 to 512 in 54e28b0

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>${maven-shade-plugin.version}</version>
<executions>
<execution>
<id>shaded-protobuf</id>
<goals>
<goal>shade</goal>
</goals>
<phase>package</phase>
<configuration>
<artifactSet>
<includes>
<include>com.google.protobuf:protobuf-java</include>
</includes>
</artifactSet>
<shadedArtifactAttached>true</shadedArtifactAttached>
<shadedClassifierName>shaded-protobuf</shadedClassifierName>
<relocations>
<relocation>
<pattern>com.google.protobuf</pattern>
<shadedPattern>org.apache.orc.protobuf</shadedPattern>
</relocation>
</relocations>

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 7, 2023

To @mystic-lama , yes. I was confused like you. Please see here.

Since orc-format repo only shades orc-format*.jar, orc repo should shade orc-core, orc-mapreduce, and orc-shims with the same pattern consistently. Otherwise, the downstream cannot find APIs.

- orc-core/1.9.2/shaded-protobuf/orc-core-1.9.2-shaded-protobuf.jar
- orc-mapreduce/1.9.2/shaded-protobuf/orc-mapreduce-1.9.2-shaded-protobuf.jar
- orc-shims/1.9.2//orc-shims-1.9.2.jar
+ orc-core/2.0.0-SNAPSHOT/shaded-protobuf/orc-core-2.0.0-SNAPSHOT-shaded-protobuf.jar
+ orc-format/1.0.0-SNAPSHOT/shaded-protobuf/orc-format-1.0.0-SNAPSHOT-shaded-protobuf.jar
+ orc-mapreduce/2.0.0-SNAPSHOT/shaded-protobuf/orc-mapreduce-2.0.0-SNAPSHOT-shaded-protobuf.jar
+ orc-shims/2.0.0-SNAPSHOT//orc-shims-2.0.0-SNAPSHOT.jar

@paliwalashish
Copy link
Contributor

@dongjoon-hyun Thanks ! That explains it well. I was assuming to be the case, the concrete example helps.

cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
### What changes were proposed in this pull request?

This PR aims to use `orc-format` 1.0.0-SNAPSHOT in `Java` as an interim stage.

1. We need to change C++ separately later.
2. After (1), we can delete `proto/orc_proto.proto`.
3. After releasing `orc-format` 1.0.0, we need to use it instead of SNAPSHOT.

### Why are the changes needed?

To verify `orc-format` repository and snapshot.

### How was this patch tested?

Pass the CIs.

Closes apache#1683 from dongjoon-hyun/ORC-1545.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants