Fix the declared Exceptions of Expression#evaluate() to match those of DoubleValues#doubleValue() (#12878)

This commit is contained in:
Uwe Schindler 2023-12-06 11:56:37 +01:00 committed by GitHub
parent b22a9b8998
commit af2aea5eb2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 63 additions and 19 deletions

View File

@ -121,6 +121,9 @@ Bug Fixes
* GITHUB#12220: Hunspell: disallow hidden title-case entries from compound middle/end * 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 Other
--------------------- ---------------------

View File

@ -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 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. 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 ## Migration from Lucene 9.0 to Lucene 9.1
### Test framework package migration and module (LUCENE-10301) ### Test framework package migration and module (LUCENE-10301)

View File

@ -17,7 +17,6 @@
package org.apache.lucene.benchmark.jmh; package org.apache.lucene.benchmark.jmh;
import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType; import java.lang.invoke.MethodType;
@ -83,12 +82,8 @@ public class ExpressionsBenchmark {
private static final Expression NATIVE_IDENTITY_EXPRESSION = private static final Expression NATIVE_IDENTITY_EXPRESSION =
new Expression(NATIVE_IDENTITY_NAME, new String[] {"x"}) { new Expression(NATIVE_IDENTITY_NAME, new String[] {"x"}) {
@Override @Override
public double evaluate(DoubleValues[] functionValues) { public double evaluate(DoubleValues[] functionValues) throws IOException {
try { return functionValues[0].doubleValue();
return functionValues[0].doubleValue();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
} }
}; };

View File

@ -16,6 +16,7 @@
*/ */
package org.apache.lucene.expressions; package org.apache.lucene.expressions;
import java.io.IOException;
import org.apache.lucene.expressions.js.JavascriptCompiler; import org.apache.lucene.expressions.js.JavascriptCompiler;
import org.apache.lucene.search.DoubleValues; import org.apache.lucene.search.DoubleValues;
import org.apache.lucene.search.DoubleValuesSource; import org.apache.lucene.search.DoubleValuesSource;
@ -89,7 +90,7 @@ public abstract class Expression {
* @param functionValues {@link DoubleValues} for each element of {@link #variables}. * @param functionValues {@link DoubleValues} for each element of {@link #variables}.
* @return The computed value of the expression for the given document. * @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 * Get a DoubleValuesSource which can compute the value of this expression in the context of the

View File

@ -120,6 +120,7 @@ public final class JavascriptCompiler {
DOUBLE_VAL_METHOD = getAsmMethod(double.class, "doubleValue"), DOUBLE_VAL_METHOD = getAsmMethod(double.class, "doubleValue"),
PATCH_STACK_METHOD = PATCH_STACK_METHOD =
getAsmMethod(Throwable.class, "patchStackTrace", Throwable.class, Expression.class); 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 = private static final Handle DYNAMIC_CONSTANT_BOOTSTRAP_HANDLE =
new Handle( new Handle(
Opcodes.H_INVOKESTATIC, Opcodes.H_INVOKESTATIC,
@ -211,16 +212,6 @@ public final class JavascriptCompiler {
return new JavascriptCompiler(sourceText, functions, picky).compileExpression(); 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 * Constructs a compiler for expressions with specific set of functions
* *
@ -355,7 +346,8 @@ public final class JavascriptCompiler {
constructor.endMethod(); constructor.endMethod();
final GeneratorAdapter gen = 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 // add a try/catch block to rewrite stack trace of any Throwable
final Label beginTry = gen.newLabel(), endTry = gen.newLabel(), catchHandler = gen.newLabel(); final Label beginTry = gen.newLabel(), endTry = gen.newLabel(), catchHandler = gen.newLabel();

View File

@ -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());
}
}