diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index f807189a3df..498055bdfc2 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -121,6 +121,9 @@ Bug Fixes * GITHUB#12220: Hunspell: disallow hidden title-case entries from compound middle/end +* GITHUB#12878: Fix the declared Exceptions of Expression#evaluate() to match those + of DoubleValues#doubleValue(). (Uwe Schindler) + Other --------------------- diff --git a/lucene/MIGRATE.md b/lucene/MIGRATE.md index 6960d435a00..ff80dd5d442 100644 --- a/lucene/MIGRATE.md +++ b/lucene/MIGRATE.md @@ -122,6 +122,13 @@ The new implementation of the Javascript expressions compiler no longer supports Due to the use of `MethodHandle`, classloader isolation is no longer needed, because JS code can only call MHs that were resolved by the application before using the expressions module. +### `Expression#evaluate()` declares to throw IOException (GITHUB#12878) + +The expressions module has changed the `Expression#evaluate()` method signature: +It now declares that it may throw `IOException`. This was an oversight because +compiled expressions call `DoubleValues#doubleValue` behind the scenes, which +may throw `IOException` on index problems, bubbling up unexpectedly to the caller. + ## Migration from Lucene 9.0 to Lucene 9.1 ### Test framework package migration and module (LUCENE-10301) diff --git a/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java b/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java index e58807dcfa4..0c65305dccd 100644 --- a/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java +++ b/lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/ExpressionsBenchmark.java @@ -17,7 +17,6 @@ package org.apache.lucene.benchmark.jmh; import java.io.IOException; -import java.io.UncheckedIOException; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; @@ -83,12 +82,8 @@ public class ExpressionsBenchmark { private static final Expression NATIVE_IDENTITY_EXPRESSION = new Expression(NATIVE_IDENTITY_NAME, new String[] {"x"}) { @Override - public double evaluate(DoubleValues[] functionValues) { - try { - return functionValues[0].doubleValue(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + public double evaluate(DoubleValues[] functionValues) throws IOException { + return functionValues[0].doubleValue(); } }; diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/Expression.java b/lucene/expressions/src/java/org/apache/lucene/expressions/Expression.java index 8efac5191f5..c7710305f33 100644 --- a/lucene/expressions/src/java/org/apache/lucene/expressions/Expression.java +++ b/lucene/expressions/src/java/org/apache/lucene/expressions/Expression.java @@ -16,6 +16,7 @@ */ package org.apache.lucene.expressions; +import java.io.IOException; import org.apache.lucene.expressions.js.JavascriptCompiler; import org.apache.lucene.search.DoubleValues; import org.apache.lucene.search.DoubleValuesSource; @@ -89,7 +90,7 @@ public abstract class Expression { * @param functionValues {@link DoubleValues} for each element of {@link #variables}. * @return The computed value of the expression for the given document. */ - public abstract double evaluate(DoubleValues[] functionValues); + public abstract double evaluate(DoubleValues[] functionValues) throws IOException; /** * Get a DoubleValuesSource which can compute the value of this expression in the context of the diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java b/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java index 8196a2f5c57..bdfc1862591 100644 --- a/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java +++ b/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java @@ -120,6 +120,7 @@ public final class JavascriptCompiler { DOUBLE_VAL_METHOD = getAsmMethod(double.class, "doubleValue"), PATCH_STACK_METHOD = getAsmMethod(Throwable.class, "patchStackTrace", Throwable.class, Expression.class); + private static final Type[] EVALUATE_EXCEPTIONS = new Type[] {Type.getType(IOException.class)}; private static final Handle DYNAMIC_CONSTANT_BOOTSTRAP_HANDLE = new Handle( Opcodes.H_INVOKESTATIC, @@ -211,16 +212,6 @@ public final class JavascriptCompiler { return new JavascriptCompiler(sourceText, functions, picky).compileExpression(); } - /** - * This method is unused, it is just here to make sure that the function signatures don't change. - * If this method fails to compile, you also have to change the byte code generator to correctly - * use the FunctionValues class. - */ - @SuppressWarnings({"unused"}) - private static void unusedTestCompile(DoubleValues f) throws IOException { - f.doubleValue(); - } - /** * Constructs a compiler for expressions with specific set of functions * @@ -355,7 +346,8 @@ public final class JavascriptCompiler { constructor.endMethod(); final GeneratorAdapter gen = - new GeneratorAdapter(Opcodes.ACC_PUBLIC, EVALUATE_METHOD, null, null, classWriter); + new GeneratorAdapter( + Opcodes.ACC_PUBLIC, EVALUATE_METHOD, null, EVALUATE_EXCEPTIONS, classWriter); // add a try/catch block to rewrite stack trace of any Throwable final Label beginTry = gen.newLabel(), endTry = gen.newLabel(), catchHandler = gen.newLabel(); diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestAPISanity.java b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestAPISanity.java new file mode 100644 index 00000000000..faf73c980fc --- /dev/null +++ b/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestAPISanity.java @@ -0,0 +1,46 @@ +/* + * 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.lucene.expressions.js; + +import java.io.IOException; +import org.apache.lucene.expressions.Expression; +import org.apache.lucene.search.DoubleValues; + +/** Some sanity checks with API as those are not detected by {@link JavascriptCompiler} */ +public class TestAPISanity extends CompilerTestCase { + + // if this class does not compile, the Exception types of evaluate and DoubleValues do not match. + @SuppressWarnings("unused") + private static class SanityExpression extends Expression { + + public SanityExpression(String sourceText, String[] variables) { + super(sourceText, variables); + } + + @Override + public double evaluate(DoubleValues[] functionValues) throws IOException { + return functionValues[0].doubleValue(); + } + } + + public void testBytecodeExceptions() throws Exception { + Expression expr = compile("1"); + assertArrayEquals( + Expression.class.getDeclaredMethod("evaluate", DoubleValues[].class).getExceptionTypes(), + expr.getClass().getDeclaredMethod("evaluate", DoubleValues[].class).getExceptionTypes()); + } +}