Skip to content

Commit

Permalink
Revise fix for [CALCITE-6587] Support Java 23 and Guava 33.3.0
Browse files Browse the repository at this point in the history
Add '-Djava.security.manager=allow' in Gradle and re-enable
tests under JDK 23. (Tests will fail if people run them via
Junit.)
  • Loading branch information
julianhyde committed Oct 5, 2024
1 parent d59871d commit 78e873d
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 37 deletions.
11 changes: 11 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,17 @@ allprojects {
showStandardStreams = true
}
exclude("**/*Suite*")
if (JavaVersion.current() >= JavaVersion.VERSION_23) {
// Subject.doAs is deprecated and does not work in JDK 23
// and higher unless the (also deprecated) SecurityManager
// is enabled. However, we depend on libraries Avatica and
// Hadoop for our remote driver and Pig and Spark
// adapters. So as a workaround we require enabling the
// security manager on JDK 23 and higher. See
// [CALCITE-6587], [CALCITE-6590] (Avatica), [HADOOP-19212],
// https://openjdk.org/jeps/411.
jvmArgs("-Djava.security.manager=allow")
}
jvmArgs("-Xmx1536m")
jvmArgs("-Djdk.net.URLClassPath.disableClassPathURLCheck=true")
// Pass the property to tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.apache.calcite.test.schemata.hr.Employee;
import org.apache.calcite.util.TestUtil;
import org.apache.calcite.util.Util;
import org.apache.calcite.util.Version;

import com.google.common.collect.ImmutableList;

Expand Down Expand Up @@ -85,7 +84,6 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assumptions.assumeFalse;

import static java.util.Objects.requireNonNull;

Expand All @@ -97,6 +95,11 @@
* <a href="https://issues.apache.org/jira/browse/CALCITE-2853">
* [CALCITE-2853] avatica.MetaImpl and calcite.jdbc.CalciteMetaImpl are not
* thread-safe</a>.
*
* <p>Under JDK 23 and higher, this test requires
* "{@code -Djava.security.manager=allow}" command-line arguments due to
* Avatica's use of deprecated methods in {@link javax.security.auth.Subject}.
* These arguments are set automatically if you run via Gradle.
*/
@Execution(ExecutionMode.SAME_THREAD)
class CalciteRemoteDriverTest {
Expand All @@ -110,11 +113,6 @@ class CalciteRemoteDriverTest {
private static @Nullable HttpServer start;

@BeforeAll public static void beforeClass() throws Exception {
assumeFalse(TestUtil.getJavaMajorVersion() >= 23
&& TestUtil.AVATICA_VERSION.compareTo(Version.of(1, 25)) > 0,
"Cannot run on JDK 23 and higher with Avatica version 1.25 or lower; "
+ "enable when [CALCITE-6588] is fixed in Avatica");

localConnection = CalciteAssert.hr().connect();

// Make sure we pick an ephemeral port for the server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.apache.calcite.piglet.PigConverter;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.tools.FrameworkConfig;
import org.apache.calcite.util.TestUtil;

import org.junit.jupiter.api.BeforeEach;

Expand All @@ -32,6 +31,11 @@

/**
* Abstract class for Pig to {@link RelNode} tests.
*
* <p>Under JDK 23 and higher, this test requires
* "{@code -Djava.security.manager=allow}" command-line arguments due to
* Hadoop's use of deprecated methods in {@link javax.security.auth.Subject}.
* These arguments are set automatically if you run via Gradle.
*/
public abstract class PigRelTestBase {
PigConverter converter;
Expand All @@ -40,8 +44,6 @@ public abstract class PigRelTestBase {
public void testSetup() throws Exception {
assumeFalse(getProperty("os.name").startsWith("Windows"),
"Skip: Pig/Hadoop tests do not work on Windows");
assumeFalse(TestUtil.getJavaMajorVersion() >= 23,
"Skip: Pig/Hadoop tests do not work on JDK 23 and higher");

final FrameworkConfig config = config().build();
converter = create(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,25 @@
*/
package org.apache.calcite.chinook;

import org.apache.calcite.util.TestUtil;
import org.apache.calcite.util.Version;

import org.junit.jupiter.api.Test;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;

import static org.junit.jupiter.api.Assumptions.assumeFalse;

/**
* Tests against parameters in prepared statement when using underlying JDBC
* sub-schema.
*
* <p>Under JDK 23 and higher, this test requires
* "{@code -Djava.security.manager=allow}" command-line arguments due to
* Avatica's use of deprecated methods in {@link javax.security.auth.Subject}.
* These arguments are set automatically if you run via Gradle.
*/
class RemotePreparedStatementParametersTest {

@Test void testSimpleStringParameterShouldWorkWithCalcite() throws Exception {
assumeFalse(TestUtil.getJavaMajorVersion() >= 23
&& TestUtil.AVATICA_VERSION.compareTo(Version.of(1, 25)) > 0,
"Cannot run on JDK 23 and higher with Avatica version 1.25 or lower; "
+ "enable when [CALCITE-6588] is fixed in Avatica");

// given
ChinookAvaticaServer server = new ChinookAvaticaServer();
server.startWithCalcite();
Expand All @@ -54,11 +49,6 @@ class RemotePreparedStatementParametersTest {
}

@Test void testSeveralParametersShouldWorkWithCalcite() throws Exception {
assumeFalse(TestUtil.getJavaMajorVersion() >= 23
&& TestUtil.AVATICA_VERSION.compareTo(Version.of(1, 25)) > 0,
"Cannot run on JDK 23 and higher with Avatica version 1.25 or lower; "
+ "enable when [CALCITE-6588] is fixed in Avatica");

// given
ChinookAvaticaServer server = new ChinookAvaticaServer();
server.startWithCalcite();
Expand All @@ -75,11 +65,6 @@ class RemotePreparedStatementParametersTest {
}

@Test void testParametersShouldWorkWithRaw() throws Exception {
assumeFalse(TestUtil.getJavaMajorVersion() >= 23
&& TestUtil.AVATICA_VERSION.compareTo(Version.of(1, 25)) > 0,
"Cannot run on JDK 23 and higher with Avatica version 1.25 or lower; "
+ "enable when [CALCITE-6588] is fixed in Avatica");

// given
ChinookAvaticaServer server = new ChinookAvaticaServer();
server.startWithRaw();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,19 @@
package org.apache.calcite.test;

import org.apache.calcite.adapter.spark.SparkRel;
import org.apache.calcite.util.TestUtil;
import org.apache.calcite.util.Util;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assumptions.assumeFalse;

/**
* Tests for using Calcite with Spark as an internal engine, as implemented by
* the {@link org.apache.calcite.adapter.spark} package.
*
* <p>Under JDK 23 and higher, this test requires
* "{@code -Djava.security.manager=allow}" command-line arguments due to
* Hadoop's use of deprecated methods in {@link javax.security.auth.Subject}.
* These arguments are set automatically if you run via Gradle.
*/
class SparkAdapterTest {
private static final String VALUES0 = "(values (1, 'a'), (2, 'b'))";
Expand All @@ -45,10 +47,6 @@ class SparkAdapterTest {
"(values (1, 'a'), (2, 'b'), (3, 'b'), (4, 'c'), (2, 'c')) as t(x, y)";

private CalciteAssert.AssertQuery sql(String sql) {
assumeFalse(TestUtil.getJavaMajorVersion() >= 23,
"Cannot run on JDK 23 and higher; "
+ "enable when [HADOOP-19212] has been fixed");

return CalciteAssert.that()
.with(CalciteAssert.Config.SPARK)
.query(sql);
Expand Down

0 comments on commit 78e873d

Please sign in to comment.