This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new ad2f496e23 [CALCITE-7617] Improve type safety of RelJson API using
generics
ad2f496e23 is described below
commit ad2f496e23d9e0a022e5b7d25373c71af4e26393
Author: Stamatis Zampetakis <[email protected]>
AuthorDate: Mon Jun 22 17:59:47 2026 +0200
[CALCITE-7617] Improve type safety of RelJson API using generics
---
.../apache/calcite/rel/externalize/RelJson.java | 20 +++++++-------
.../calcite/rel/externalize/RelJsonReader.java | 4 +--
.../apache/calcite/InvalidStaticInitializer.java | 32 ++++++++++++++++++++++
.../apache/calcite/plan/RelOptPlanReaderTest.java | 26 ++++++++++++++----
4 files changed, 64 insertions(+), 18 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
index 2409ac6090..51c178db06 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
@@ -120,7 +120,7 @@ public class RelJson {
ImmutableList.of(NlsString.class, BigDecimal.class, ByteString.class,
Boolean.class, TimestampString.class, DateString.class,
TimeString.class);
- private final Map<String, Constructor> constructorMap = new HashMap<>();
+ private final Map<String, Constructor<? extends RelNode>> constructorMap =
new HashMap<>();
private final @Nullable JsonBuilder jsonBuilder;
private final InputTranslator inputTranslator;
private final SqlOperatorTable operatorTable;
@@ -209,9 +209,9 @@ private static <T extends Enum<T>> T enumVal(Class<T>
clazz, Map<String, Object>
public RelNode create(Map<String, Object> map) {
String type = get(map, "type");
- Constructor constructor = getConstructor(type);
+ Constructor<? extends RelNode> constructor = getConstructor(type);
try {
- return (RelNode) constructor.newInstance(map);
+ return constructor.newInstance(map);
} catch (InstantiationException | ClassCastException |
InvocationTargetException
| IllegalAccessException e) {
throw new RuntimeException(
@@ -219,12 +219,11 @@ public RelNode create(Map<String, Object> map) {
}
}
- public Constructor getConstructor(String type) {
- Constructor constructor = constructorMap.get(type);
+ public Constructor<? extends RelNode> getConstructor(String type) {
+ Constructor<? extends RelNode> constructor = constructorMap.get(type);
if (constructor == null) {
- Class clazz = typeNameToClass(type);
+ Class<? extends RelNode> clazz = typeNameToClass(type);
try {
- //noinspection unchecked
constructor = clazz.getConstructor(RelInput.class);
} catch (NoSuchMethodException e) {
throw new RuntimeException("class does not have required constructor, "
@@ -239,18 +238,19 @@ public Constructor getConstructor(String type) {
* Converts a type name to a class. E.g. {@code getClass("LogicalProject")}
* returns {@link org.apache.calcite.rel.logical.LogicalProject}.class.
*/
- public Class typeNameToClass(String type) {
+ public Class<? extends RelNode> typeNameToClass(String type) {
if (!type.contains(".")) {
for (String package_ : PACKAGES) {
try {
- return Class.forName(package_ + type);
+ return Class.forName(package_ + type, false,
RelJson.class.getClassLoader())
+ .asSubclass(RelNode.class);
} catch (ClassNotFoundException e) {
// ignore
}
}
}
try {
- return Class.forName(type);
+ return Class.forName(type, false,
RelJson.class.getClassLoader()).asSubclass(RelNode.class);
} catch (ClassNotFoundException e) {
throw new RuntimeException("unknown type " + type);
}
diff --git
a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java
index 61a86d9b5f..2cdd785fe8 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java
@@ -131,7 +131,7 @@ private void readRels(List<Map<String, Object>> jsonRels) {
private void readRel(final Map<String, Object> jsonRel) {
String id = (String) requireNonNull(jsonRel.get("id"), "jsonRel.id");
String type = (String) requireNonNull(jsonRel.get("relOp"),
"jsonRel.relOp");
- Constructor constructor = relJson.getConstructor(type);
+ Constructor<? extends RelNode> constructor = relJson.getConstructor(type);
RelInput input = new RelInput() {
@Override public RelOptCluster getCluster() {
return cluster;
@@ -309,7 +309,7 @@ public ImmutableList<RexLiteral> getTuple(List jsonTuple) {
}
};
try {
- final RelNode rel = (RelNode) constructor.newInstance(input);
+ final RelNode rel = constructor.newInstance(input);
relMap.put(id, rel);
lastRel = rel;
} catch (InstantiationException | IllegalAccessException e) {
diff --git
a/core/src/test/java/org/apache/calcite/InvalidStaticInitializer.java
b/core/src/test/java/org/apache/calcite/InvalidStaticInitializer.java
new file mode 100644
index 0000000000..448c766a88
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/InvalidStaticInitializer.java
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite;
+
+/**
+ * An invalid class that always fail if initialized. The class may be
sub-classed to cover
+ * test cases where a static initializer is not allowed to be triggered. All
the classes in the
+ * hierarchy are called by reflection thus appear as unused.
+ */
+@SuppressWarnings("unused")
+public class InvalidStaticInitializer {
+ static {
+ throwError();
+ }
+ private static void throwError() {
+ throw new AssertionError("Static initializer must not be triggered");
+ }
+}
diff --git
a/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java
b/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java
index c274e3d177..cb38e64106 100644
--- a/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java
+++ b/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java
@@ -27,6 +27,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;
/**
@@ -40,22 +41,22 @@ class RelOptPlanReaderTest {
assertThat(relJson.classToTypeName(LogicalProject.class),
is("LogicalProject"));
assertThat(relJson.typeNameToClass("LogicalProject"),
- sameInstance((Class) LogicalProject.class));
+ sameInstance(LogicalProject.class));
// in org.apache.calcite.adapter.jdbc.JdbcRules outer class
assertThat(relJson.classToTypeName(JdbcRules.JdbcProject.class),
is("JdbcProject"));
assertThat(relJson.typeNameToClass("JdbcProject"),
- equalTo((Class) JdbcRules.JdbcProject.class));
+ equalTo(JdbcRules.JdbcProject.class));
try {
- Class clazz = relJson.typeNameToClass("NonExistentRel");
+ Class<?> clazz = relJson.typeNameToClass("NonExistentRel");
fail("expected exception, got " + clazz);
} catch (RuntimeException e) {
assertThat(e.getMessage(), is("unknown type NonExistentRel"));
}
try {
- Class clazz =
+ Class<?> clazz =
relJson.typeNameToClass("org.apache.calcite.rel.NonExistentRel");
fail("expected exception, got " + clazz);
} catch (RuntimeException e) {
@@ -67,11 +68,11 @@ class RelOptPlanReaderTest {
assertThat(relJson.classToTypeName(MyRel.class),
is("org.apache.calcite.plan.RelOptPlanReaderTest$MyRel"));
assertThat(relJson.typeNameToClass(MyRel.class.getName()),
- equalTo((Class) MyRel.class));
+ equalTo(MyRel.class));
// Using canonical name (with '$'), not found
try {
- Class clazz =
+ Class<?> clazz =
relJson.typeNameToClass(MyRel.class.getCanonicalName());
fail("expected exception, got " + clazz);
} catch (RuntimeException e) {
@@ -81,6 +82,19 @@ class RelOptPlanReaderTest {
}
}
+ /**
+ * Tests loading of a class not implementing the {@code RelNode} interface
+ * throws an informative {@code ClassCastException}. Additionally, the test
+ * ensures that the type conversion does not trigger class initialization.
+ */
+ @Test void testTypeNameToClassWithNoRelNodeClass() {
+ RelJson relJson = RelJson.create();
+ ClassCastException e =
+ assertThrows(ClassCastException.class,
+ () ->
relJson.typeNameToClass("org.apache.calcite.InvalidStaticInitializer"));
+ assertThat(e.getMessage(), is("class
org.apache.calcite.InvalidStaticInitializer"));
+ }
+
/** Dummy relational expression. */
static class MyRel extends AbstractRelNode {
MyRel(RelOptCluster cluster, RelTraitSet traitSet) {
