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

Bump to Parquet 1.14.1 #10209

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Apr 23, 2024

To assess the impact of the changes.

Blocked by #8485

build.gradle Outdated
}
dependencies {
classpath 'com.github.johnrengelman:shadow:8.1.1'
classpath "io.github.goooler.shadow:shadow-gradle-plugin:8.1.7"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an issue with the plugin: GradleUp/shadow#911

@github-actions github-actions bot removed the INFRA label May 4, 2024
@Fokko Fokko force-pushed the fd-validate-parquet-1-14 branch 2 times, most recently from 5c3b0bc to 8d3ce50 Compare May 5, 2024 09:45
@Fokko Fokko force-pushed the fd-validate-parquet-1-14 branch 3 times, most recently from 7fb72a2 to e0b7277 Compare May 5, 2024 20:25
@Fokko
Copy link
Contributor Author

Fokko commented May 5, 2024

The tests are failing, but that's because Jackson ships with JDK21-specific code, and the shadow plugin cannot cope with that. There is a fork of the plugin, but that one requires Gradle 8.3+. Gradle plugins are blocked because revapi is not being maintained and does not work with Gradle >=8.1.1. @jbonofre is digging into bumping Gradle by running RevAPI standalone 🙏 I ran ./gradlew clean test locally and everything passed.

@jbonofre
Copy link
Member

jbonofre commented May 7, 2024

Yes, I will move forward on the Gradle/revapi PR (a bit busy with a few Avro stuff 😄 ).

@ajantha-bhat ajantha-bhat added this to the Iceberg 1.6.0 milestone May 30, 2024
@Fokko Fokko mentioned this pull request Jun 11, 2024
@Fokko Fokko changed the title Bump to Parquet 1.14.0 Bump to Parquet 1.14.1 Jun 25, 2024
@Fokko Fokko marked this pull request as ready for review June 25, 2024 13:57
@Fokko Fokko closed this Jun 26, 2024
@Fokko Fokko reopened this Jun 26, 2024
@Fokko
Copy link
Contributor Author

Fokko commented Jun 27, 2024

Looks like there are erroneously issues with the duplicate classes, but they are JVM specific:

  # Danger! Multiple jars contain identically named classes. This may cause different behaviour depending on classpath ordering.
  # Run ./gradlew checkClassUniqueness --write-locks to update this file
  
  ## runtimeClasspath
  [com.fasterxml.jackson.core:jackson-core, org.apache.parquet:parquet-jackson]
    - META-INF.versions.11.com.fasterxml.jackson.core.io.doubleparser.BigSignificand
    - META-INF.versions.11.com.fasterxml.jackson.core.io.doubleparser.FastDoubleSwar
    - META-INF.versions.11.com.fasterxml.jackson.core.io.doubleparser.FastIntegerMath
    - META-INF.versions.17.com.fasterxml.jackson.core.io.doubleparser.FastDoubleSwar
    - META-INF.versions.17.com.fasterxml.jackson.core.io.doubleparser.FastIntegerMath
    - META-INF.versions.21.com.fasterxml.jackson.core.io.doubleparser.FastDoubleSwar
    - META-INF.versions.21.com.fasterxml.jackson.core.io.doubleparser.FastIntegerMath

Unfortunally, we cannot upgrade gradle-baseline-java because that would require Java11+

@jbonofre
Copy link
Member

@Fokko do you want me to check if I can do a baseline gradle plugin version JDK8 compliant dealing with that duplicated classes ?

@Fokko
Copy link
Contributor Author

Fokko commented Jun 27, 2024

Feel free to dig into it. The current version that we use is from November 2021: https://github.com/palantir/gradle-baseline/releases/tag/4.42.0 :(

@jbonofre
Copy link
Member

@Fokko ok let me take a look. I think it would be great to include Parquet update in Iceberg 1.6.0 release.

@Fokko
Copy link
Contributor Author

Fokko commented Jun 28, 2024

I'm also good to leave this one out for the 1.6.0 release. I think it is going to be quite a bit of work.

@jbonofre
Copy link
Member

Yeah I think it's reasonable. Thanks.

@ajantha-bhat
Copy link
Member

@Fokko do you want me to check if I can do a baseline gradle plugin version JDK8 compliant dealing with that duplicated classes ?

@jbonofre: I don't think we should maintain private forks of these.
Dropping JDK8 seems like a good idea, so we can bump the gradle-baseline-java which handles the multi-version-jar.

@jbonofre
Copy link
Member

@ajantha-bhat my previous comment was not about doing a private plugin fork, it was more about fixing the plugin. Palantir seems to still maintain the baseline plugin, so I can provide a PR.

@Fokko Fokko requested a review from findepi August 2, 2024 08:28
@findepi
Copy link
Member

findepi commented Aug 2, 2024

Change LGTM, but FYI, the build is failing.

@Fokko
Copy link
Contributor Author

Fokko commented Aug 5, 2024

@findepi Looks like some false positive from the shadow plugin that's unable to coop with the Java-version specific files. Have to dig deeper into this.

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.

4 participants