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

[CALCITE-6387] Make Arrow adapter passing tests with jdk17+ #3776

Merged
merged 1 commit into from Apr 28, 2024

Conversation

snuyanzin
Copy link
Contributor

@snuyanzin snuyanzin commented Apr 27, 2024

First commit removes add-opens flag to reproduce the issue on gha

Second commit to fix the problem both for gha and local run

@snuyanzin snuyanzin changed the title [CALCITE-6387] Remove --add-opens=java.base/java.nio=ALL-UNNAMED flag [CALCITE-6387] Make Arrow adapter passing tests with jdk11+ Apr 27, 2024
@snuyanzin snuyanzin changed the title [CALCITE-6387] Make Arrow adapter passing tests with jdk11+ [CALCITE-6387] Make Arrow adapter passing tests with jdk17+ Apr 27, 2024
@snuyanzin
Copy link
Contributor Author

@macroguo-ghy, @asolimando could you please have a look since you were involved in the original Arrow adapter implementation/review

Copy link
Member

@asolimando asolimando left a comment

Choose a reason for hiding this comment

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

Thanks @snuyanzin, tested locally with 17.0.9-zulu and I don't have any error now

@macroguo-ghy
Copy link
Contributor

Thanks @snuyanzin .
A minor comment: better remove --add-opens in

calcite/Jenkinsfile

Lines 44 to 46 in d4e8830

// The following option `--add-opens=java.base/java.nio=ALL-UNNAMED` is required jdk17+
// to avoid error. See https://arrow.apache.org/docs/java/install.html#java-compatibility
withEnv(["Path+JDK=$JAVA_JDK_17/bin","JAVA_HOME=$JAVA_JDK_17","_JAVA_OPTIONS=--add-opens=java.base/java.nio=ALL-UNNAMED"]) {

@snuyanzin
Copy link
Contributor Author

Thanks for the hint @macroguo-ghy, changed it for jenkins file as well

@snuyanzin
Copy link
Contributor Author

Thanks for taking a look,
I'm going to merge it

@snuyanzin
Copy link
Contributor Author

squash commits before merging

Copy link

sonarcloud bot commented Apr 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@snuyanzin snuyanzin merged commit 3e633f6 into apache:main Apr 28, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants