This is an automated email from the ASF dual-hosted git repository.
mihaibudiu 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 43f94f47ef [CALCITE-6753] DeterministicCodeOptimizer may lift method
calls out of try-catch blocks
43f94f47ef is described below
commit 43f94f47efbdba7ac4176a842868094d3741d93a
Author: Mihai Budiu <[email protected]>
AuthorDate: Tue Aug 4 21:06:27 2026 -0700
[CALCITE-6753] DeterministicCodeOptimizer may lift method calls out of
try-catch blocks
Signed-off-by: Mihai Budiu <[email protected]>
---
.../org/apache/calcite/runtime/SqlFunctions.java | 4 -
.../linq4j/tree/ClassDeclarationFinder.java | 26 +++
.../calcite/linq4j/test/DeterministicTest.java | 183 +++++++++++++++++++++
3 files changed, 209 insertions(+), 4 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index cf80e7fc0e..e3ad7c502d 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -5305,10 +5305,6 @@ public static int toInt(java.sql.Time v) {
return v == null ? castNonNull(null) : toInt(v);
}
- // Method tagged as non-deterministic because it can throw.
- // The DeterministicCodeOptimizer may otherwise try to lift it out of
try-catch blocks.
- // See https://issues.apache.org/jira/browse/CALCITE-6753
- @NonDeterministic
public static int toInt(String s) {
return parseInt(s.trim());
}
diff --git
a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ClassDeclarationFinder.java
b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ClassDeclarationFinder.java
index bb0d93b013..b952534772 100644
---
a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ClassDeclarationFinder.java
+++
b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/ClassDeclarationFinder.java
@@ -34,6 +34,10 @@
public class ClassDeclarationFinder extends Shuttle {
protected final @Nullable ClassDeclarationFinder parent;
+ /** Visits a subtree without changing it. Used for the subtrees
+ * which must not be optimized. */
+ private static final Shuttle PASS_THROUGH = new Shuttle();
+
/**
* The list of new final static fields to be added to the current class.
*/
@@ -153,6 +157,28 @@ protected ClassDeclarationFinder(ClassDeclarationFinder
parent) {
return visitor;
}
+ /**
+ * Skips optimization of the entire {@code try} statement.
+ *
+ * <p>An expression must not be factored out of a {@code try} statement:
+ * the initializer of the resulting static field runs during class
+ * initialization, outside the reach of the {@code catch} and
+ * {@code finally} handlers. For example, factoring a method call out of
+ * {@code try { return f(x); } catch (Exception e) { return null; }}
+ * (the shape generated for a safe cast) would make the exception escape
+ * as an {@code ExceptionInInitializerError} instead of yielding
+ * {@code null}. See
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6753";>[CALCITE-6753]
+ * DeterministicCodeOptimizer may lift method calls out of try-catch
+ * blocks</a>.
+ *
+ * @param tryStatement statement to leave unchanged
+ * @return pass-through visitor
+ */
+ @Override public Shuttle preVisit(TryStatement tryStatement) {
+ return PASS_THROUGH;
+ }
+
@Override public Expression visit(NewExpression newExpression,
List<Expression> arguments, @Nullable List<MemberDeclaration>
memberDeclarations) {
if (parent == null) {
diff --git
a/linq4j/src/test/java/org/apache/calcite/linq4j/test/DeterministicTest.java
b/linq4j/src/test/java/org/apache/calcite/linq4j/test/DeterministicTest.java
index 1ec9de8a60..55b50d61fd 100644
--- a/linq4j/src/test/java/org/apache/calcite/linq4j/test/DeterministicTest.java
+++ b/linq4j/src/test/java/org/apache/calcite/linq4j/test/DeterministicTest.java
@@ -511,6 +511,189 @@ private boolean isConstant(Expression e) {
+ "}\n"));
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6753";>[CALCITE-6753]
+ * DeterministicCodeOptimizer may lift method calls out of try-catch
+ * blocks</a>. A deterministic method call must stay inside the try
+ * statement; a static field initializer would run outside the reach of
+ * the catch handler. */
+ @Test void testMethodCallWithinTryCatchNotFactored() {
+ assertThat(
+ optimize(
+ Expressions.new_(
+ Runnable.class,
+ Collections.emptyList(),
+ Expressions.methodDecl(
+ 0,
+ int.class,
+ "test",
+ Collections.emptyList(),
+ Expressions.block(
+ Expressions.tryCatch(
+ Expressions.return_(null,
+ Expressions.call(
+ getMethod(Integer.class, "valueOf",
+ int.class),
+ Expressions.constant(0))),
+ Expressions.catch_(
+ Expressions.parameter(Exception.class, "e"),
+ Expressions.return_(null,
+ Expressions.constant(-1)))))))),
+ equalTo("{\n"
+ + " return new Runnable(){\n"
+ + " int test() {\n"
+ + " try {\n"
+ + " return Integer.valueOf(0);\n"
+ + " } catch (Exception e) {\n"
+ + " return -1;\n"
+ + " }\n"
+ + " }\n"
+ + "\n"
+ + " };\n"
+ + "}\n"));
+ }
+
+ /** Expressions in a catch block are not factored out either, and factoring
+ * resumes for statements that follow the try statement. */
+ @Test void testFactoringResumesAfterTryCatch() {
+ assertThat(
+ optimize(
+ Expressions.new_(
+ Runnable.class,
+ Collections.emptyList(),
+ Expressions.methodDecl(
+ 0,
+ int.class,
+ "test",
+ Collections.emptyList(),
+ Expressions.block(
+ Expressions.tryCatch(
+ Expressions.statement(
+ Expressions.call(
+ getMethod(Integer.class, "valueOf",
+ int.class),
+ Expressions.constant(0))),
+ Expressions.catch_(
+ Expressions.parameter(Exception.class, "e"),
+ Expressions.statement(
+ Expressions.call(
+ getMethod(Integer.class, "valueOf",
+ int.class),
+ Expressions.constant(1))))),
+ Expressions.return_(null,
+ Expressions.add(ONE, TWO)))))),
+ equalTo("{\n"
+ + " return new Runnable(){\n"
+ + " int test() {\n"
+ + " try {\n"
+ + " Integer.valueOf(0);\n"
+ + " } catch (Exception e) {\n"
+ + " Integer.valueOf(1);\n"
+ + " }\n"
+ + " return $L4J$C$1_2;\n"
+ + " }\n"
+ + "\n"
+ + " static final int $L4J$C$1_2 = 1 + 2;\n"
+ + " };\n"
+ + "}\n"));
+ }
+
+ /** A try statement with only a finally block is not optimized either;
+ * moving an expression to a static field initializer would bypass the
+ * finally handler. */
+ @Test void testExpressionWithinTryFinallyNotFactored() {
+ assertThat(
+ optimize(
+ Expressions.new_(
+ Runnable.class,
+ Collections.emptyList(),
+ Expressions.methodDecl(
+ 0,
+ int.class,
+ "test",
+ Collections.emptyList(),
+ Expressions.block(
+ Expressions.tryFinally(
+ Expressions.return_(null,
+ Expressions.call(
+ getMethod(Integer.class, "valueOf",
+ int.class),
+ Expressions.constant(0))),
+ Expressions.statement(
+ Expressions.add(ONE, TWO))))))),
+ equalTo("{\n"
+ + " return new Runnable(){\n"
+ + " int test() {\n"
+ + " try {\n"
+ + " return Integer.valueOf(0);\n"
+ + " } finally {\n"
+ + " 1 + 2;\n"
+ + " }\n"
+ + " }\n"
+ + "\n"
+ + " };\n"
+ + "}\n"));
+ }
+
+ /** The optimizer must not add static fields to a class declared within a
+ * try statement.
+ *
+ * <p>Factoring {@code 1 + 2} out to a static field of the {@code Callable}
+ * would move the evaluation into that field's initializer. The initializer
+ * still runs inside the try, when {@code new} first instantiates the
+ * class, but the JVM wraps anything a static initializer throws in an
+ * {@code ExceptionInInitializerError} (an {@code Error}, per JLS 12.4.2),
+ * which {@code catch (Exception e)} does not match. */
+ @Test void testNestedClassWithinTryCatchNotFactored() {
+ assertThat(
+ optimize(
+ Expressions.new_(
+ Runnable.class,
+ Collections.emptyList(),
+ Expressions.methodDecl(
+ 0,
+ int.class,
+ "test",
+ Collections.emptyList(),
+ Expressions.block(
+ Expressions.tryCatch(
+ Expressions.return_(null,
+ Expressions.call(
+ Expressions.new_(
+ Callable.class,
+ Collections.emptyList(),
+ Expressions.methodDecl(
+ 0,
+ Object.class,
+ "call",
+ Collections.emptyList(),
+ Blocks.toFunctionBlock(
+ Expressions.add(ONE, TWO)))),
+ "call",
+ Collections.emptyList())),
+ Expressions.catch_(
+ Expressions.parameter(Exception.class, "e"),
+ Expressions.return_(null,
+ Expressions.constant(-1)))))))),
+ equalTo("{\n"
+ + " return new Runnable(){\n"
+ + " int test() {\n"
+ + " try {\n"
+ + " return new java.util.concurrent.Callable(){\n"
+ + " Object call() {\n"
+ + " return 1 + 2;\n"
+ + " }\n"
+ + "\n"
+ + " }.call();\n"
+ + " } catch (Exception e) {\n"
+ + " return -1;\n"
+ + " }\n"
+ + " }\n"
+ + "\n"
+ + " };\n"
+ + "}\n"));
+ }
+
@Test void testDeterministicClassNonDeterministicMethod() {
assertThat(
optimize(
