From cf07b27c8c24bea02a4e9eed70c5dd2083ce43d5 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 6 Dec 2019 14:46:02 -0700 Subject: [PATCH 01/44] Notes --- .../main/java/org/elasticsearch/painless/Compiler.java | 1 + .../org/elasticsearch/painless/PainlessScriptEngine.java | 6 ++++++ .../main/java/org/elasticsearch/painless/ScriptRoot.java | 3 +++ .../java/org/elasticsearch/painless/node/ENewObj.java | 8 ++++++++ .../java/org/elasticsearch/painless/node/PCallInvoke.java | 3 +++ 5 files changed, 21 insertions(+) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index f6de8be896cb3..a1d05750dc705 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -211,6 +211,7 @@ Constructor compile(Loader loader, Set extractedVariables, String nam ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass); SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null); root.extractVariables(extractedVariables); + // TODO(stu): ScriptRoot created here, needs to be passed up to root.analyze(painlessLookup, settings); Map statics = root.write(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 91448a4a3788e..49b7ac88d5690 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -143,11 +143,14 @@ public Loader run() { }); Set extractedVariables = new HashSet<>(); + // TODO(stu): get isDeterministic out of scriptroot from the compile compile(contextsToCompilers.get(context), loader, extractedVariables, scriptName, scriptSource, params); if (context.statefulFactoryClazz != null) { + // TODO(stu): isDeterministic needs to go here return generateFactory(loader, context, extractedVariables, generateStatefulFactory(loader, context, extractedVariables)); } else { + // TODO(stu): or here return generateFactory(loader, context, extractedVariables, WriterConstants.CLASS_TYPE); } } @@ -279,6 +282,7 @@ private T generateFactory( Set extractedVariables, Type classType ) { + // TODO(stu): use ASM to override the isDeterministic method and create it with correct value int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS; int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER| Opcodes.ACC_FINAL; String interfaceBase = Type.getType(context.factoryClazz).getInternalName(); @@ -374,6 +378,8 @@ void compile(Compiler compiler, Loader loader, Set extractedVariables, @Override public Void run() { String name = scriptName == null ? source : scriptName; + // TODO(stu): isDeterministic needs to come here, once compiled is part of class loader + // so we can find it in class loader for the factory compiler.compile(loader, extractedVariables, name, source, compilerSettings); return null; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java index 72d5e50a8a6ae..ca377d8a673d2 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java @@ -38,6 +38,7 @@ public class ScriptRoot { protected final FunctionTable functionTable = new FunctionTable(); protected int syntheticCounter = 0; + protected boolean deterministic = true; public ScriptRoot(PainlessLookup painlessLookup, CompilerSettings compilerSettings, ScriptClassInfo scriptClassInfo, SClass classRoot) { this.painlessLookup = Objects.requireNonNull(painlessLookup); @@ -72,4 +73,6 @@ public FunctionTable getFunctionTable() { public String getNextSyntheticName(String prefix) { return prefix + "$synthetic$" + syntheticCounter++; } + + public void setDeterministic(boolean deterministic) { this.deterministic &= deterministic; } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index 5f17cd9696473..ff22eefa7cbd7 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -75,6 +75,9 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "constructor [" + typeToCanonicalTypeName(actual) + ", /" + arguments.size() + "] not found")); } + // TODO(stu): set deterministic in scriptRoot based on constructor + // scriptRoot.setDeterministic(constructor.annotations.contains(DeterministicAnnotation.class); + Class[] types = new Class[constructor.typeParameters.size()]; constructor.typeParameters.toArray(types); @@ -118,4 +121,9 @@ void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) public String toString() { return singleLineToStringWithOptionalArgs(arguments, type); } + + private boolean isDeterministic() { + // TODO(stu): check for annotation + return false; + } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java index 47bd54c288640..ce8d3ac950aa2 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java @@ -79,6 +79,9 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "method [" + typeToCanonicalTypeName(prefix.actual) + ", " + name + "/" + arguments.size() + "] not found")); } + // TODO(stu): set deterministic in scriptRoot based on method + // scriptRoot.setDeterministic(method.annotations.contains(DeterministicAnnotation.class); + sub = new PSubCallInvoke(location, method, prefix.actual, arguments); } From 593da606b68bb4be4bc487ebbaf3469a33b80c6c Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 6 Dec 2019 15:30:48 -0700 Subject: [PATCH 02/44] Wiring from whitelist annotations to PainlessMethod and PainlessConstructor --- .../annotation/DeterministicAnnotation.java | 29 ++++++++++++++ .../DeterministicAnnotationParser.java | 40 +++++++++++++++++++ .../annotation/WhitelistAnnotationParser.java | 3 +- .../painless/lookup/PainlessConstructor.java | 11 +++-- .../lookup/PainlessLookupBuilder.java | 31 ++++++++------ .../painless/lookup/PainlessMethod.java | 10 +++-- 6 files changed, 105 insertions(+), 19 deletions(-) create mode 100644 modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java create mode 100644 modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java new file mode 100644 index 0000000000000..51797a3ba80c7 --- /dev/null +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java @@ -0,0 +1,29 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.painless.spi.annotation; + +public class DeterministicAnnotation { + + public static final String NAME = "deterministic"; + + public static final DeterministicAnnotation INSTANCE = new DeterministicAnnotation(); + + private DeterministicAnnotation() {} +} diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java new file mode 100644 index 0000000000000..7cfa9f0c16820 --- /dev/null +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java @@ -0,0 +1,40 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.painless.spi.annotation; + +import java.util.Map; + +public class DeterministicAnnotationParser implements WhitelistAnnotationParser { + + public static final DeterministicAnnotationParser INSTANCE = new DeterministicAnnotationParser(); + + private DeterministicAnnotationParser() {} + + @Override + public Object parse(Map arguments) { + if (arguments.isEmpty() == false) { + throw new IllegalArgumentException( + "unexpected parameters for [@" + DeterministicAnnotation.NAME + "] annotation, found " + arguments + ); + } + + return DeterministicAnnotation.INSTANCE; + } +} diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java index d5dcbab36f34f..18787d3d618e2 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java @@ -34,7 +34,8 @@ public interface WhitelistAnnotationParser { Map BASE_ANNOTATION_PARSERS = Collections.unmodifiableMap( Stream.of( new AbstractMap.SimpleEntry<>(NoImportAnnotation.NAME, NoImportAnnotationParser.INSTANCE), - new AbstractMap.SimpleEntry<>(DeprecatedAnnotation.NAME, DeprecatedAnnotationParser.INSTANCE) + new AbstractMap.SimpleEntry<>(DeprecatedAnnotation.NAME, DeprecatedAnnotationParser.INSTANCE), + new AbstractMap.SimpleEntry<>(DeterministicAnnotation.NAME, DeterministicAnnotationParser.INSTANCE) ).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)) ); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java index 0f890e88b736b..b9b7a65925c24 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessConstructor.java @@ -23,6 +23,7 @@ import java.lang.invoke.MethodType; import java.lang.reflect.Constructor; import java.util.List; +import java.util.Map; import java.util.Objects; public class PainlessConstructor { @@ -31,12 +32,15 @@ public class PainlessConstructor { public final List> typeParameters; public final MethodHandle methodHandle; public final MethodType methodType; + public final Map, Object> annotations; - PainlessConstructor(Constructor javaConstructor, List> typeParameters, MethodHandle methodHandle, MethodType methodType) { + PainlessConstructor(Constructor javaConstructor, List> typeParameters, MethodHandle methodHandle, MethodType methodType, + Map, Object> annotations) { this.javaConstructor = javaConstructor; this.typeParameters = typeParameters; this.methodHandle = methodHandle; this.methodType = methodType; + this.annotations = annotations; } @Override @@ -53,11 +57,12 @@ public boolean equals(Object object) { return Objects.equals(javaConstructor, that.javaConstructor) && Objects.equals(typeParameters, that.typeParameters) && - Objects.equals(methodType, that.methodType); + Objects.equals(methodType, that.methodType) && + Objects.equals(annotations, that.annotations); } @Override public int hashCode() { - return Objects.hash(javaConstructor, typeParameters, methodType); + return Objects.hash(javaConstructor, typeParameters, methodType, annotations); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index cad8bcc9a1c34..e9813c37ed69a 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -50,6 +50,7 @@ import java.security.SecureClassLoader; import java.security.cert.Certificate; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -132,7 +133,8 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { for (WhitelistConstructor whitelistConstructor : whitelistClass.whitelistConstructors) { origin = whitelistConstructor.origin; painlessLookupBuilder.addPainlessConstructor( - targetCanonicalClassName, whitelistConstructor.canonicalTypeNameParameters); + targetCanonicalClassName, whitelistConstructor.canonicalTypeNameParameters, + whitelistConstructor.painlessAnnotations); } for (WhitelistMethod whitelistMethod : whitelistClass.whitelistMethods) { @@ -140,7 +142,7 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { painlessLookupBuilder.addPainlessMethod( whitelist.classLoader, targetCanonicalClassName, whitelistMethod.augmentedCanonicalClassName, whitelistMethod.methodName, whitelistMethod.returnCanonicalTypeName, - whitelistMethod.canonicalTypeNameParameters); + whitelistMethod.canonicalTypeNameParameters, whitelistMethod.painlessAnnotations); } for (WhitelistField whitelistField : whitelistClass.whitelistFields) { @@ -150,6 +152,7 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { } } + // TODO(stu): handle annotations here, need to wire up ClassBinding and ECallLocal for (WhitelistMethod whitelistStatic : whitelist.whitelistImportedMethods) { origin = whitelistStatic.origin; painlessLookupBuilder.addImportedPainlessMethod( @@ -313,7 +316,8 @@ public void addPainlessClass(Class clazz, boolean importClassName) { } } - public void addPainlessConstructor(String targetCanonicalClassName, List canonicalTypeNameParameters) { + public void addPainlessConstructor(String targetCanonicalClassName, List canonicalTypeNameParameters, + Map, Object> annotations) { Objects.requireNonNull(targetCanonicalClassName); Objects.requireNonNull(canonicalTypeNameParameters); @@ -337,10 +341,10 @@ public void addPainlessConstructor(String targetCanonicalClassName, List typeParameters.add(typeParameter); } - addPainlessConstructor(targetClass, typeParameters); + addPainlessConstructor(targetClass, typeParameters, annotations); } - public void addPainlessConstructor(Class targetClass, List> typeParameters) { + public void addPainlessConstructor(Class targetClass, List> typeParameters, Map, Object> annotations) { Objects.requireNonNull(targetClass); Objects.requireNonNull(typeParameters); @@ -390,7 +394,8 @@ public void addPainlessConstructor(Class targetClass, List> typePara String painlessConstructorKey = buildPainlessConstructorKey(typeParametersSize); PainlessConstructor existingPainlessConstructor = painlessClassBuilder.constructors.get(painlessConstructorKey); - PainlessConstructor newPainlessConstructor = new PainlessConstructor(javaConstructor, typeParameters, methodHandle, methodType); + PainlessConstructor newPainlessConstructor = new PainlessConstructor(javaConstructor, typeParameters, methodHandle, methodType, + annotations); if (existingPainlessConstructor == null) { newPainlessConstructor = painlessConstructorCache.computeIfAbsent(newPainlessConstructor, key -> key); @@ -403,7 +408,8 @@ public void addPainlessConstructor(Class targetClass, List> typePara } public void addPainlessMethod(ClassLoader classLoader, String targetCanonicalClassName, String augmentedCanonicalClassName, - String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { + String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters, + Map, Object> annotations) { Objects.requireNonNull(classLoader); Objects.requireNonNull(targetCanonicalClassName); @@ -449,11 +455,11 @@ public void addPainlessMethod(ClassLoader classLoader, String targetCanonicalCla "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } - addPainlessMethod(targetClass, augmentedClass, methodName, returnType, typeParameters); + addPainlessMethod(targetClass, augmentedClass, methodName, returnType, typeParameters, annotations); } public void addPainlessMethod(Class targetClass, Class augmentedClass, - String methodName, Class returnType, List> typeParameters) { + String methodName, Class returnType, List> typeParameters, Map, Object> annotations) { Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); @@ -562,7 +568,7 @@ public void addPainlessMethod(Class targetClass, Class augmentedClass, painlessClassBuilder.staticMethods.get(painlessMethodKey) : painlessClassBuilder.methods.get(painlessMethodKey); PainlessMethod newPainlessMethod = - new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType); + new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType, annotations); if (existingPainlessMethod == null) { newPainlessMethod = painlessMethodCache.computeIfAbsent(newPainlessMethod, key -> key); @@ -840,8 +846,9 @@ public void addImportedPainlessMethod(Class targetClass, String methodName, C MethodType methodType = methodHandle.type(); PainlessMethod existingImportedPainlessMethod = painlessMethodKeysToImportedPainlessMethods.get(painlessMethodKey); + // TODO(stu): update with annotations PainlessMethod newImportedPainlessMethod = - new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType); + new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType, Collections.emptyMap()); if (existingImportedPainlessMethod == null) { newImportedPainlessMethod = painlessMethodCache.computeIfAbsent(newImportedPainlessMethod, key -> key); @@ -1444,7 +1451,7 @@ public BridgeLoader run() { painlessMethod.javaMethod.getName(), bridgeTypeParameters.toArray(new Class[0])); MethodHandle bridgeHandle = MethodHandles.publicLookup().in(bridgeClass).unreflect(bridgeClass.getMethods()[0]); bridgePainlessMethod = new PainlessMethod(bridgeMethod, bridgeClass, - painlessMethod.returnType, bridgeTypeParameters, bridgeHandle, bridgeMethodType); + painlessMethod.returnType, bridgeTypeParameters, bridgeHandle, bridgeMethodType, Collections.emptyMap()); painlessClassBuilder.runtimeMethods.put(painlessMethodKey.intern(), bridgePainlessMethod); painlessBridgeCache.put(painlessMethod, bridgePainlessMethod); } catch (Exception exception) { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java index 358449c36f3e5..6e84f5b28e77e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java @@ -23,6 +23,7 @@ import java.lang.invoke.MethodType; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; import java.util.Objects; public class PainlessMethod { @@ -33,9 +34,10 @@ public class PainlessMethod { public final List> typeParameters; public final MethodHandle methodHandle; public final MethodType methodType; + public final Map, Object> annotations; public PainlessMethod(Method javaMethod, Class targetClass, Class returnType, List> typeParameters, - MethodHandle methodHandle, MethodType methodType) { + MethodHandle methodHandle, MethodType methodType, Map, Object> annotations) { this.javaMethod = javaMethod; this.targetClass = targetClass; @@ -43,6 +45,7 @@ public PainlessMethod(Method javaMethod, Class targetClass, Class returnTy this.typeParameters = List.copyOf(typeParameters); this.methodHandle = methodHandle; this.methodType = methodType; + this.annotations = annotations; } @Override @@ -61,11 +64,12 @@ public boolean equals(Object object) { Objects.equals(targetClass, that.targetClass) && Objects.equals(returnType, that.returnType) && Objects.equals(typeParameters, that.typeParameters) && - Objects.equals(methodType, that.methodType); + Objects.equals(methodType, that.methodType) && + Objects.equals(annotations, that.annotations); } @Override public int hashCode() { - return Objects.hash(javaMethod, targetClass, returnType, typeParameters, methodType); + return Objects.hash(javaMethod, targetClass, returnType, typeParameters, methodType, annotations); } } From d1ae37fca32892a00c8346cb372552ab01417e90 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 6 Dec 2019 16:25:32 -0700 Subject: [PATCH 03/44] Generate isResultDeterministic --- .../org/elasticsearch/painless/Compiler.java | 10 ++--- .../painless/PainlessScriptEngine.java | 38 +++++++++++-------- .../painless/node/ECapturingFunctionRef.java | 2 +- .../elasticsearch/painless/node/ECast.java | 2 +- .../elasticsearch/painless/node/ENewObj.java | 4 +- .../painless/node/PCallInvoke.java | 4 +- .../elasticsearch/painless/node/SClass.java | 3 +- 7 files changed, 35 insertions(+), 28 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index a1d05750dc705..35fb0dc5eb944 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -26,7 +26,6 @@ import org.elasticsearch.painless.spi.Whitelist; import org.objectweb.asm.util.Printer; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; @@ -205,14 +204,15 @@ private static void addFactoryMethod(Map> additionalClasses, Cl * @param name The name of the script. * @param source The source code for the script. * @param settings The CompilerSettings to be used during the compilation. - * @return An executable script that implements both a specified interface and is a subclass of {@link PainlessScript} + * @return The ScriptRoot used to compile */ - Constructor compile(Loader loader, Set extractedVariables, String name, String source, CompilerSettings settings) { + ScriptRoot compile(Loader loader, Set extractedVariables, String name, String source, + CompilerSettings settings) { ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass); SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null); root.extractVariables(extractedVariables); // TODO(stu): ScriptRoot created here, needs to be passed up to - root.analyze(painlessLookup, settings); + ScriptRoot scriptRoot = root.analyze(painlessLookup, settings); Map statics = root.write(); try { @@ -226,7 +226,7 @@ Constructor compile(Loader loader, Set extractedVariables, String nam clazz.getField(statik.getKey()).set(null, statik.getValue()); } - return clazz.getConstructors()[0]; + return scriptRoot; } catch (Exception exception) { // Catch everything to let the user know this is something caused internally. throw new IllegalStateException("An internal error occurred attempting to define the script [" + name + "].", exception); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 49b7ac88d5690..00bc93a51867b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -143,15 +143,13 @@ public Loader run() { }); Set extractedVariables = new HashSet<>(); - // TODO(stu): get isDeterministic out of scriptroot from the compile - compile(contextsToCompilers.get(context), loader, extractedVariables, scriptName, scriptSource, params); + ScriptRoot scriptRoot = compile(contextsToCompilers.get(context), loader, extractedVariables, scriptName, scriptSource, params); if (context.statefulFactoryClazz != null) { - // TODO(stu): isDeterministic needs to go here - return generateFactory(loader, context, extractedVariables, generateStatefulFactory(loader, context, extractedVariables)); + return generateFactory(loader, context, extractedVariables, generateStatefulFactory(loader, context, extractedVariables), + scriptRoot); } else { - // TODO(stu): or here - return generateFactory(loader, context, extractedVariables, WriterConstants.CLASS_TYPE); + return generateFactory(loader, context, extractedVariables, WriterConstants.CLASS_TYPE, scriptRoot); } } @@ -273,6 +271,7 @@ private Type generateStatefulFactory( * @param context The {@link ScriptContext}'s semantics are used to define the factory class. * @param classType The type to be instaniated in the newFactory or newInstance method. Depends * on whether a {@link ScriptContext#statefulFactoryClazz} is specified. + * @param scriptRoot the {@link ScriptRoot} used to do the compilation * @param The factory class. * @return A factory class that will return script instances. */ @@ -280,7 +279,8 @@ private T generateFactory( Loader loader, ScriptContext context, Set extractedVariables, - Type classType + Type classType, + ScriptRoot scriptRoot ) { // TODO(stu): use ASM to override the isDeterministic method and create it with correct value int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS; @@ -334,8 +334,18 @@ private T generateFactory( adapter.endMethod(); writeNeedsMethods(context.factoryClazz, writer, extractedVariables); - writer.visitEnd(); + String methodName = "isResultDeterministic"; + org.objectweb.asm.commons.Method isResultDeterministic = new org.objectweb.asm.commons.Method(methodName, + MethodType.methodType(boolean.class).toMethodDescriptorString()); + GeneratorAdapter deterAdapter = new GeneratorAdapter(Opcodes.ASM5, instance, + writer.visitMethod(Opcodes.ACC_PUBLIC, methodName, isResultDeterministic.getDescriptor(), null, null)); + deterAdapter.visitCode(); + deterAdapter.push(scriptRoot.deterministic); + deterAdapter.returnValue(); + deterAdapter.endMethod(); + + writer.visitEnd(); Class factory = loader.defineFactory(className.replace('/', '.'), writer.toByteArray()); try { @@ -368,21 +378,17 @@ private void writeNeedsMethods(Class clazz, ClassWriter writer, Set e } } - void compile(Compiler compiler, Loader loader, Set extractedVariables, + ScriptRoot compile(Compiler compiler, Loader loader, Set extractedVariables, String scriptName, String source, Map params) { final CompilerSettings compilerSettings = buildCompilerSettings(params); try { // Drop all permissions to actually compile the code itself. - AccessController.doPrivileged(new PrivilegedAction() { + return AccessController.doPrivileged(new PrivilegedAction() { @Override - public Void run() { + public ScriptRoot run() { String name = scriptName == null ? source : scriptName; - // TODO(stu): isDeterministic needs to come here, once compiled is part of class loader - // so we can find it in class loader for the factory - compiler.compile(loader, extractedVariables, name, source, compilerSettings); - - return null; + return compiler.compile(loader, extractedVariables, name, source, compilerSettings); } }, COMPILATION_CONTEXT); // Note that it is safe to catch any of the following errors since Painless is stateless. diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java index 00f2283472e49..dd76ddc6d0974 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECapturingFunctionRef.java @@ -37,7 +37,7 @@ import java.util.Set; /** - * Represents a capturing function reference. + * Represents a capturing function reference. For member functions that require a this reference, ie not static. */ public final class ECapturingFunctionRef extends AExpression implements ILambda { private final String variable; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java index d33f37fb6049b..0e50368bf7c3b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECast.java @@ -32,7 +32,7 @@ import java.util.Set; /** - * Represents a cast that is inserted into the tree replacing other casts. (Internal only.) + * Represents a cast that is inserted into the tree replacing other casts. (Internal only.) Casts are inserted during semantic checking. */ final class ECast extends AExpression { diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index ff22eefa7cbd7..79425ee0404cb 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -27,6 +27,7 @@ import org.elasticsearch.painless.ScriptRoot; import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookupUtility; +import org.elasticsearch.painless.spi.annotation.DeterministicAnnotation; import org.objectweb.asm.Type; import org.objectweb.asm.commons.Method; @@ -75,8 +76,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "constructor [" + typeToCanonicalTypeName(actual) + ", /" + arguments.size() + "] not found")); } - // TODO(stu): set deterministic in scriptRoot based on constructor - // scriptRoot.setDeterministic(constructor.annotations.contains(DeterministicAnnotation.class); + scriptRoot.setDeterministic(constructor.annotations.containsKey(DeterministicAnnotation.class)); Class[] types = new Class[constructor.typeParameters.size()]; constructor.typeParameters.toArray(types); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java index ce8d3ac950aa2..98e7753910e23 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java @@ -27,6 +27,7 @@ import org.elasticsearch.painless.ScriptRoot; import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.lookup.def; +import org.elasticsearch.painless.spi.annotation.DeterministicAnnotation; import java.util.List; import java.util.Objects; @@ -79,8 +80,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "method [" + typeToCanonicalTypeName(prefix.actual) + ", " + name + "/" + arguments.size() + "] not found")); } - // TODO(stu): set deterministic in scriptRoot based on method - // scriptRoot.setDeterministic(method.annotations.contains(DeterministicAnnotation.class); + scriptRoot.setDeterministic(method.annotations.containsKey(DeterministicAnnotation.class)); sub = new PSubCallInvoke(location, method, prefix.actual, arguments); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java index 356f56d7cf491..7145a4a9e33ad 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SClass.java @@ -130,7 +130,7 @@ public void extractVariables(Set variables) { extractedVariables.addAll(variables); } - public void analyze(PainlessLookup painlessLookup, CompilerSettings settings) { + public ScriptRoot analyze(PainlessLookup painlessLookup, CompilerSettings settings) { this.settings = settings; table = new ScriptRoot(painlessLookup, settings, scriptClassInfo, this); @@ -148,6 +148,7 @@ public void analyze(PainlessLookup painlessLookup, CompilerSettings settings) { Locals locals = Locals.newProgramScope(); analyze(table, locals); + return table; } @Override From 45e3aed9f63126a61904c004d9a37b76b6609d90 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 6 Dec 2019 16:59:02 -0700 Subject: [PATCH 04/44] formatting, parseInt is deterministic --- .../org/elasticsearch/painless/PainlessScriptEngine.java | 5 +++-- .../resources/org/elasticsearch/painless/spi/java.lang.txt | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 00bc93a51867b..83f6886660bb0 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -337,8 +337,9 @@ private T generateFactory( String methodName = "isResultDeterministic"; org.objectweb.asm.commons.Method isResultDeterministic = new org.objectweb.asm.commons.Method(methodName, - MethodType.methodType(boolean.class).toMethodDescriptorString()); - GeneratorAdapter deterAdapter = new GeneratorAdapter(Opcodes.ASM5, instance, + MethodType.methodType(boolean.class).toMethodDescriptorString()); + + GeneratorAdapter deterAdapter = new GeneratorAdapter(Opcodes.ASM5, isResultDeterministic, writer.visitMethod(Opcodes.ACC_PUBLIC, methodName, isResultDeterministic.getDescriptor(), null, null)); deterAdapter.visitCode(); deterAdapter.push(scriptRoot.deterministic); diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt index 63ed6d41c676d..d0d307dafa21f 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt @@ -547,7 +547,7 @@ class java.lang.Integer { int min(int,int) int numberOfLeadingZeros(int) int numberOfTrailingZeros(int) - int parseInt(String) + int parseInt(String) @deterministic int parseInt(String,int) int parseUnsignedInt(String) int parseUnsignedInt(String,int) From db77cde4637b8519c21237ff6761c850b8a6ab31 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 10 Dec 2019 13:49:34 -0700 Subject: [PATCH 05/44] WIP --- .../common/xcontent/XContentParser.java | 3 +- .../common/xcontent/XContentSubParser.java | 3 +- .../support/AbstractXContentParser.java | 6 +-- ...n.java => NonDeterministicAnnotation.java} | 8 ++-- ... => NonDeterministicAnnotationParser.java} | 10 ++--- .../annotation/WhitelistAnnotationParser.java | 2 +- .../org/elasticsearch/painless/Compiler.java | 1 - .../painless/PainlessScriptEngine.java | 1 - .../elasticsearch/painless/node/ENewObj.java | 9 +--- .../painless/node/PCallInvoke.java | 4 +- .../elasticsearch/painless/spi/java.lang.txt | 7 +-- .../elasticsearch/painless/spi/java.util.txt | 31 +++++++------ .../elasticsearch/painless/FactoryTests.java | 26 +++++++++++ .../index/query/QueryShardContext.java | 2 + .../metrics/AvgAggregatorTests.java | 8 ++-- .../metrics/MaxAggregatorTests.java | 2 +- .../script/MockScriptEngine.java | 43 +++++++++---------- .../test/test/ESTestCaseTests.java | 4 +- .../xcontent/WatcherXContentParser.java | 3 +- 19 files changed, 101 insertions(+), 72 deletions(-) rename modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/{DeterministicAnnotation.java => NonDeterministicAnnotation.java} (77%) rename modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/{DeterministicAnnotationParser.java => NonDeterministicAnnotationParser.java} (72%) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index 82a663bd9dc5d..4bc9cbb682fab 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -24,6 +24,7 @@ import java.io.Closeable; import java.io.IOException; import java.nio.CharBuffer; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -132,7 +133,7 @@ enum NumberType { Map map() throws IOException; - Map mapOrdered() throws IOException; + LinkedHashMap mapOrdered() throws IOException; Map mapStrings() throws IOException; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java index 9a8686001e2dc..39657ca168922 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.nio.CharBuffer; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -99,7 +100,7 @@ public Map map() throws IOException { } @Override - public Map mapOrdered() throws IOException { + public LinkedHashMap mapOrdered() throws IOException { return parser.mapOrdered(); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index 264af205e488b..22d695acccf06 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -267,7 +267,7 @@ public Map map() throws IOException { } @Override - public Map mapOrdered() throws IOException { + public LinkedHashMap mapOrdered() throws IOException { return readOrderedMap(this); } @@ -302,8 +302,8 @@ static Map readMap(XContentParser parser) throws IOException { return readMap(parser, SIMPLE_MAP_FACTORY); } - static Map readOrderedMap(XContentParser parser) throws IOException { - return readMap(parser, ORDERED_MAP_FACTORY); + static LinkedHashMap readOrderedMap(XContentParser parser) throws IOException { + return (LinkedHashMap)readMap(parser, ORDERED_MAP_FACTORY); } static Map readMapStrings(XContentParser parser) throws IOException { diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotation.java similarity index 77% rename from modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java rename to modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotation.java index 51797a3ba80c7..9724e42431419 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotation.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotation.java @@ -19,11 +19,11 @@ package org.elasticsearch.painless.spi.annotation; -public class DeterministicAnnotation { +public class NonDeterministicAnnotation { - public static final String NAME = "deterministic"; + public static final String NAME = "nondeterministic"; - public static final DeterministicAnnotation INSTANCE = new DeterministicAnnotation(); + public static final NonDeterministicAnnotation INSTANCE = new NonDeterministicAnnotation(); - private DeterministicAnnotation() {} + private NonDeterministicAnnotation() {} } diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotationParser.java similarity index 72% rename from modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java rename to modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotationParser.java index 7cfa9f0c16820..4277cf3b1d699 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/DeterministicAnnotationParser.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/NonDeterministicAnnotationParser.java @@ -21,20 +21,20 @@ import java.util.Map; -public class DeterministicAnnotationParser implements WhitelistAnnotationParser { +public class NonDeterministicAnnotationParser implements WhitelistAnnotationParser { - public static final DeterministicAnnotationParser INSTANCE = new DeterministicAnnotationParser(); + public static final NonDeterministicAnnotationParser INSTANCE = new NonDeterministicAnnotationParser(); - private DeterministicAnnotationParser() {} + private NonDeterministicAnnotationParser() {} @Override public Object parse(Map arguments) { if (arguments.isEmpty() == false) { throw new IllegalArgumentException( - "unexpected parameters for [@" + DeterministicAnnotation.NAME + "] annotation, found " + arguments + "unexpected parameters for [@" + NonDeterministicAnnotation.NAME + "] annotation, found " + arguments ); } - return DeterministicAnnotation.INSTANCE; + return NonDeterministicAnnotation.INSTANCE; } } diff --git a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java index 18787d3d618e2..ecf1c45c7602f 100644 --- a/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java +++ b/modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/annotation/WhitelistAnnotationParser.java @@ -35,7 +35,7 @@ public interface WhitelistAnnotationParser { Stream.of( new AbstractMap.SimpleEntry<>(NoImportAnnotation.NAME, NoImportAnnotationParser.INSTANCE), new AbstractMap.SimpleEntry<>(DeprecatedAnnotation.NAME, DeprecatedAnnotationParser.INSTANCE), - new AbstractMap.SimpleEntry<>(DeterministicAnnotation.NAME, DeterministicAnnotationParser.INSTANCE) + new AbstractMap.SimpleEntry<>(NonDeterministicAnnotation.NAME, NonDeterministicAnnotationParser.INSTANCE) ).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)) ); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index 35fb0dc5eb944..174936ec6ff40 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -211,7 +211,6 @@ ScriptRoot compile(Loader loader, Set extractedVariables, String name, S ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass); SClass root = Walker.buildPainlessTree(scriptClassInfo, name, source, settings, painlessLookup, null); root.extractVariables(extractedVariables); - // TODO(stu): ScriptRoot created here, needs to be passed up to ScriptRoot scriptRoot = root.analyze(painlessLookup, settings); Map statics = root.write(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 83f6886660bb0..45db4428aeced 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -282,7 +282,6 @@ private T generateFactory( Type classType, ScriptRoot scriptRoot ) { - // TODO(stu): use ASM to override the isDeterministic method and create it with correct value int classFrames = ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS; int classAccess = Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER| Opcodes.ACC_FINAL; String interfaceBase = Type.getType(context.factoryClazz).getInternalName(); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index 79425ee0404cb..d75814a0205c0 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -27,7 +27,7 @@ import org.elasticsearch.painless.ScriptRoot; import org.elasticsearch.painless.lookup.PainlessConstructor; import org.elasticsearch.painless.lookup.PainlessLookupUtility; -import org.elasticsearch.painless.spi.annotation.DeterministicAnnotation; +import org.elasticsearch.painless.spi.annotation.NonDeterministicAnnotation; import org.objectweb.asm.Type; import org.objectweb.asm.commons.Method; @@ -76,7 +76,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "constructor [" + typeToCanonicalTypeName(actual) + ", /" + arguments.size() + "] not found")); } - scriptRoot.setDeterministic(constructor.annotations.containsKey(DeterministicAnnotation.class)); + scriptRoot.setDeterministic(constructor.annotations.containsKey(NonDeterministicAnnotation.class) == false); Class[] types = new Class[constructor.typeParameters.size()]; constructor.typeParameters.toArray(types); @@ -121,9 +121,4 @@ void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) public String toString() { return singleLineToStringWithOptionalArgs(arguments, type); } - - private boolean isDeterministic() { - // TODO(stu): check for annotation - return false; - } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java index 98e7753910e23..44031f2431c3c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java @@ -27,7 +27,7 @@ import org.elasticsearch.painless.ScriptRoot; import org.elasticsearch.painless.lookup.PainlessMethod; import org.elasticsearch.painless.lookup.def; -import org.elasticsearch.painless.spi.annotation.DeterministicAnnotation; +import org.elasticsearch.painless.spi.annotation.NonDeterministicAnnotation; import java.util.List; import java.util.Objects; @@ -80,7 +80,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "method [" + typeToCanonicalTypeName(prefix.actual) + ", " + name + "/" + arguments.size() + "] not found")); } - scriptRoot.setDeterministic(method.annotations.containsKey(DeterministicAnnotation.class)); + scriptRoot.setDeterministic(method.annotations.containsKey(NonDeterministicAnnotation.class) == false); sub = new PSubCallInvoke(location, method, prefix.actual, arguments); } diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt index d0d307dafa21f..6987458293993 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt @@ -48,6 +48,7 @@ class java.lang.Comparable { int compareTo(def) } +# TODO(stu): evaluate in the case of hashset/hashmap class java.lang.Iterable { void forEach(Consumer) Iterator iterator() @@ -547,7 +548,7 @@ class java.lang.Integer { int min(int,int) int numberOfLeadingZeros(int) int numberOfTrailingZeros(int) - int parseInt(String) @deterministic + int parseInt(String) int parseInt(String,int) int parseUnsignedInt(String) int parseUnsignedInt(String,int) @@ -636,7 +637,7 @@ class java.lang.Math { double nextDown(double) double nextUp(double) double pow(double,double) - double random() + double random() @nondeterministic double rint(double) long round(double) double scalb(double,int) @@ -729,7 +730,7 @@ class java.lang.StrictMath { double nextDown(double) double nextUp(double) double pow(double,double) - double random() + double random() @nondeterministic double rint(double) long round(double) double scalb(double,int) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt index e7cc5bb7db463..50c566cedef5f 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt @@ -145,13 +145,13 @@ class java.util.Map { def computeIfPresent(def,BiFunction) boolean containsKey(def) boolean containsValue(def) - Set entrySet() + Set entrySet() @nondeterministic boolean equals(Object) - void forEach(BiConsumer) + void forEach(BiConsumer) @nondeterministic def get(def) def getOrDefault(def,def) boolean isEmpty() - Set keySet() + Set keySet() @nondeterministic def merge(def,def,BiFunction) def put(def,def) void putAll(Map) @@ -162,7 +162,7 @@ class java.util.Map { boolean replace(def,def,def) void replaceAll(BiFunction) int size() - Collection values() + Collection values() @nondeterministic Object org.elasticsearch.painless.api.Augmentation getByPath(String) Object org.elasticsearch.painless.api.Augmentation getByPath(String, Object) @@ -678,26 +678,26 @@ class java.util.GregorianCalendar { } class java.util.HashMap { - () - (Map) + () @nondeterministic + (Map) @nondeterministic def clone() } class java.util.HashSet { - () - (Collection) + () @nondeterministic + (Collection) @nondeterministic def clone() } class java.util.Hashtable { - () - (Map) + () @nondeterministic + (Map) @nondeterministic def clone() } class java.util.IdentityHashMap { - () - (Map) + () @nondeterministic + (Map) @nondeterministic def clone() } @@ -1016,6 +1016,11 @@ class java.util.TreeMap { () (Comparator) def clone() + # TODO(stu): add methods from map declared non-determ in map + Set entrySet() + void forEach(BiConsumer) + Set keySet() + Collection values() } class java.util.TreeSet { @@ -1031,7 +1036,7 @@ class java.util.UUID { UUID fromString(String) long getLeastSignificantBits() long getMostSignificantBits() - UUID randomUUID() + UUID randomUUID() @nondeterministic UUID nameUUIDFromBytes(byte[]) long node() long timestamp() diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java index 1d3fd829d2512..33645e24c0b3c 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java @@ -162,6 +162,32 @@ public void testFactory() { assertEquals(false, factory.needsNothing()); } + public void testDeterministic() { + FactoryTestScript.Factory factory = + scriptEngine.compile("deterministic_test", "Integer.parseInt('123')", + FactoryTestScript.CONTEXT, Collections.emptyMap()); + assertTrue(factory.isResultDeterministic()); + assertEquals(123, factory.newInstance(Collections.emptyMap()).execute(0)); + } + + public void testNotDeterministic() { + FactoryTestScript.Factory factory = + scriptEngine.compile("not_deterministic_test", "Math.random()", + FactoryTestScript.CONTEXT, Collections.emptyMap()); + assertFalse(factory.isResultDeterministic()); + Double d = (Double)factory.newInstance(Collections.emptyMap()).execute(0); + assertTrue(d >= 0.0 && d <= 1.0); + } + + public void testMixedDeterministicIsNotDeterministic() { + FactoryTestScript.Factory factory = + scriptEngine.compile("not_deterministic_test", "Integer.parseInt('123') + Math.random()", + FactoryTestScript.CONTEXT, Collections.emptyMap()); + assertFalse(factory.isResultDeterministic()); + Double d = (Double)factory.newInstance(Collections.emptyMap()).execute(0); + assertTrue(d >= 123.0 && d <= 124.0); + } + public abstract static class EmptyTestScript { public static final String[] PARAMETERS = {}; public abstract Object execute(); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 88a8351790c0a..56161d8f2fa34 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -325,6 +325,8 @@ public Index index() { /** Compile script using script service */ public FactoryType compile(Script script, ScriptContext context) { FactoryType factory = scriptService.compile(script, context); + // TODO(stu): can check `factory instanceof ScriptFactory && ((ScriptFactory) factory).isResultDeterministic() == false` + // to avoid being so intrusive if (factory.isResultDeterministic() == false) { failIfFrozen(); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java index 4dfdfd2fd91ca..4afc8d91771fd 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java @@ -638,9 +638,9 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script does not get cached. + * Make sure that an aggregation using a script deterministic script from MockScriptEngine (Aggs is deterministic there) do get cached. */ - public void testDontCacheScripts() throws IOException { + public void testCacheScripts() throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; @@ -675,8 +675,8 @@ public void testDontCacheScripts() throws IOException { assertEquals("avg", avg.getName()); assertTrue(AggregationInspectionHelper.hasValue(avg)); - // Test that an aggregation using a script does not get cached - assertFalse(aggregator.context().getQueryShardContext().isCacheable()); + // Test that an aggregation using a script does get cached + assertTrue(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); directory.close(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index 739419a534240..cccbcb8084122 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -987,7 +987,7 @@ public void testDontCacheScripts() throws IOException { assertTrue(AggregationInspectionHelper.hasValue(max)); // Test that an aggregation using a script does not get cached - assertFalse(aggregator.context().getQueryShardContext().isCacheable()); + assertTrue(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); directory.close(); diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 84aad77377a30..731ef6298ce55 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -102,17 +102,6 @@ public Object execute() { } }; return context.factoryClazz.cast(factory); - } else if (context.instanceClazz.equals(FieldScript.class)) { - FieldScript.Factory factory = (parameters, lookup) -> - ctx -> new FieldScript(parameters, lookup, ctx) { - @Override - public Object execute() { - Map vars = createVars(parameters); - vars.putAll(getLeafLookup().asMap()); - return script.apply(vars); - } - }; - return context.factoryClazz.cast(factory); } else if(context.instanceClazz.equals(TermsSetQueryScript.class)) { TermsSetQueryScript.Factory factory = (parameters, lookup) -> (TermsSetQueryScript.LeafFactory) ctx -> new TermsSetQueryScript(parameters, lookup, ctx) { @@ -167,24 +156,34 @@ public void execute(Map ctx) { }; return context.factoryClazz.cast(factory); } else if(context.instanceClazz.equals(AggregationScript.class)) { - AggregationScript.Factory factory = (parameters, lookup) -> new AggregationScript.LeafFactory() { + AggregationScript.Factory factory = new AggregationScript.Factory() { @Override - public AggregationScript newInstance(final LeafReaderContext ctx) { - return new AggregationScript(parameters, lookup, ctx) { + public AggregationScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return new AggregationScript.LeafFactory() { @Override - public Object execute() { - Map vars = new HashMap<>(parameters); - vars.put("params", parameters); - vars.put("doc", getDoc()); - vars.put("_score", get_score()); - vars.put("_value", get_value()); - return script.apply(vars); + public AggregationScript newInstance(final LeafReaderContext ctx) { + return new AggregationScript(params, lookup, ctx) { + @Override + public Object execute() { + Map vars = new HashMap<>(params); + vars.put("params", params); + vars.put("doc", getDoc()); + vars.put("_score", get_score()); + vars.put("_value", get_value()); + return script.apply(vars); + } + }; + } + + @Override + public boolean needs_score() { + return true; } }; } @Override - public boolean needs_score() { + public boolean isResultDeterministic() { return true; } }; diff --git a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java index af041a70d977c..e667d441900ba 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java @@ -78,7 +78,7 @@ public void testShuffleMap() throws IOException { XContentType xContentType = randomFrom(XContentType.values()); BytesReference source = RandomObjects.randomSource(random(), xContentType, 5); try (XContentParser parser = createParser(xContentType.xContent(), source)) { - LinkedHashMap initialMap = (LinkedHashMap)parser.mapOrdered(); + LinkedHashMap initialMap = parser.mapOrdered(); Set> distinctKeys = new HashSet<>(); for (int i = 0; i < 10; i++) { @@ -123,7 +123,7 @@ public void testShuffleXContentExcludeFields() throws IOException { BytesReference bytes = BytesReference.bytes(builder); final LinkedHashMap initialMap; try (XContentParser parser = createParser(xContentType.xContent(), bytes)) { - initialMap = (LinkedHashMap)parser.mapOrdered(); + initialMap = parser.mapOrdered(); } List expectedInnerKeys1 = Arrays.asList("inner1", "inner2", "inner3"); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java index 20b0086c1e4e2..05e8e76e8e9cf 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java @@ -20,6 +20,7 @@ import java.nio.CharBuffer; import java.time.Clock; import java.time.ZonedDateTime; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -111,7 +112,7 @@ public Map map() throws IOException { } @Override - public Map mapOrdered() throws IOException { + public LinkedHashMap mapOrdered() throws IOException { return parser.mapOrdered(); } From 16a2f4471ade10023ddbb83b89b18e2b2f768538 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 11 Dec 2019 13:19:25 -0700 Subject: [PATCH 06/44] update whitelist based on core-infra discussion, map/set iterators are ok --- .../elasticsearch/painless/spi/java.util.txt | 62 +++++++++---------- .../painless/spi/org.elasticsearch.score.txt | 2 + 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt index 50c566cedef5f..5559390c1c2c9 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt @@ -145,13 +145,13 @@ class java.util.Map { def computeIfPresent(def,BiFunction) boolean containsKey(def) boolean containsValue(def) - Set entrySet() @nondeterministic + Set entrySet() boolean equals(Object) - void forEach(BiConsumer) @nondeterministic + void forEach(BiConsumer) def get(def) def getOrDefault(def,def) boolean isEmpty() - Set keySet() @nondeterministic + Set keySet() def merge(def,def,BiFunction) def put(def,def) void putAll(Map) @@ -162,7 +162,7 @@ class java.util.Map { boolean replace(def,def,def) void replaceAll(BiFunction) int size() - Collection values() @nondeterministic + Collection values() Object org.elasticsearch.painless.api.Augmentation getByPath(String) Object org.elasticsearch.painless.api.Augmentation getByPath(String, Object) @@ -574,8 +574,8 @@ class java.util.Collections { Comparator reverseOrder() Comparator reverseOrder(Comparator) void rotate(List,int) - void shuffle(List) - void shuffle(List,Random) + void shuffle(List) @nondeterministic + void shuffle(List,Random) @nondeterministic Set singleton(def) List singletonList(def) Map singletonMap(def,def) @@ -605,7 +605,7 @@ class java.util.Currency { } class java.util.Date { - () + () @nondeterministic (long) boolean after(Date) boolean before(Date) @@ -678,26 +678,26 @@ class java.util.GregorianCalendar { } class java.util.HashMap { - () @nondeterministic - (Map) @nondeterministic + () + (Map) def clone() } class java.util.HashSet { - () @nondeterministic - (Collection) @nondeterministic + () + (Collection) def clone() } class java.util.Hashtable { - () @nondeterministic - (Map) @nondeterministic + () + (Map) def clone() } class java.util.IdentityHashMap { - () @nondeterministic - (Map) @nondeterministic + () + (Map) def clone() } @@ -910,22 +910,22 @@ class java.util.PriorityQueue { } class java.util.Random { - () - (long) - DoubleStream doubles(long) - DoubleStream doubles(long,double,double) - IntStream ints(long) - IntStream ints(long,int,int) - LongStream longs(long) - LongStream longs(long,long,long) - boolean nextBoolean() - void nextBytes(byte[]) - double nextDouble() - float nextFloat() - double nextGaussian() - int nextInt() - int nextInt(int) - long nextLong() + () @nondeterministic + (long) @nondeterministic + DoubleStream doubles(long) @nondeterministic + DoubleStream doubles(long,double,double) @nondeterministic + IntStream ints(long) @nondeterministic + IntStream ints(long,int,int) @nondeterministic + LongStream longs(long) @nondeterministic + LongStream longs(long,long,long) @nondeterministic + boolean nextBoolean() @nondeterministic + void nextBytes(byte[]) @nondeterministic + double nextDouble() @nondeterministic + float nextFloat() @nondeterministic + double nextGaussian() @nondeterministic + int nextInt() @nondeterministic + int nextInt(int) @nondeterministic + long nextLong() @nondeterministic void setSeed(long) } diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt index fd49cbb2d5222..71007a3312a5d 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt @@ -25,7 +25,9 @@ class org.elasticsearch.script.ScoreScript @no_import { static_import { double saturation(double, double) from_class org.elasticsearch.script.ScoreScriptUtils double sigmoid(double, double, double) from_class org.elasticsearch.script.ScoreScriptUtils + #TODO(stu): @nondeterministic double randomScore(org.elasticsearch.script.ScoreScript, int, String) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreField + #TODO(stu): @nondeterministic double randomScore(org.elasticsearch.script.ScoreScript, int) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreDoc double decayGeoLinear(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoLinear double decayGeoExp(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoExp From 91883d362df59a29a3d16c3127eb1d6aaa1fbed2 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 11 Dec 2019 15:14:30 -0700 Subject: [PATCH 07/44] Revert LinkedHashMap --- .../org/elasticsearch/common/xcontent/XContentParser.java | 3 +-- .../elasticsearch/common/xcontent/XContentSubParser.java | 3 +-- .../common/xcontent/support/AbstractXContentParser.java | 6 +++--- .../java/org/elasticsearch/test/test/ESTestCaseTests.java | 4 ++-- .../watcher/support/xcontent/WatcherXContentParser.java | 3 +-- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index 4bc9cbb682fab..82a663bd9dc5d 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -24,7 +24,6 @@ import java.io.Closeable; import java.io.IOException; import java.nio.CharBuffer; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -133,7 +132,7 @@ enum NumberType { Map map() throws IOException; - LinkedHashMap mapOrdered() throws IOException; + Map mapOrdered() throws IOException; Map mapStrings() throws IOException; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java index 39657ca168922..9a8686001e2dc 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.nio.CharBuffer; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -100,7 +99,7 @@ public Map map() throws IOException { } @Override - public LinkedHashMap mapOrdered() throws IOException { + public Map mapOrdered() throws IOException { return parser.mapOrdered(); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index 22d695acccf06..264af205e488b 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -267,7 +267,7 @@ public Map map() throws IOException { } @Override - public LinkedHashMap mapOrdered() throws IOException { + public Map mapOrdered() throws IOException { return readOrderedMap(this); } @@ -302,8 +302,8 @@ static Map readMap(XContentParser parser) throws IOException { return readMap(parser, SIMPLE_MAP_FACTORY); } - static LinkedHashMap readOrderedMap(XContentParser parser) throws IOException { - return (LinkedHashMap)readMap(parser, ORDERED_MAP_FACTORY); + static Map readOrderedMap(XContentParser parser) throws IOException { + return readMap(parser, ORDERED_MAP_FACTORY); } static Map readMapStrings(XContentParser parser) throws IOException { diff --git a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java index e667d441900ba..af041a70d977c 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java @@ -78,7 +78,7 @@ public void testShuffleMap() throws IOException { XContentType xContentType = randomFrom(XContentType.values()); BytesReference source = RandomObjects.randomSource(random(), xContentType, 5); try (XContentParser parser = createParser(xContentType.xContent(), source)) { - LinkedHashMap initialMap = parser.mapOrdered(); + LinkedHashMap initialMap = (LinkedHashMap)parser.mapOrdered(); Set> distinctKeys = new HashSet<>(); for (int i = 0; i < 10; i++) { @@ -123,7 +123,7 @@ public void testShuffleXContentExcludeFields() throws IOException { BytesReference bytes = BytesReference.bytes(builder); final LinkedHashMap initialMap; try (XContentParser parser = createParser(xContentType.xContent(), bytes)) { - initialMap = parser.mapOrdered(); + initialMap = (LinkedHashMap)parser.mapOrdered(); } List expectedInnerKeys1 = Arrays.asList("inner1", "inner2", "inner3"); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java index 05e8e76e8e9cf..20b0086c1e4e2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/support/xcontent/WatcherXContentParser.java @@ -20,7 +20,6 @@ import java.nio.CharBuffer; import java.time.Clock; import java.time.ZonedDateTime; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -112,7 +111,7 @@ public Map map() throws IOException { } @Override - public LinkedHashMap mapOrdered() throws IOException { + public Map mapOrdered() throws IOException { return parser.mapOrdered(); } From a5468acaa51a70af724e1dfa6573ff99b6504173 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 11 Dec 2019 15:19:56 -0700 Subject: [PATCH 08/44] MaxAggregatorTests.testDontCacheScripts -> testCacheScripts --- .../search/aggregations/metrics/MaxAggregatorTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index cccbcb8084122..f86aaefc297a6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -948,9 +948,9 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script does not get cached. + * Make sure that an aggregation using a script does get cached. */ - public void testDontCacheScripts() throws IOException { + public void testCacheScripts() throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; From 5b14a330063796769e6a18875ee7cf2769b346d3 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 11 Dec 2019 15:23:50 -0700 Subject: [PATCH 09/44] remove stale TODOs --- .../resources/org/elasticsearch/painless/spi/java.lang.txt | 1 - .../resources/org/elasticsearch/painless/spi/java.util.txt | 5 ----- 2 files changed, 6 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt index 6987458293993..996b94df94cee 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt @@ -48,7 +48,6 @@ class java.lang.Comparable { int compareTo(def) } -# TODO(stu): evaluate in the case of hashset/hashmap class java.lang.Iterable { void forEach(Consumer) Iterator iterator() diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt index 5559390c1c2c9..8485bc53e5e91 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt @@ -1016,11 +1016,6 @@ class java.util.TreeMap { () (Comparator) def clone() - # TODO(stu): add methods from map declared non-determ in map - Set entrySet() - void forEach(BiConsumer) - Set keySet() - Collection values() } class java.util.TreeSet { From 25220c412e28e97c582f8fdaaa06eeacf515cf86 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 11 Dec 2019 16:22:06 -0700 Subject: [PATCH 10/44] handle imported and static methods --- .../painless/lookup/PainlessClassBinding.java | 6 +++- .../lookup/PainlessLookupBuilder.java | 28 +++++++++++-------- .../painless/node/ECallLocal.java | 3 ++ .../painless/spi/org.elasticsearch.score.txt | 6 ++-- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java index aedbc936bb1d4..971c39d3fe617 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClassBinding.java @@ -22,6 +22,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; import java.util.Objects; public class PainlessClassBinding { @@ -31,13 +32,16 @@ public class PainlessClassBinding { public final Class returnType; public final List> typeParameters; + public final Map, Object> annotations; - PainlessClassBinding(Constructor javaConstructor, Method javaMethod, Class returnType, List> typeParameters) { + PainlessClassBinding(Constructor javaConstructor, Method javaMethod, Class returnType, List> typeParameters, + Map, Object> annotations) { this.javaConstructor = javaConstructor; this.javaMethod = javaMethod; this.returnType = returnType; this.typeParameters = typeParameters; + this.annotations = annotations; } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index e9813c37ed69a..059382a414006 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -152,20 +152,21 @@ public static PainlessLookup buildFromWhitelists(List whitelists) { } } - // TODO(stu): handle annotations here, need to wire up ClassBinding and ECallLocal for (WhitelistMethod whitelistStatic : whitelist.whitelistImportedMethods) { origin = whitelistStatic.origin; painlessLookupBuilder.addImportedPainlessMethod( whitelist.classLoader, whitelistStatic.augmentedCanonicalClassName, whitelistStatic.methodName, whitelistStatic.returnCanonicalTypeName, - whitelistStatic.canonicalTypeNameParameters); + whitelistStatic.canonicalTypeNameParameters, + whitelistStatic.painlessAnnotations); } for (WhitelistClassBinding whitelistClassBinding : whitelist.whitelistClassBindings) { origin = whitelistClassBinding.origin; painlessLookupBuilder.addPainlessClassBinding( whitelist.classLoader, whitelistClassBinding.targetJavaClassName, whitelistClassBinding.methodName, - whitelistClassBinding.returnCanonicalTypeName, whitelistClassBinding.canonicalTypeNameParameters); + whitelistClassBinding.returnCanonicalTypeName, whitelistClassBinding.canonicalTypeNameParameters, + whitelistClassBinding.painlessAnnotations); } for (WhitelistInstanceBinding whitelistInstanceBinding : whitelist.whitelistInstanceBindings) { @@ -714,7 +715,8 @@ public void addPainlessField(Class targetClass, String fieldName, Class ty } public void addImportedPainlessMethod(ClassLoader classLoader, String targetJavaClassName, - String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { + String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters, + Map, Object> annotations) { Objects.requireNonNull(classLoader); Objects.requireNonNull(targetJavaClassName); @@ -757,10 +759,11 @@ public void addImportedPainlessMethod(ClassLoader classLoader, String targetJava "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } - addImportedPainlessMethod(targetClass, methodName, returnType, typeParameters); + addImportedPainlessMethod(targetClass, methodName, returnType, typeParameters, annotations); } - public void addImportedPainlessMethod(Class targetClass, String methodName, Class returnType, List> typeParameters) { + public void addImportedPainlessMethod(Class targetClass, String methodName, Class returnType, List> typeParameters, + Map, Object> annotations) { Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); Objects.requireNonNull(returnType); @@ -846,9 +849,8 @@ public void addImportedPainlessMethod(Class targetClass, String methodName, C MethodType methodType = methodHandle.type(); PainlessMethod existingImportedPainlessMethod = painlessMethodKeysToImportedPainlessMethods.get(painlessMethodKey); - // TODO(stu): update with annotations PainlessMethod newImportedPainlessMethod = - new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType, Collections.emptyMap()); + new PainlessMethod(javaMethod, targetClass, returnType, typeParameters, methodHandle, methodType, annotations); if (existingImportedPainlessMethod == null) { newImportedPainlessMethod = painlessMethodCache.computeIfAbsent(newImportedPainlessMethod, key -> key); @@ -866,7 +868,8 @@ public void addImportedPainlessMethod(Class targetClass, String methodName, C } public void addPainlessClassBinding(ClassLoader classLoader, String targetJavaClassName, - String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters) { + String methodName, String returnCanonicalTypeName, List canonicalTypeNameParameters, + Map, Object> annotations) { Objects.requireNonNull(classLoader); Objects.requireNonNull(targetJavaClassName); @@ -903,10 +906,11 @@ public void addPainlessClassBinding(ClassLoader classLoader, String targetJavaCl "[[" + targetCanonicalClassName + "], [" + methodName + "], " + canonicalTypeNameParameters + "]"); } - addPainlessClassBinding(targetClass, methodName, returnType, typeParameters); + addPainlessClassBinding(targetClass, methodName, returnType, typeParameters, annotations); } - public void addPainlessClassBinding(Class targetClass, String methodName, Class returnType, List> typeParameters) { + public void addPainlessClassBinding(Class targetClass, String methodName, Class returnType, List> typeParameters, + Map, Object> annotations) { Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); Objects.requireNonNull(returnType); @@ -1043,7 +1047,7 @@ public void addPainlessClassBinding(Class targetClass, String methodName, Cla PainlessClassBinding existingPainlessClassBinding = painlessMethodKeysToPainlessClassBindings.get(painlessMethodKey); PainlessClassBinding newPainlessClassBinding = - new PainlessClassBinding(javaConstructor, javaMethod, returnType, typeParameters); + new PainlessClassBinding(javaConstructor, javaMethod, returnType, typeParameters, annotations); if (existingPainlessClassBinding == null) { newPainlessClassBinding = painlessClassBindingCache.computeIfAbsent(newPainlessClassBinding, key -> key); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index e386f94d01b69..7ac2d96598e1a 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -28,6 +28,7 @@ import org.elasticsearch.painless.lookup.PainlessClassBinding; import org.elasticsearch.painless.lookup.PainlessInstanceBinding; import org.elasticsearch.painless.lookup.PainlessMethod; +import org.elasticsearch.painless.spi.annotation.NonDeterministicAnnotation; import org.elasticsearch.painless.symbol.FunctionTable; import org.objectweb.asm.Label; import org.objectweb.asm.Type; @@ -127,9 +128,11 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { typeParameters = new ArrayList<>(localFunction.getTypeParameters()); actual = localFunction.getReturnType(); } else if (importedMethod != null) { + scriptRoot.setDeterministic(importedMethod.annotations.containsKey(NonDeterministicAnnotation.class) == false); typeParameters = new ArrayList<>(importedMethod.typeParameters); actual = importedMethod.returnType; } else if (classBinding != null) { + scriptRoot.setDeterministic(classBinding.annotations.containsKey(NonDeterministicAnnotation.class) == false); typeParameters = new ArrayList<>(classBinding.typeParameters); actual = classBinding.returnType; bindingName = scriptRoot.getNextSyntheticName("class_binding"); diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt index 71007a3312a5d..18504e8b9c911 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt @@ -25,10 +25,8 @@ class org.elasticsearch.script.ScoreScript @no_import { static_import { double saturation(double, double) from_class org.elasticsearch.script.ScoreScriptUtils double sigmoid(double, double, double) from_class org.elasticsearch.script.ScoreScriptUtils - #TODO(stu): @nondeterministic - double randomScore(org.elasticsearch.script.ScoreScript, int, String) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreField - #TODO(stu): @nondeterministic - double randomScore(org.elasticsearch.script.ScoreScript, int) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreDoc + double randomScore(org.elasticsearch.script.ScoreScript, int, String) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreField @nondeterministic + double randomScore(org.elasticsearch.script.ScoreScript, int) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreDoc @nondeterministic double decayGeoLinear(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoLinear double decayGeoExp(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoExp double decayGeoGauss(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoGauss From 39337fb0dfa2c9eaa0afaea93bae61ca4a17e53c Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 12 Dec 2019 15:36:20 -0700 Subject: [PATCH 11/44] setDeterministic -> markNonDeterministic --- .../src/main/java/org/elasticsearch/painless/ScriptRoot.java | 2 +- .../main/java/org/elasticsearch/painless/node/ECallLocal.java | 4 ++-- .../main/java/org/elasticsearch/painless/node/ENewObj.java | 2 +- .../java/org/elasticsearch/painless/node/PCallInvoke.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java index ca377d8a673d2..79795522064f4 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java @@ -74,5 +74,5 @@ public String getNextSyntheticName(String prefix) { return prefix + "$synthetic$" + syntheticCounter++; } - public void setDeterministic(boolean deterministic) { this.deterministic &= deterministic; } + public void markNonDeterministic(boolean deterministic) { this.deterministic &= deterministic; } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java index 7ac2d96598e1a..b96782f14855f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java @@ -128,11 +128,11 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { typeParameters = new ArrayList<>(localFunction.getTypeParameters()); actual = localFunction.getReturnType(); } else if (importedMethod != null) { - scriptRoot.setDeterministic(importedMethod.annotations.containsKey(NonDeterministicAnnotation.class) == false); + scriptRoot.markNonDeterministic(importedMethod.annotations.containsKey(NonDeterministicAnnotation.class)); typeParameters = new ArrayList<>(importedMethod.typeParameters); actual = importedMethod.returnType; } else if (classBinding != null) { - scriptRoot.setDeterministic(classBinding.annotations.containsKey(NonDeterministicAnnotation.class) == false); + scriptRoot.markNonDeterministic(classBinding.annotations.containsKey(NonDeterministicAnnotation.class)); typeParameters = new ArrayList<>(classBinding.typeParameters); actual = classBinding.returnType; bindingName = scriptRoot.getNextSyntheticName("class_binding"); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java index d75814a0205c0..0bc5b751f8831 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java @@ -76,7 +76,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "constructor [" + typeToCanonicalTypeName(actual) + ", /" + arguments.size() + "] not found")); } - scriptRoot.setDeterministic(constructor.annotations.containsKey(NonDeterministicAnnotation.class) == false); + scriptRoot.markNonDeterministic(constructor.annotations.containsKey(NonDeterministicAnnotation.class)); Class[] types = new Class[constructor.typeParameters.size()]; constructor.typeParameters.toArray(types); diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java index 44031f2431c3c..e71f70d632579 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java @@ -80,7 +80,7 @@ void analyze(ScriptRoot scriptRoot, Locals locals) { "method [" + typeToCanonicalTypeName(prefix.actual) + ", " + name + "/" + arguments.size() + "] not found")); } - scriptRoot.setDeterministic(method.annotations.containsKey(NonDeterministicAnnotation.class) == false); + scriptRoot.markNonDeterministic(method.annotations.containsKey(NonDeterministicAnnotation.class)); sub = new PSubCallInvoke(location, method, prefix.actual, arguments); } From fec15a6499f3103a156ced9c72a06277f56763d4 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 12 Dec 2019 16:30:40 -0700 Subject: [PATCH 12/44] MultiValuesSourceFieldConfig, use {read,write}OptionalString for fieldName --- .../aggregations/support/MultiValuesSourceFieldConfig.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java index 54baba9b6b7e5..d0bafcaf3c54a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java @@ -80,7 +80,7 @@ private MultiValuesSourceFieldConfig(String fieldName, Object missing, Script sc } public MultiValuesSourceFieldConfig(StreamInput in) throws IOException { - this.fieldName = in.readString(); + this.fieldName = in.readOptionalString(); this.missing = in.readGenericValue(); this.script = in.readOptionalWriteable(Script::new); this.timeZone = in.readOptionalZoneId(); @@ -104,7 +104,7 @@ public String getFieldName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeString(fieldName); + out.writeOptionalString(fieldName); out.writeGenericValue(missing); out.writeOptionalWriteable(script); out.writeOptionalZoneId(timeZone); From 91062b427d7974909ef19296db10c9e64ca56256 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 13:18:58 -0700 Subject: [PATCH 13/44] this.deterministic &= deterministic -> this.deterministic &= !nondeterministic; --- .../src/main/java/org/elasticsearch/painless/ScriptRoot.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java index 79795522064f4..775da59795b94 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptRoot.java @@ -74,5 +74,5 @@ public String getNextSyntheticName(String prefix) { return prefix + "$synthetic$" + syntheticCounter++; } - public void markNonDeterministic(boolean deterministic) { this.deterministic &= deterministic; } + public void markNonDeterministic(boolean nondeterministic) { this.deterministic &= !nondeterministic; } } From 681d2713ed38f1e7b5fe875a06433c6daaf879b2 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 13:53:14 -0700 Subject: [PATCH 14/44] guard writeOptional against pre-v7.6.0 --- .../support/MultiValuesSourceFieldConfig.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java index d0bafcaf3c54a..a7412f10ceccb 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.support; +import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -80,7 +81,11 @@ private MultiValuesSourceFieldConfig(String fieldName, Object missing, Script sc } public MultiValuesSourceFieldConfig(StreamInput in) throws IOException { - this.fieldName = in.readOptionalString(); + if (in.getVersion().onOrAfter(Version.V_7_6_0)) { + this.fieldName = in.readOptionalString(); + } else { + this.fieldName = in.readString(); + } this.missing = in.readGenericValue(); this.script = in.readOptionalWriteable(Script::new); this.timeZone = in.readOptionalZoneId(); @@ -104,7 +109,11 @@ public String getFieldName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeOptionalString(fieldName); + if (out.getVersion().onOrAfter(Version.V_7_6_0)) { + out.writeOptionalString(fieldName); + } else { + out.writeString(fieldName); + } out.writeGenericValue(missing); out.writeOptionalWriteable(script); out.writeOptionalZoneId(timeZone); From c5ccb6f1e295b53807bf60105a7c3b852cd93747 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 14:30:09 -0700 Subject: [PATCH 15/44] disable bwc testing until 7.6 backport --- build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index df1641dcc9efe..5acaea077440f 100644 --- a/build.gradle +++ b/build.gradle @@ -205,7 +205,9 @@ task verifyVersions { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = true +// TODO(stu): re-enable when backported to 7.6 +// boolean bwc_tests_enabled = true +boolean bwc_tests_enabled = false final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { From e26999e37c7aa8050f2cc188ceef895cd719c6c7 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 14:31:47 -0700 Subject: [PATCH 16/44] add pr link --- build.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 5acaea077440f..6f786b5c3beea 100644 --- a/build.gradle +++ b/build.gradle @@ -205,10 +205,8 @@ task verifyVersions { * after the backport of the backcompat code is complete. */ -// TODO(stu): re-enable when backported to 7.6 -// boolean bwc_tests_enabled = true boolean bwc_tests_enabled = false -final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ +final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/50106" /* place a PR link here when committing bwc changes */ if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") From b11dd3a4e7b24284266c8f5ea8334fb2d83af109 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 14:39:27 -0700 Subject: [PATCH 17/44] System.nanoTime/currentTimeMillis non, Collections.shuffle with rand det, new Random with seed det, remove scorescriptutils non --- .../resources/org/elasticsearch/painless/spi/java.lang.txt | 4 ++-- .../resources/org/elasticsearch/painless/spi/java.util.txt | 4 ++-- .../elasticsearch/painless/spi/org.elasticsearch.score.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt index 996b94df94cee..b900f62d7fe98 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.lang.txt @@ -844,8 +844,8 @@ class java.lang.StringBuilder { class java.lang.System { void arraycopy(Object,int,Object,int,int) - long currentTimeMillis() - long nanoTime() + long currentTimeMillis() @nondeterministic + long nanoTime() @nondeterministic } # Thread: skipped diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt index 8485bc53e5e91..39b52ee104c75 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt @@ -575,7 +575,7 @@ class java.util.Collections { Comparator reverseOrder(Comparator) void rotate(List,int) void shuffle(List) @nondeterministic - void shuffle(List,Random) @nondeterministic + void shuffle(List,Random) Set singleton(def) List singletonList(def) Map singletonMap(def,def) @@ -911,7 +911,7 @@ class java.util.PriorityQueue { class java.util.Random { () @nondeterministic - (long) @nondeterministic + (long) DoubleStream doubles(long) @nondeterministic DoubleStream doubles(long,double,double) @nondeterministic IntStream ints(long) @nondeterministic diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt index 18504e8b9c911..fd49cbb2d5222 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.score.txt @@ -25,8 +25,8 @@ class org.elasticsearch.script.ScoreScript @no_import { static_import { double saturation(double, double) from_class org.elasticsearch.script.ScoreScriptUtils double sigmoid(double, double, double) from_class org.elasticsearch.script.ScoreScriptUtils - double randomScore(org.elasticsearch.script.ScoreScript, int, String) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreField @nondeterministic - double randomScore(org.elasticsearch.script.ScoreScript, int) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreDoc @nondeterministic + double randomScore(org.elasticsearch.script.ScoreScript, int, String) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreField + double randomScore(org.elasticsearch.script.ScoreScript, int) bound_to org.elasticsearch.script.ScoreScriptUtils$RandomScoreDoc double decayGeoLinear(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoLinear double decayGeoExp(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoExp double decayGeoGauss(String, String, String, double, GeoPoint) bound_to org.elasticsearch.script.ScoreScriptUtils$DecayGeoGauss From 2d45bed4bdae970dcb2c9c5995e7f84fed16c54b Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Fri, 13 Dec 2019 16:41:38 -0700 Subject: [PATCH 18/44] Update script cache testing in HDRPercentilesIT, TDigestPercentileRanksIT andCardinalityIT --- .../aggregations/metrics/CardinalityIT.java | 18 ++++-------------- .../metrics/HDRPercentilesIT.java | 19 ++++--------------- .../metrics/TDigestPercentileRanksIT.java | 18 ++++-------------- 3 files changed, 12 insertions(+), 43 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java index 347ec478e19e0..6e781ce0a8fcb 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java @@ -449,10 +449,10 @@ public void testAsSubAgg() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} + * AggregationScript.Factory is always cacheable. */ - public void testDontCacheScripts() throws Exception { + public void testCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -465,23 +465,13 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a script gets cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation( cardinality("foo").field("d").script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value", emptyMap()))) .get(); assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getHitCount(), equalTo(0L)); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(0L)); - - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(cardinality("foo").field("d")).get(); - assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java index 79f8ce4f829a6..6e097aa01057c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java @@ -523,10 +523,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} + * AggregationScript.Factory is always cacheable. */ - public void testDontCacheScripts() throws Exception { + public void testCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -539,24 +539,13 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a script gets cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0) .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .get(); assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getHitCount(), equalTo(0L)); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(0L)); - - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0) - .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0)).get(); - assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java index 5051cee0dbf07..3501ed57b6430 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java @@ -484,10 +484,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} + * AggregationScript.Factory is always cacheable. */ - public void testDontCacheScripts() throws Exception { + public void testCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -500,23 +500,13 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a script gets cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentileRanks("foo", new double[]{50.0}) .field("d") .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getHitCount(), equalTo(0L)); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(0L)); - - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get(); - assertSearchResponse(r); - assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() From 0b58b9d18a896a30f8017244739e980a59f67c5b Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 15:59:35 -0700 Subject: [PATCH 19/44] Add MockDeterministicScript and use it in MockScriptEngine. Add nonDeterministicPluginScripts in MockScriptPlugin as default, overrideable method. If a tests wants to ensure non-deterministic scripts are cached, then it can return a non-empty collection here and use the script later. Allows tests to add deterministic->cacheable tests for scripts. Add script cache testings to DateHistorgramIT. Revert AvgAggregatorTests, CardinalityIT, ExtendedStatsIT, HDRPercentilesIT, MaxAggregatorTests, ScriptedMetricIT, TDigestPercentileRanksIT to prepare to reimplement --- .../aggregations/bucket/DateHistogramIT.java | 24 +++-- .../bucket/DateScriptMocksPlugin.java | 8 ++ .../metrics/AvgAggregatorTests.java | 8 +- .../aggregations/metrics/CardinalityIT.java | 18 +++- .../aggregations/metrics/ExtendedStatsIT.java | 4 +- .../metrics/HDRPercentilesIT.java | 19 +++- .../metrics/MaxAggregatorTests.java | 6 +- .../metrics/ScriptedMetricIT.java | 8 +- .../metrics/TDigestPercentileRanksIT.java | 18 +++- .../script/MockDeterministicScript.java | 45 +++++++++ .../script/MockScriptEngine.java | 95 ++++++++++--------- .../script/MockScriptPlugin.java | 4 +- 12 files changed, 178 insertions(+), 79 deletions(-) create mode 100644 test/framework/src/main/java/org/elasticsearch/script/MockDeterministicScript.java diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java index dbf527aa6811a..ad26eb5b83b9a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java @@ -1466,10 +1466,10 @@ public void testDSTEndTransition() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request + * Make sure that a request using a deterministic script does not get cached and a request * not using a script does get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=date") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -1484,10 +1484,21 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached Map params = new HashMap<>(); params.put("fieldname", "d"); SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateHistogram("histo").field("d") + .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.CURRENT_DATE, params)) + .dateHistogramInterval(DateHistogramInterval.MONTH)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateHistogram("histo").field("d") .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.LONG_PLUS_ONE_MONTH, params)) .dateHistogramInterval(DateHistogramInterval.MONTH)).get(); assertSearchResponse(r); @@ -1495,10 +1506,9 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(0L)); + .getMissCount(), equalTo(1L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(dateHistogram("histo").field("d").dateHistogramInterval(DateHistogramInterval.MONTH)).get(); assertSearchResponse(r); @@ -1506,7 +1516,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } public void testSingleValuedFieldOrderedBySingleValueSubAggregationAscAndKeyDesc() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateScriptMocksPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateScriptMocksPlugin.java index 1398961ced8af..07f7adf5e8446 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateScriptMocksPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateScriptMocksPlugin.java @@ -38,6 +38,7 @@ public class DateScriptMocksPlugin extends MockScriptPlugin { static final String EXTRACT_FIELD = "extract_field"; static final String DOUBLE_PLUS_ONE_MONTH = "double_date_plus_1_month"; static final String LONG_PLUS_ONE_MONTH = "long_date_plus_1_month"; + static final String CURRENT_DATE = "current_date"; @Override public Map, Object>> pluginScripts() { @@ -53,4 +54,11 @@ public Map, Object>> pluginScripts() { new DateTime((long) params.get("_value"), DateTimeZone.UTC).plusMonths(1).getMillis()); return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + scripts.put(CURRENT_DATE, params -> new DateTime().getMillis()); + return scripts; + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java index 4afc8d91771fd..4dfdfd2fd91ca 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java @@ -638,9 +638,9 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script deterministic script from MockScriptEngine (Aggs is deterministic there) do get cached. + * Make sure that an aggregation using a script does not get cached. */ - public void testCacheScripts() throws IOException { + public void testDontCacheScripts() throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; @@ -675,8 +675,8 @@ public void testCacheScripts() throws IOException { assertEquals("avg", avg.getName()); assertTrue(AggregationInspectionHelper.hasValue(avg)); - // Test that an aggregation using a script does get cached - assertTrue(aggregator.context().getQueryShardContext().isCacheable()); + // Test that an aggregation using a script does not get cached + assertFalse(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); directory.close(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java index 6e781ce0a8fcb..347ec478e19e0 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java @@ -449,10 +449,10 @@ public void testAsSubAgg() throws Exception { } /** - * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} - * AggregationScript.Factory is always cacheable. + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. */ - public void testCacheScripts() throws Exception { + public void testDontCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -465,13 +465,23 @@ public void testCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script gets cached + // Test that a request using a script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation( cardinality("foo").field("d").script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value", emptyMap()))) .get(); assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(cardinality("foo").field("d")).get(); + assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java index 80a26ee9ebb2f..11856ea34dc20 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java @@ -639,8 +639,8 @@ private void checkUpperLowerBounds(ExtendedStats stats, double sigma) { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} + * AggregationScript.Factory is always cacheable. */ public void testDontCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java index 6e097aa01057c..79f8ce4f829a6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java @@ -523,10 +523,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} - * AggregationScript.Factory is always cacheable. + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. */ - public void testCacheScripts() throws Exception { + public void testDontCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -539,13 +539,24 @@ public void testCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script gets cached + // Test that a request using a script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0) .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .get(); assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0)).get(); + assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index f86aaefc297a6..739419a534240 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -948,9 +948,9 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script does get cached. + * Make sure that an aggregation using a script does not get cached. */ - public void testCacheScripts() throws IOException { + public void testDontCacheScripts() throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; @@ -987,7 +987,7 @@ public void testCacheScripts() throws IOException { assertTrue(AggregationInspectionHelper.hasValue(max)); // Test that an aggregation using a script does not get cached - assertTrue(aggregator.context().getQueryShardContext().isCacheable()); + assertFalse(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); directory.close(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index 71ceeb1abeb72..9aaa09bdd8dac 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -1016,8 +1016,8 @@ public void testEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} + * AggregationScript.Factory is always cacheable. */ public void testDontCacheScripts() throws Exception { Script mapScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "state['count'] = 1", Collections.emptyMap()); @@ -1038,7 +1038,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Ensure that a request not using a script also gets cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(scriptedMetric("foo").mapScript(mapScript).combineScript(combineScript).reduceScript(reduceScript)).get(); assertSearchResponse(r); @@ -1046,7 +1046,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(0L)); + .getMissCount(), equalTo(2L)); } public void testConflictingAggAndScriptParams() { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java index 3501ed57b6430..5051cee0dbf07 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java @@ -484,10 +484,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} - * AggregationScript.Factory is always cacheable. + * Make sure that a request using a script does not get cached and a request + * not using a script does get cached. */ - public void testCacheScripts() throws Exception { + public void testDontCacheScripts() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -500,13 +500,23 @@ public void testCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script gets cached + // Test that a request using a script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentileRanks("foo", new double[]{50.0}) .field("d") .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // To make sure that the cache is working test that a request not using + // a script is cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get(); + assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockDeterministicScript.java b/test/framework/src/main/java/org/elasticsearch/script/MockDeterministicScript.java new file mode 100644 index 0000000000000..8cafd512450a7 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/script/MockDeterministicScript.java @@ -0,0 +1,45 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.script; + +import java.util.Map; +import java.util.function.Function; + +/** + * A mocked script used for testing purposes. {@code deterministic} implies cacheability in query shard cache. + */ +public abstract class MockDeterministicScript implements Function, Object>, ScriptFactory { + public abstract Object apply(Map vars); + public abstract boolean isResultDeterministic(); + + public static MockDeterministicScript asDeterministic(Function, Object> script) { + return new MockDeterministicScript() { + @Override public boolean isResultDeterministic() { return true; } + @Override public Object apply(Map vars) { return script.apply(vars); } + }; + } + + public static MockDeterministicScript asNonDeterministic(Function, Object> script) { + return new MockDeterministicScript() { + @Override public boolean isResultDeterministic() { return false; } + @Override public Object apply(Map vars) { return script.apply(vars); } + }; + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 731ef6298ce55..a38f14e128727 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -62,11 +62,22 @@ public interface ContextCompiler { public static final String NAME = "mockscript"; private final String type; - private final Map, Object>> scripts; + private final Map scripts; private final Map, ContextCompiler> contexts; public MockScriptEngine(String type, Map, Object>> scripts, Map, ContextCompiler> contexts) { + this(type, scripts, Collections.emptyMap(), contexts); + } + + public MockScriptEngine(String type, Map, Object>> deterministicScripts, + Map, Object>> nonDeterministicScripts, + Map, ContextCompiler> contexts) { + + Map scripts = new HashMap<>(deterministicScripts.size() + nonDeterministicScripts.size()); + deterministicScripts.forEach((key, value) -> scripts.put(key, MockDeterministicScript.asDeterministic(value))); + nonDeterministicScripts.forEach((key, value) -> scripts.put(key, MockDeterministicScript.asNonDeterministic(value))); + this.type = type; this.scripts = Collections.unmodifiableMap(scripts); this.contexts = Collections.unmodifiableMap(contexts); @@ -85,12 +96,12 @@ public String getType() { public T compile(String name, String source, ScriptContext context, Map params) { // Scripts are always resolved using the script's source. For inline scripts, it's easy because they don't have names and the // source is always provided. For stored and file scripts, the source of the script must match the key of a predefined script. - Function, Object> script = scripts.get(source); + MockDeterministicScript script = scripts.get(source); if (script == null) { throw new IllegalArgumentException("No pre defined script matching [" + source + "] for script with name [" + name + "], " + "did you declare the mocked script?"); } - MockCompiledScript mockCompiled = new MockCompiledScript(name, params, source, script); + MockCompiledScript mockCompiled = new MockCompiledScript(name, params, source, script::apply); if (context.instanceClazz.equals(FieldScript.class)) { FieldScript.Factory factory = (parameters, lookup) -> ctx -> new FieldScript(parameters, lookup, ctx) { @@ -156,47 +167,7 @@ public void execute(Map ctx) { }; return context.factoryClazz.cast(factory); } else if(context.instanceClazz.equals(AggregationScript.class)) { - AggregationScript.Factory factory = new AggregationScript.Factory() { - @Override - public AggregationScript.LeafFactory newFactory(Map params, SearchLookup lookup) { - return new AggregationScript.LeafFactory() { - @Override - public AggregationScript newInstance(final LeafReaderContext ctx) { - return new AggregationScript(params, lookup, ctx) { - @Override - public Object execute() { - Map vars = new HashMap<>(params); - vars.put("params", params); - vars.put("doc", getDoc()); - vars.put("_score", get_score()); - vars.put("_value", get_value()); - return script.apply(vars); - } - }; - } - - @Override - public boolean needs_score() { - return true; - } - }; - } - - @Override - public boolean isResultDeterministic() { - return true; - } - }; - return context.factoryClazz.cast(factory); - } else if (context.instanceClazz.equals(IngestScript.class)) { - IngestScript.Factory factory = vars -> - new IngestScript(vars) { - @Override - public void execute(Map ctx) { - script.apply(ctx); - } - }; - return context.factoryClazz.cast(factory); + return context.factoryClazz.cast(new MockAggregationScript(script)); } else if (context.instanceClazz.equals(IngestConditionalScript.class)) { IngestConditionalScript.Factory factory = parameters -> new IngestConditionalScript(parameters) { @Override @@ -279,7 +250,7 @@ public double execute(Map params1, double[] values) { }; return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScoreScript.class)) { - ScoreScript.Factory factory = new MockScoreScript(script); + ScoreScript.Factory factory = new MockScoreScript(script::apply); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScriptedMetricAggContexts.InitScript.class)) { ScriptedMetricAggContexts.InitScript.Factory factory = mockCompiled::createMetricAggInitScript; @@ -299,7 +270,7 @@ public double execute(Map params1, double[] values) { } ContextCompiler compiler = contexts.get(context); if (compiler != null) { - return context.factoryClazz.cast(compiler.compile(script, params)); + return context.factoryClazz.cast(compiler.compile(script::apply, params)); } throw new IllegalArgumentException("mock script engine does not know how to handle context [" + context.name + "]"); } @@ -614,4 +585,36 @@ public void setScorer(Scorable scorer) { } } + class MockAggregationScript implements AggregationScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + public MockAggregationScript(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public AggregationScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return new AggregationScript.LeafFactory() { + @Override + public AggregationScript newInstance(final LeafReaderContext ctx) { + return new AggregationScript(params, lookup, ctx) { + @Override + public Object execute() { + Map vars = new HashMap<>(params); + vars.put("params", params); + vars.put("doc", getDoc()); + vars.put("_score", get_score()); + vars.put("_value", get_value()); + return script.apply(vars); + } + }; + } + + @Override + public boolean needs_score() { + return true; + } + }; + } + } + + } diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptPlugin.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptPlugin.java index 34aca79ec4725..972879a735a72 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptPlugin.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptPlugin.java @@ -37,11 +37,13 @@ public abstract class MockScriptPlugin extends Plugin implements ScriptPlugin { @Override public ScriptEngine getScriptEngine(Settings settings, Collection> contexts) { - return new MockScriptEngine(pluginScriptLang(), pluginScripts(), pluginContextCompilers()); + return new MockScriptEngine(pluginScriptLang(), pluginScripts(), nonDeterministicPluginScripts(), pluginContextCompilers()); } protected abstract Map, Object>> pluginScripts(); + protected Map, Object>> nonDeterministicPluginScripts() { return Collections.emptyMap(); } + protected Map, MockScriptEngine.ContextCompiler> pluginContextCompilers() { return Collections.emptyMap(); } From dd2c3c3dd2091f48a40e40a1d44145872582b249 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:05:35 -0700 Subject: [PATCH 20/44] Update DateRangeIT --- .../aggregations/bucket/DateHistogramIT.java | 4 ++-- .../aggregations/bucket/DateRangeIT.java | 24 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java index ad26eb5b83b9a..0dceff9d0f0d1 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java @@ -1466,8 +1466,8 @@ public void testDSTEndTransition() throws Exception { } /** - * Make sure that a request using a deterministic script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=date") diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java index 820ea3786508c..a53f5be595eec 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java @@ -884,10 +884,10 @@ public void testNoRangesInQuery() { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "date", "type=date") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -903,11 +903,11 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached Map params = new HashMap<>(); params.put("fieldname", "date"); SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") - .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.DOUBLE_PLUS_ONE_MONTH, params)) + .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.CURRENT_DATE, params)) .addRange(ZonedDateTime.of(2012, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), ZonedDateTime.of(2013, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC))) .get(); @@ -918,6 +918,18 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") + .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.DOUBLE_PLUS_ONE_MONTH, params)) + .addRange(ZonedDateTime.of(2012, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), + ZonedDateTime.of(2013, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + // To make sure that the cache is working test that a request not using // a script is cached r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") @@ -929,7 +941,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } /** From 9020cdd2c9bdb724fea457c5e3d237ae814286c6 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:10:24 -0700 Subject: [PATCH 21/44] Update DoubleTermsIT --- .../aggregations/bucket/DateRangeIT.java | 4 +-- .../aggregations/bucket/DoubleTermsIT.java | 35 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java index a53f5be595eec..4b6bdf96b891e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java @@ -918,6 +918,7 @@ public void testScriptCaching() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); + // Test that a request using a deterministic script gets cached r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") .script(new Script(ScriptType.INLINE, "mockscript", DateScriptMocksPlugin.DOUBLE_PLUS_ONE_MONTH, params)) .addRange(ZonedDateTime.of(2012, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), @@ -930,8 +931,7 @@ public void testScriptCaching() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(dateRange("foo").field("date") .addRange(ZonedDateTime.of(2012, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC), ZonedDateTime.of(2013, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC))) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java index e99931c62102b..94b9dc73841d6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java @@ -112,6 +112,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } private static final int NUM_DOCS = 5; // TODO: randomize the size? @@ -917,10 +926,10 @@ public void testOtherDocCount() { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=float") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -933,10 +942,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( terms("terms").field("d").script( - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -944,14 +953,24 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + terms("terms").field("d").script( + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } } From b0fc18b4af2299d90984615e5e592754b60d39f5 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:18:40 -0700 Subject: [PATCH 22/44] Update HistogramIT --- .../aggregations/bucket/HistogramIT.java | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java index f86aef40834af..c071fbc2a7b10 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java @@ -110,6 +110,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } @Override @@ -1102,10 +1111,10 @@ public void testDecimalIntervalAndOffset() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=float") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -1118,9 +1127,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(histogram("histo").field("d") - .script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", emptyMap())).interval(0.7).offset(0.05)).get(); + .script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", emptyMap())).interval(0.7).offset(0.05)) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -1128,8 +1138,17 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(histogram("histo").field("d") + .script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", emptyMap())).interval(0.7).offset(0.05)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(histogram("histo").field("d").interval(0.7).offset(0.05)) .get(); assertSearchResponse(r); @@ -1137,7 +1156,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } public void testSingleValuedFieldOrderedBySingleValueSubAggregationAscAndKeyDesc() throws Exception { From 81d6df93ddc5a85285e15e9f0faa6dfb3e22c5de Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:22:31 -0700 Subject: [PATCH 23/44] Update LongTermsIT --- .../aggregations/bucket/LongTermsIT.java | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java index 1c0a5de546766..2903d19ac7cf5 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java @@ -99,6 +99,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } private static final int NUM_DOCS = 5; // TODO randomize the size? @@ -894,10 +903,10 @@ public void testOtherDocCount() { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -910,10 +919,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( terms("terms").field("d").script( - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -921,14 +930,24 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + terms("terms").field("d").script( + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From fcb6ac6337ff23f778c2304a9d61b619a1c1aa0c Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:26:20 -0700 Subject: [PATCH 24/44] Update RangeIT --- .../search/aggregations/bucket/RangeIT.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java index f7a1ce30d1ed7..05f25aa1dbda7 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java @@ -92,6 +92,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } @Override @@ -945,10 +954,10 @@ public void testEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "i", "type=integer") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -962,12 +971,12 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached Map params = new HashMap<>(); params.put("fieldname", "date"); SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( range("foo").field("i").script( - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap())).addRange(0, 10)) + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap())).addRange(0, 10)) .get(); assertSearchResponse(r); @@ -976,15 +985,26 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(range("foo").field("i").addRange(0, 10)).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + range("foo").field("i").script( + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value + 1", Collections.emptyMap())).addRange(0, 10)) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(range("foo").field("i").addRange(0, 10)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } public void testFieldAlias() { From 79641f71fd823d5e32fd3fa8b42f5ddf2b12aa1b Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:36:11 -0700 Subject: [PATCH 25/44] Update SignificantTermsSignificanceScoreIT --- .../SignificantTermsSignificanceScoreIT.java | 46 ++++++++++++++----- .../script/MockScriptEngine.java | 22 ++++++--- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java index 2f231083c684d..65c4d583e3d9a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java @@ -193,6 +193,15 @@ public Map, Object>> pluginScripts() { return scripts; } + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } + private static long longValue(Object value) { return ((ScriptHeuristic.LongAccessor) value).longValue(); } @@ -678,10 +687,10 @@ public void testReduceFromSeveralShards() throws IOException, ExecutionException } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -694,8 +703,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached - ScriptHeuristic scriptHeuristic = getScriptSignificanceHeuristic(); + // Test that a request using a nondeterministic script does not get cached + ScriptHeuristic scriptHeuristic = new ScriptHeuristic( + new Script(ScriptType.INLINE, "mockscript", "Math.random()", Collections.emptyMap()) + ); boolean useSigText = randomBoolean(); SearchResponse r; if (useSigText) { @@ -712,12 +723,15 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Test that a request using a deterministic script gets cached + scriptHeuristic = getScriptSignificanceHeuristic(); + useSigText = randomBoolean(); if (useSigText) { - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(significantText("foo", "s")).get(); + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(significantText("foo", "s").significanceHeuristic(scriptHeuristic)).get(); } else { - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(significantTerms("foo").field("s")).get(); + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(significantTerms("foo").field("s").significanceHeuristic(scriptHeuristic)).get(); } assertSearchResponse(r); @@ -725,8 +739,18 @@ public void testDontCacheScripts() throws Exception { .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); - } - + // Ensure that non-scripted requests are cached as normal + if (useSigText) { + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(significantText("foo", "s")).get(); + } else { + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(significantTerms("foo").field("s")).get(); + } + assertSearchResponse(r); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index a38f14e128727..1df582587fef9 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -210,13 +210,7 @@ public boolean execute() { }; return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(SignificantTermsHeuristicScoreScript.class)) { - SignificantTermsHeuristicScoreScript.Factory factory = () -> new SignificantTermsHeuristicScoreScript() { - @Override - public double execute(Map vars) { - return ((Number) script.apply(vars)).doubleValue(); - } - }; - return context.factoryClazz.cast(factory); + return context.factoryClazz.cast(new MockSignificantTermsHeuristicScoreScript(script)); } else if (context.instanceClazz.equals(TemplateScript.class)) { TemplateScript.Factory factory = vars -> { Map varsWithParams = new HashMap<>(); @@ -616,5 +610,19 @@ public boolean needs_score() { } } + class MockSignificantTermsHeuristicScoreScript implements SignificantTermsHeuristicScoreScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + public MockSignificantTermsHeuristicScoreScript(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + @Override + public SignificantTermsHeuristicScoreScript newInstance() { + return new SignificantTermsHeuristicScoreScript() { + @Override + public double execute(Map vars) { + return ((Number) script.apply(vars)).doubleValue(); + } + }; + } + } } From edb16ba789f7263627accce2f6ec215e154c1dfc Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:40:50 -0700 Subject: [PATCH 26/44] Update StringTermsIT --- .../bucket/terms/StringTermsIT.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java index a748eb95bbf58..016bcf099d778 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java @@ -119,6 +119,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } @Override @@ -1115,10 +1124,10 @@ public void testOtherDocCount() { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=keyword") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -1131,11 +1140,11 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation( terms("terms").field("d").script( - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "'foo_' + _value", Collections.emptyMap()))) + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap()))) .get(); assertSearchResponse(r); @@ -1144,6 +1153,19 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation( + terms("terms").field("d").script( + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "'foo_' + _value", Collections.emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + // To make sure that the cache is working test that a request not using // a script is cached r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); @@ -1152,6 +1174,6 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } } From a9c9256bac84740eb509bf1dec0661889c0cca28 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:54:09 -0700 Subject: [PATCH 27/44] Update AvgAggregatorTests --- .../metrics/AvgAggregatorTests.java | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java index 4dfdfd2fd91ca..21925247865ee 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java @@ -83,6 +83,9 @@ public class AvgAggregatorTests extends AggregatorTestCase { /** Script to return the {@code _value} provided by aggs framework. */ public static final String VALUE_SCRIPT = "_value"; + /** Script to return a random double */ + public static final String RANDOM_SCRIPT = "Math.random()"; + @Override protected ScriptService getMockScriptService() { Map, Object>> scripts = new HashMap<>(); @@ -115,8 +118,12 @@ protected ScriptService getMockScriptService() { return ((Number) vars.get("_value")).doubleValue() + inc; }); + Map, Object>> nonDeterministicScripts = new HashMap<>(); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, + nonDeterministicScripts, Collections.emptyMap()); Map engines = Collections.singletonMap(scriptEngine.getType(), scriptEngine); @@ -638,9 +645,10 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script does not get cached. + * Make sure that an aggregation using a deterministic script does gets cached while + * one using a nondeterministic script does not. */ - public void testDontCacheScripts() throws IOException { + public void testScriptCaching() throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; @@ -675,7 +683,26 @@ public void testDontCacheScripts() throws IOException { assertEquals("avg", avg.getName()); assertTrue(AggregationInspectionHelper.hasValue(avg)); - // Test that an aggregation using a script does not get cached + // Test that an aggregation using a deterministic script gets cached + assertTrue(aggregator.context().getQueryShardContext().isCacheable()); + + aggregationBuilder = new AvgAggregationBuilder("avg") + .field("value") + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, RANDOM_SCRIPT, Collections.emptyMap())); + + aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType); + aggregator.preCollection(); + indexSearcher.search(new MatchAllDocsQuery(), aggregator); + aggregator.postCollection(); + + avg = (InternalAvg) aggregator.buildAggregation(0L); + + assertTrue(avg.getValue() >= 0.0); + assertTrue(avg.getValue() <= 1.0); + assertEquals("avg", avg.getName()); + assertTrue(AggregationInspectionHelper.hasValue(avg)); + + // Test that an aggregation using a nondeterministic script does not get cached assertFalse(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); From 4b9c283a3b9374dbf9d8d1305b43de07063ed2af Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 16 Dec 2019 16:59:51 -0700 Subject: [PATCH 28/44] CardinalityIT --- .../bucket/terms/StringTermsIT.java | 3 +- .../aggregations/metrics/CardinalityIT.java | 37 +++++++++++++++---- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java index 016bcf099d778..22ca5af717124 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java @@ -1166,8 +1166,7 @@ public void testScriptCaching() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(terms("terms").field("d")).get(); assertSearchResponse(r); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java index 347ec478e19e0..921f04232307f 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java @@ -90,6 +90,15 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } @Override @@ -449,10 +458,10 @@ public void testAsSubAgg() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -465,10 +474,11 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation( - cardinality("foo").field("d").script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value", emptyMap()))) + cardinality("foo").field("d").script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", + emptyMap()))) .get(); assertSearchResponse(r); @@ -477,14 +487,25 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(cardinality("foo").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation( + cardinality("foo").field("d").script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_value", emptyMap()))) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(cardinality("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From 528a6a4dbbaadd6a37f44342530fd4548c7266f8 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 09:10:03 -0700 Subject: [PATCH 29/44] Update ExtendedStatsIT --- .../AggregationTestScriptsPlugin.java | 9 +++++++ .../aggregations/metrics/ExtendedStatsIT.java | 24 ++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java index 5d71176e79820..738f04336cb55 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java @@ -116,4 +116,13 @@ protected Map, Object>> pluginScripts() { return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java index 11856ea34dc20..496c1e07d4ad0 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java @@ -639,10 +639,10 @@ private void checkUpperLowerBounds(ExtendedStats stats, double sigma) { } /** - * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} - * AggregationScript.Factory is always cacheable. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -655,10 +655,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(extendedStats("foo").field("d") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value + 1", Collections.emptyMap()))) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", Collections.emptyMap()))) .get(); assertSearchResponse(r); @@ -667,6 +667,18 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(extendedStats("foo").field("d") + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value + 1", Collections.emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + // To make sure that the cache is working test that a request not using // a script is cached r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(extendedStats("foo").field("d")).get(); @@ -675,7 +687,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } } From 53be9e468100507b4725e9cc6f945b6e310d68d7 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 09:16:52 -0700 Subject: [PATCH 30/44] Update HDRPercentileRanksIT --- .../aggregations/metrics/ExtendedStatsIT.java | 3 +- .../metrics/HDRPercentileRanksIT.java | 29 ++++++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java index 496c1e07d4ad0..58967969d54a0 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsIT.java @@ -679,8 +679,7 @@ public void testScriptCaching() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(extendedStats("foo").field("d")).get(); assertSearchResponse(r); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java index 7b7aef1b90b74..ae578208ae01b 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java @@ -558,10 +558,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -574,12 +574,12 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client() .prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentileRanks("foo", new double[]{50.0}) .method(PercentilesMethod.HDR).field("d") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))) .get(); assertSearchResponse(r); @@ -588,8 +588,21 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Test that a request using a deterministic script gets cached + r = client() + .prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentileRanks("foo", new double[]{50.0}) + .method(PercentilesMethod.HDR).field("d") + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentileRanks("foo", new double[]{50.0}).method(PercentilesMethod.HDR).field("d")).get(); assertSearchResponse(r); @@ -597,7 +610,7 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() - .getMissCount(), equalTo(1L)); + .getMissCount(), equalTo(2L)); } } From 33064611bf17cb16186f10660a1e8d9db7596226 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 09:21:02 -0700 Subject: [PATCH 31/44] Update HDRPercentilesIT --- .../metrics/HDRPercentilesIT.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java index 79f8ce4f829a6..05ffe4ddc4638 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java @@ -523,10 +523,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -539,10 +539,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0) - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))) .get(); assertSearchResponse(r); @@ -551,16 +551,27 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached + // Test that a request using a deterministic script gets cached r = client().prepareSearch("cache_test_idx").setSize(0) - .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0)).get(); + .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentiles("foo").method(PercentilesMethod.HDR).field("d").percentiles(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From e464221d04444afaef74e26c8ea481d3c99bee62 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 09:34:01 -0700 Subject: [PATCH 32/44] Update MaxAggregatorTests MedianAbsoluteDeviationIT --- .../metrics/MaxAggregatorTests.java | 31 +++++++++++++++++-- .../metrics/MedianAbsoluteDeviationIT.java | 27 +++++++++++----- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index 739419a534240..d70f19f042ebc 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -110,6 +110,9 @@ public class MaxAggregatorTests extends AggregatorTestCase { /** Script to return the {@code _value} provided by aggs framework. */ public static final String VALUE_SCRIPT = "_value"; + /** Script to return a random double */ + public static final String RANDOM_SCRIPT = "Math.random()"; + @Override protected ScriptService getMockScriptService() { Map, Object>> scripts = new HashMap<>(); @@ -143,8 +146,12 @@ protected ScriptService getMockScriptService() { return ((Number) vars.get("_value")).doubleValue() + inc; }); + Map, Object>> nonDeterministicScripts = new HashMap<>(); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, + nonDeterministicScripts, Collections.emptyMap()); Map engines = Collections.singletonMap(scriptEngine.getType(), scriptEngine); @@ -948,9 +955,10 @@ public void testCacheAggregation() throws IOException { } /** - * Make sure that an aggregation using a script does not get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws IOException { + public void testScriptCaching() throws Exception { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); final int numDocs = 10; @@ -968,7 +976,6 @@ public void testDontCacheScripts() throws IOException { MultiReader multiReader = new MultiReader(indexReader, unamappedIndexReader); IndexSearcher indexSearcher = newSearcher(multiReader, true, true); - MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER); fieldType.setName("value"); MaxAggregationBuilder aggregationBuilder = new MaxAggregationBuilder("max") @@ -987,6 +994,24 @@ public void testDontCacheScripts() throws IOException { assertTrue(AggregationInspectionHelper.hasValue(max)); // Test that an aggregation using a script does not get cached + assertTrue(aggregator.context().getQueryShardContext().isCacheable()); + aggregationBuilder = new MaxAggregationBuilder("max") + .field("value") + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, RANDOM_SCRIPT, Collections.emptyMap())); + + aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType); + aggregator.preCollection(); + indexSearcher.search(new MatchAllDocsQuery(), aggregator); + aggregator.postCollection(); + + max = (InternalMax) aggregator.buildAggregation(0L); + + assertTrue(max.getValue() >= 0.0); + assertTrue(max.getValue() <= 1.0); + assertEquals("max", max.getName()); + assertTrue(AggregationInspectionHelper.hasValue(max)); + + // Test that an aggregation using a nondeterministic script does not get cached assertFalse(aggregator.context().getQueryShardContext().isCacheable()); multiReader.close(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationIT.java index b40c8ee8acdf3..7345e16c2fc1c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationIT.java @@ -556,10 +556,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked( prepareCreate("cache_test_idx") .addMapping("type", "d", "type=long") @@ -579,11 +579,11 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(randomBuilder() .field("d") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -591,14 +591,25 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(randomBuilder().field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(randomBuilder() + .field("d") + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(randomBuilder().field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From cf5585f7b5bb8ee868634df7a55b24810d0a6ca9 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 09:37:46 -0700 Subject: [PATCH 33/44] Update MinIT --- .../search/aggregations/metrics/MinIT.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java index 11df5e1cc96c6..a4081345f57be 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinIT.java @@ -369,10 +369,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -385,9 +385,9 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( - min("foo").field("d").script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + min("foo").field("d").script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))) .get(); assertSearchResponse(r); @@ -396,15 +396,25 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(min("foo").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + min("foo").field("d").script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(min("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } public void testEarlyTermination() throws Exception { From b2a113d01c64d09cb7daa3800b3462d4cfcaa55f Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:11:09 -0700 Subject: [PATCH 34/44] Update ScriptedMetricIT --- .../metrics/ScriptedMetricIT.java | 80 +++++++++++++++++-- .../script/MockScriptEngine.java | 80 ++++++++++++------- 2 files changed, 127 insertions(+), 33 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index 9aaa09bdd8dac..dc93e85c7f9f6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -247,6 +247,23 @@ protected Map, Object>> pluginScripts() { return scripts; } + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("state.data = Math.random()", vars -> + aggScript(vars, state -> state.put("data", Math.random()))); + + + scripts.put("state['count'] = Math.random() >= 0.5 ? 1 : 0", vars -> + aggScript(vars, state -> state.put("count", Math.random() >= 0.5 ? 1 : 0))); + + + scripts.put("return Math.random()", vars -> Math.random()); + + return scripts; + } + @SuppressWarnings("unchecked") static Map aggScript(Map vars, Consumer> fn) { Map aggState = (Map) vars.get("state"); @@ -1015,17 +1032,27 @@ public void testEmptyAggregation() throws Exception { assertThat(aggregationResult.get(0), equalTo(0)); } + /** - * Ensure requests using a script get cached. Note: {@link org.elasticsearch.script.MockScriptEngine} - * AggregationScript.Factory is always cacheable. + * Make sure that a request using a deterministic script gets cached and nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { + // TODO(stu): add non-determinism in init, agg, combine and reduce, ensure not cached Script mapScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "state['count'] = 1", Collections.emptyMap()); Script combineScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "no-op aggregation", Collections.emptyMap()); Script reduceScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "no-op list aggregation", Collections.emptyMap()); + Script ndInitScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "state.data = Math.random()", + Collections.emptyMap()); + + Script ndMapScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "state['count'] = Math.random() >= 0.5 ? 1 : 0", + Collections.emptyMap()); + + Script ndRandom = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "return Math.random()", + Collections.emptyMap()); + assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -1038,9 +1065,52 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Ensure that a request not using a script also gets cached + // Test that a non-deterministic init script causes the result to not be cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) - .addAggregation(scriptedMetric("foo").mapScript(mapScript).combineScript(combineScript).reduceScript(reduceScript)).get(); + .addAggregation(scriptedMetric("foo").initScript(ndInitScript).mapScript(mapScript).combineScript(combineScript) + .reduceScript(reduceScript)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a non-deterministic map script causes the result to not be cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(scriptedMetric("foo").mapScript(ndMapScript).combineScript(combineScript).reduceScript(reduceScript)) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // Test that a non-deterministic combine script causes the result to not be cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(scriptedMetric("foo").mapScript(mapScript).combineScript(ndRandom).reduceScript(reduceScript)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(0L)); + + // NOTE: random reduce scripts don't hit the query shard context (they are done on the coordinator) and so can be cached. + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(scriptedMetric("foo").mapScript(mapScript).combineScript(combineScript).reduceScript(ndRandom)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(1L)); + + // Test that all deterministic scripts cause the request to be cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(scriptedMetric("foo").mapScript(mapScript).combineScript(combineScript).reduceScript(reduceScript)) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 1df582587fef9..846f4f829eec1 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -101,7 +101,7 @@ public T compile(String name, String source, ScriptCon throw new IllegalArgumentException("No pre defined script matching [" + source + "] for script with name [" + name + "], " + "did you declare the mocked script?"); } - MockCompiledScript mockCompiled = new MockCompiledScript(name, params, source, script::apply); + MockCompiledScript mockCompiled = new MockCompiledScript(name, params, source, script); if (context.instanceClazz.equals(FieldScript.class)) { FieldScript.Factory factory = (parameters, lookup) -> ctx -> new FieldScript(parameters, lookup, ctx) { @@ -247,16 +247,16 @@ public double execute(Map params1, double[] values) { ScoreScript.Factory factory = new MockScoreScript(script::apply); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScriptedMetricAggContexts.InitScript.class)) { - ScriptedMetricAggContexts.InitScript.Factory factory = mockCompiled::createMetricAggInitScript; + ScriptedMetricAggContexts.InitScript.Factory factory = new MockMetricAggInitScriptFactory(script); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScriptedMetricAggContexts.MapScript.class)) { - ScriptedMetricAggContexts.MapScript.Factory factory = mockCompiled::createMetricAggMapScript; + ScriptedMetricAggContexts.MapScript.Factory factory = new MockMetricAggMapScriptFactory(script); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScriptedMetricAggContexts.CombineScript.class)) { - ScriptedMetricAggContexts.CombineScript.Factory factory = mockCompiled::createMetricAggCombineScript; + ScriptedMetricAggContexts.CombineScript.Factory factory = new MockMetricAggCombineScriptFactory(script); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(ScriptedMetricAggContexts.ReduceScript.class)) { - ScriptedMetricAggContexts.ReduceScript.Factory factory = mockCompiled::createMetricAggReduceScript; + ScriptedMetricAggContexts.ReduceScript.Factory factory = new MockMetricAggReduceScriptFactory(script); return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(IntervalFilterScript.class)) { IntervalFilterScript.Factory factory = mockCompiled::createIntervalFilterScript; @@ -334,25 +334,6 @@ public SimilarityWeightScript createSimilarityWeightScript() { return new MockSimilarityWeightScript(script != null ? script : ctx -> 42d); } - public ScriptedMetricAggContexts.InitScript createMetricAggInitScript(Map params, Map state) { - return new MockMetricAggInitScript(params, state, script != null ? script : ctx -> 42d); - } - - public ScriptedMetricAggContexts.MapScript.LeafFactory createMetricAggMapScript(Map params, - Map state, - SearchLookup lookup) { - return new MockMetricAggMapScript(params, state, lookup, script != null ? script : ctx -> 42d); - } - - public ScriptedMetricAggContexts.CombineScript createMetricAggCombineScript(Map params, - Map state) { - return new MockMetricAggCombineScript(params, state, script != null ? script : ctx -> 42d); - } - - public ScriptedMetricAggContexts.ReduceScript createMetricAggReduceScript(Map params, List states) { - return new MockMetricAggReduceScript(params, states, script != null ? script : ctx -> 42d); - } - public IntervalFilterScript createIntervalFilterScript() { return new IntervalFilterScript() { @Override @@ -433,6 +414,17 @@ public double execute(Query query, Field field, Term term) { } } + public static class MockMetricAggInitScriptFactory implements ScriptedMetricAggContexts.InitScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + MockMetricAggInitScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public ScriptedMetricAggContexts.InitScript newInstance(Map params, Map state) { + return new MockMetricAggInitScript(params, state, script); + } + } + public static class MockMetricAggInitScript extends ScriptedMetricAggContexts.InitScript { private final Function, Object> script; @@ -455,6 +447,18 @@ public void execute() { } } + public static class MockMetricAggMapScriptFactory implements ScriptedMetricAggContexts.MapScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + MockMetricAggMapScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public ScriptedMetricAggContexts.MapScript.LeafFactory newFactory(Map params, Map state, + SearchLookup lookup) { + return new MockMetricAggMapScript(params, state, lookup, script); + } + } + public static class MockMetricAggMapScript implements ScriptedMetricAggContexts.MapScript.LeafFactory { private final Map params; private final Map state; @@ -491,11 +495,21 @@ public void execute() { } } + public static class MockMetricAggCombineScriptFactory implements ScriptedMetricAggContexts.CombineScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + MockMetricAggCombineScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public ScriptedMetricAggContexts.CombineScript newInstance(Map params, Map state) { + return new MockMetricAggCombineScript(params, state, script); + } + } + public static class MockMetricAggCombineScript extends ScriptedMetricAggContexts.CombineScript { private final Function, Object> script; - MockMetricAggCombineScript(Map params, Map state, - Function, Object> script) { + MockMetricAggCombineScript(Map params, Map state, Function, Object> script) { super(params, state); this.script = script; } @@ -513,11 +527,21 @@ public Object execute() { } } + public static class MockMetricAggReduceScriptFactory implements ScriptedMetricAggContexts.ReduceScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + MockMetricAggReduceScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public ScriptedMetricAggContexts.ReduceScript newInstance(Map params, List states) { + return new MockMetricAggReduceScript(params, states, script); + } + } + public static class MockMetricAggReduceScript extends ScriptedMetricAggContexts.ReduceScript { private final Function, Object> script; - MockMetricAggReduceScript(Map params, List states, - Function, Object> script) { + MockMetricAggReduceScript(Map params, List states, Function, Object> script) { super(params, states); this.script = script; } From f21c252448da81e77037bfb00b4b27261564a920 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:17:57 -0700 Subject: [PATCH 35/44] Update StatsIT --- .../search/aggregations/metrics/StatsIT.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java index 13c5fd9e3e43b..bc3695edd9f0c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsIT.java @@ -488,10 +488,10 @@ private void assertShardExecutionState(SearchResponse response, int expectedFail } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -504,10 +504,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( stats("foo").field("d").script( - new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); + new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -515,14 +515,24 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(stats("foo").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation( + stats("foo").field("d").script( + new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value + 1", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(stats("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From 7acb0feedd62c06b27f242e9ab7f7a2fccaaba8f Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:24:01 -0700 Subject: [PATCH 36/44] Update SumIT --- .../metrics/MetricAggScriptPlugin.java | 18 ++++++++++--- .../search/aggregations/metrics/SumIT.java | 27 +++++++++++++------ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java index 362aef62ab23e..206ca37f3c504 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java @@ -19,6 +19,9 @@ package org.elasticsearch.search.aggregations.metrics; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.search.lookup.LeafDocLookup; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -26,9 +29,6 @@ import java.util.function.BiFunction; import java.util.function.Function; -import org.elasticsearch.script.MockScriptPlugin; -import org.elasticsearch.search.lookup.LeafDocLookup; - /** * Provides a number of dummy scripts for tests. * @@ -52,6 +52,9 @@ public class MetricAggScriptPlugin extends MockScriptPlugin { /** Script to return the {@code _value} provided by aggs framework. */ public static final String VALUE_SCRIPT = "_value"; + /** Script to return a random double */ + public static final String RANDOM_SCRIPT = "Math.random()"; + @Override public String pluginScriptLang() { return METRIC_SCRIPT_ENGINE; @@ -88,4 +91,13 @@ protected Map, Object>> pluginScripts() { }); return scripts; } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("Math.random()", vars -> Math.random()); + + return scripts; + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java index e9bef29d445be..87883852a615a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java @@ -47,6 +47,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.METRIC_SCRIPT_ENGINE; +import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.RANDOM_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_VALUES_FIELD_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.VALUE_FIELD_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.VALUE_SCRIPT; @@ -374,10 +375,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -390,10 +391,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(sum("foo").field("d").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, Collections.emptyMap()))).get(); + new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, RANDOM_SCRIPT, Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -401,15 +402,25 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(sum("foo").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(sum("foo").field("d").script( + new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_SCRIPT, Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(sum("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } public void testFieldAlias() { From 45a9f073c39ccb74eec67a74f6bcf9b15cdd7336 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:26:49 -0700 Subject: [PATCH 37/44] Update TDigestPercentileRanksIT --- .../metrics/TDigestPercentileRanksIT.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java index 5051cee0dbf07..4974378d49010 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksIT.java @@ -484,10 +484,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -500,11 +500,11 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(percentileRanks("foo", new double[]{50.0}) .field("d") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -512,15 +512,26 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentileRanks("foo", new double[]{50.0}) + .field("d") + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From 872bed4d00e371efc05839ec6dee479a61a879fd Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:31:06 -0700 Subject: [PATCH 38/44] Update TDigestPercentilesIT --- .../metrics/TDigestPercentilesIT.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java index e334213f1d2e1..7bd871e63fab4 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java @@ -467,10 +467,10 @@ public void testOrderByEmptyAggregation() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -483,9 +483,9 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d") - .percentiles(50.0).script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .percentiles(50.0).script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "Math.random()", emptyMap()))) .get(); assertSearchResponse(r); @@ -494,14 +494,24 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d").percentiles(50.0)).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d") + .percentiles(50.0).script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentiles("foo").field("d").percentiles(50.0)).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } } From e0ed60a4d873d175b43bcdc04febe65489f27f1c Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:49:00 -0700 Subject: [PATCH 39/44] Update TopHitsIT --- .../aggregations/metrics/TopHitsIT.java | 49 ++++++++++++--- .../script/MockScriptEngine.java | 61 ++++++++++++------- 2 files changed, 79 insertions(+), 31 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java index 417328bec4e63..239f6155affee 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java @@ -107,6 +107,11 @@ public static class CustomScriptPlugin extends MockScriptPlugin { protected Map, Object>> pluginScripts() { return Collections.singletonMap("5", script -> "5"); } + + @Override + protected Map, Object>> nonDeterministicPluginScripts() { + return Collections.singletonMap("Math.random()", script -> Math.random()); + } } public static String randomExecutionHint() { @@ -1086,10 +1091,10 @@ public void testNoStoredFields() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { try { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings( @@ -1107,10 +1112,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script field does not get cached + // Test that a request using a nondeterministic script field does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(topHits("foo").scriptField("bar", - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "5", Collections.emptyMap()))).get(); + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() @@ -1118,11 +1123,12 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script sort does not get cached + // Test that a request using a nondeterministic script sort does not get cached r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(topHits("foo").sort( SortBuilders.scriptSort( - new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "5", Collections.emptyMap()), ScriptSortType.STRING))) + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "Math.random()", Collections.emptyMap()), + ScriptSortType.STRING))) .get(); assertSearchResponse(r); @@ -1131,15 +1137,38 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(topHits("foo")).get(); + // Test that a request using a deterministic script field does not get cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(topHits("foo").scriptField("bar", + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "5", Collections.emptyMap()))).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Test that a request using a deterministic script sort does not get cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(topHits("foo").sort( + SortBuilders.scriptSort( + new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "5", Collections.emptyMap()), ScriptSortType.STRING))) + .get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(topHits("foo")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(3L)); } finally { assertAcked(client().admin().indices().prepareDelete("cache_test_idx")); // delete this - if we use tests.iters it would fail } diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 846f4f829eec1..9113897304f2e 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -103,16 +103,7 @@ public T compile(String name, String source, ScriptCon } MockCompiledScript mockCompiled = new MockCompiledScript(name, params, source, script); if (context.instanceClazz.equals(FieldScript.class)) { - FieldScript.Factory factory = (parameters, lookup) -> - ctx -> new FieldScript(parameters, lookup, ctx) { - @Override - public Object execute() { - Map vars = createVars(parameters); - vars.putAll(getLeafLookup().asMap()); - return script.apply(vars); - } - }; - return context.factoryClazz.cast(factory); + return context.factoryClazz.cast(new MockFieldScriptFactory(script)); } else if(context.instanceClazz.equals(TermsSetQueryScript.class)) { TermsSetQueryScript.Factory factory = (parameters, lookup) -> (TermsSetQueryScript.LeafFactory) ctx -> new TermsSetQueryScript(parameters, lookup, ctx) { @@ -147,17 +138,7 @@ public boolean needs_score() { }; return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(StringSortScript.class)) { - StringSortScript.Factory factory = (parameters, lookup) -> (StringSortScript.LeafFactory) ctx - -> new StringSortScript(parameters, lookup, ctx) { - @Override - public String execute() { - Map vars = new HashMap<>(parameters); - vars.put("params", parameters); - vars.put("doc", getDoc()); - return String.valueOf(script.apply(vars)); - } - }; - return context.factoryClazz.cast(factory); + return context.factoryClazz.cast(new MockStringSortScriptFactory(script)); } else if (context.instanceClazz.equals(IngestScript.class)) { IngestScript.Factory factory = vars -> new IngestScript(vars) { @Override @@ -649,4 +630,42 @@ public double execute(Map vars) { }; } } + + class MockFieldScriptFactory implements FieldScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + public MockFieldScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public FieldScript.LeafFactory newFactory(Map parameters, SearchLookup lookup) { + return ctx -> new FieldScript(parameters, lookup, ctx) { + @Override + public Object execute() { + Map vars = createVars(parameters); + vars.putAll(getLeafLookup().asMap()); + return script.apply(vars); + + } + }; + } + } + + class MockStringSortScriptFactory implements StringSortScript.Factory, ScriptFactory { + private final MockDeterministicScript script; + public MockStringSortScriptFactory(MockDeterministicScript script) { this.script = script; } + @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } + + @Override + public StringSortScript.LeafFactory newFactory(Map parameters, SearchLookup lookup) { + return ctx -> new StringSortScript(parameters, lookup, ctx) { + @Override + public String execute() { + Map vars = new HashMap<>(parameters); + vars.put("params", parameters); + vars.put("doc", getDoc()); + return String.valueOf(script.apply(vars)); + } + }; + } + } } From 40292e0c9c5625186d4b1cc3983b8d05c684a46e Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 12:52:01 -0700 Subject: [PATCH 40/44] Update ValueCountIT --- .../aggregations/metrics/ValueCountIT.java | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java index 2976d1310bcf9..642326010a2f8 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java @@ -43,6 +43,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.METRIC_SCRIPT_ENGINE; +import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.RANDOM_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_FIELD_PARAMS_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_VALUES_FIELD_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.VALUE_FIELD_SCRIPT; @@ -211,10 +212,10 @@ public void testMultiValuedScriptWithParams() throws Exception { } /** - * Make sure that a request using a script does not get cached and a request - * not using a script does get cached. + * Make sure that a request using a deterministic script or not using a script get cached. + * Ensure requests using nondeterministic scripts do not get cached. */ - public void testDontCacheScripts() throws Exception { + public void testScriptCaching() throws Exception { assertAcked(prepareCreate("cache_test_idx").addMapping("type", "d", "type=long") .setSettings(Settings.builder().put("requests.cache.enable", true).put("number_of_shards", 1).put("number_of_replicas", 1)) .get()); @@ -227,10 +228,10 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // Test that a request using a script does not get cached + // Test that a request using a nondeterministic script does not get cached SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) .addAggregation(count("foo").field("d").script( - new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_FIELD_SCRIPT, Collections.emptyMap()))) + new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, RANDOM_SCRIPT, Collections.emptyMap()))) .get(); assertSearchResponse(r); @@ -239,15 +240,26 @@ public void testDontCacheScripts() throws Exception { assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(0L)); - // To make sure that the cache is working test that a request not using - // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(count("foo").field("d")).get(); + // Test that a request using a deterministic script gets cached + r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(count("foo").field("d").script( + new Script(ScriptType.INLINE, METRIC_SCRIPT_ENGINE, VALUE_FIELD_SCRIPT, Collections.emptyMap()))) + .get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getHitCount(), equalTo(0L)); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() .getMissCount(), equalTo(1L)); + + // Ensure that non-scripted requests are cached as normal + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(count("foo").field("d")).get(); + assertSearchResponse(r); + + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getHitCount(), equalTo(0L)); + assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() + .getMissCount(), equalTo(2L)); } public void testOrderByEmptyAggregation() throws Exception { From 04e5530bb2e8bf44a0d3d4ef556d82a9eb70c520 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 13:12:04 -0700 Subject: [PATCH 41/44] Update MinAggregatorTests --- .../metrics/MinAggregatorTests.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java index 922536a76544a..a0f017bc6a057 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java @@ -127,6 +127,8 @@ public class MinAggregatorTests extends AggregatorTestCase { private static final String INVERT_SCRIPT = "invert"; + private static final String RANDOM_SCRIPT = "random"; + @Override protected ScriptService getMockScriptService() { Map, Object>> scripts = new HashMap<>(); @@ -161,8 +163,12 @@ protected ScriptService getMockScriptService() { }); scripts.put(INVERT_SCRIPT, vars -> -((Number) vars.get("_value")).doubleValue()); + Map, Object>> nonDeterministicScripts = new HashMap<>(); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, + nonDeterministicScripts, Collections.emptyMap()); Map engines = Collections.singletonMap(scriptEngine.getType(), scriptEngine); @@ -649,7 +655,7 @@ public void testCaching() throws IOException { } } - public void testNoCachingWithScript() throws IOException { + public void testScriptCaching() throws IOException { MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER); fieldType.setName("number"); @@ -657,6 +663,10 @@ public void testNoCachingWithScript() throws IOException { .field("number") .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, INVERT_SCRIPT, Collections.emptyMap()));; + MinAggregationBuilder nonDeterministicAggregationBuilder = new MinAggregationBuilder("min") + .field("number") + .script(new Script(ScriptType.INLINE, MockScriptEngine.NAME, RANDOM_SCRIPT, Collections.emptyMap()));; + try (Directory directory = newDirectory()) { RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); indexWriter.addDocument(singleton(new NumericDocValuesField("number", 7))); @@ -668,11 +678,19 @@ public void testNoCachingWithScript() throws IOException { try (IndexReader indexReader = DirectoryReader.open(directory)) { IndexSearcher indexSearcher = newSearcher(indexReader, true, true); - InternalMin min = searchAndReduce(indexSearcher, new MatchAllDocsQuery(), aggregationBuilder, fieldType); - assertEquals(-7.0, min.getValue(), 0); + InternalMin min = searchAndReduce(indexSearcher, new MatchAllDocsQuery(), nonDeterministicAggregationBuilder, fieldType); + assertTrue(min.getValue() >= 0.0 && min.getValue() <= 1.0); assertTrue(AggregationInspectionHelper.hasValue(min)); assertFalse(queryShardContext.isCacheable()); + + indexSearcher = newSearcher(indexReader, true, true); + + min = searchAndReduce(indexSearcher, new MatchAllDocsQuery(), aggregationBuilder, fieldType); + assertEquals(-7.0, min.getValue(), 0); + assertTrue(AggregationInspectionHelper.hasValue(min)); + + assertTrue(queryShardContext.isCacheable()); } } } From a1a572e89d77087584383f8b1a52b01d12794596 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 13:40:54 -0700 Subject: [PATCH 42/44] Use reproducable random methods instead of Math.random --- .../search/aggregations/AggregationTestScriptsPlugin.java | 3 ++- .../search/aggregations/bucket/DoubleTermsIT.java | 2 +- .../search/aggregations/bucket/HistogramIT.java | 2 +- .../search/aggregations/bucket/LongTermsIT.java | 2 +- .../elasticsearch/search/aggregations/bucket/RangeIT.java | 2 +- .../bucket/SignificantTermsSignificanceScoreIT.java | 2 +- .../search/aggregations/bucket/terms/StringTermsIT.java | 2 +- .../search/aggregations/metrics/AvgAggregatorTests.java | 2 +- .../search/aggregations/metrics/CardinalityIT.java | 2 +- .../search/aggregations/metrics/MaxAggregatorTests.java | 2 +- .../search/aggregations/metrics/MetricAggScriptPlugin.java | 3 ++- .../search/aggregations/metrics/MinAggregatorTests.java | 2 +- .../search/aggregations/metrics/ScriptedMetricIT.java | 6 +++--- .../search/aggregations/metrics/TopHitsIT.java | 2 +- 14 files changed, 18 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java index 738f04336cb55..873dd89bdcfd1 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationTestScriptsPlugin.java @@ -23,6 +23,7 @@ import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.test.ESTestCase; import java.util.HashMap; import java.util.Map; @@ -121,7 +122,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> ESTestCase.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java index 94b9dc73841d6..c3a8c9b18d390 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java @@ -117,7 +117,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> DoubleTermsIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java index c071fbc2a7b10..f8c5390de54cd 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramIT.java @@ -115,7 +115,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> HistogramIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java index 2903d19ac7cf5..2d5d7d33aefdf 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java @@ -104,7 +104,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> LongTermsIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java index 05f25aa1dbda7..32e8516910ea8 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/RangeIT.java @@ -97,7 +97,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> RangeIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java index 65c4d583e3d9a..80b67fe850d8d 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java @@ -197,7 +197,7 @@ public Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> SignificantTermsSignificanceScoreIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java index 22ca5af717124..dda55c9e8f9df 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java @@ -124,7 +124,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> StringTermsIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java index 21925247865ee..46f08d786a856 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java @@ -119,7 +119,7 @@ protected ScriptService getMockScriptService() { }); Map, Object>> nonDeterministicScripts = new HashMap<>(); - nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> AvgAggregatorTests.randomDouble()); MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java index 921f04232307f..b290d44a3e85d 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityIT.java @@ -95,7 +95,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> CardinalityIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index d70f19f042ebc..598d19795db20 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -147,7 +147,7 @@ protected ScriptService getMockScriptService() { }); Map, Object>> nonDeterministicScripts = new HashMap<>(); - nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> MaxAggregatorTests.randomDouble()); MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java index 206ca37f3c504..9ad5038e21cb8 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java @@ -21,6 +21,7 @@ import org.elasticsearch.script.MockScriptPlugin; import org.elasticsearch.search.lookup.LeafDocLookup; +import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; import java.util.HashMap; @@ -96,7 +97,7 @@ protected Map, Object>> pluginScripts() { protected Map, Object>> nonDeterministicPluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("Math.random()", vars -> Math.random()); + scripts.put("Math.random()", vars -> ESTestCase.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java index a0f017bc6a057..d6b65dfc62d74 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java @@ -164,7 +164,7 @@ protected ScriptService getMockScriptService() { scripts.put(INVERT_SCRIPT, vars -> -((Number) vars.get("_value")).doubleValue()); Map, Object>> nonDeterministicScripts = new HashMap<>(); - nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> Math.random()); + nonDeterministicScripts.put(RANDOM_SCRIPT, vars -> AggregatorTestCase.randomDouble()); MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, scripts, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index dc93e85c7f9f6..406a96e3c34cd 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -252,14 +252,14 @@ protected Map, Object>> nonDeterministicPlu Map, Object>> scripts = new HashMap<>(); scripts.put("state.data = Math.random()", vars -> - aggScript(vars, state -> state.put("data", Math.random()))); + aggScript(vars, state -> state.put("data", ScriptedMetricIT.randomDouble()))); scripts.put("state['count'] = Math.random() >= 0.5 ? 1 : 0", vars -> - aggScript(vars, state -> state.put("count", Math.random() >= 0.5 ? 1 : 0))); + aggScript(vars, state -> state.put("count", ScriptedMetricIT.randomDouble() >= 0.5 ? 1 : 0))); - scripts.put("return Math.random()", vars -> Math.random()); + scripts.put("return Math.random()", vars -> ScriptedMetricIT.randomDouble()); return scripts; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java index 239f6155affee..207060d05ce72 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java @@ -110,7 +110,7 @@ protected Map, Object>> pluginScripts() { @Override protected Map, Object>> nonDeterministicPluginScripts() { - return Collections.singletonMap("Math.random()", script -> Math.random()); + return Collections.singletonMap("Math.random()", script -> TopHitsIT.randomDouble()); } } From e7c1171e35d9b2ef68bf78c9634fa879e2b856c8 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 13:54:41 -0700 Subject: [PATCH 43/44] Remove unnecessary publics, rearrange imports, correct indent --- .../painless/lookup/PainlessLookupBuilder.java | 2 +- .../aggregations/metrics/MetricAggScriptPlugin.java | 8 ++++---- .../java/org/elasticsearch/script/MockScriptEngine.java | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java index 059382a414006..2e687b481bbc5 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java @@ -910,7 +910,7 @@ public void addPainlessClassBinding(ClassLoader classLoader, String targetJavaCl } public void addPainlessClassBinding(Class targetClass, String methodName, Class returnType, List> typeParameters, - Map, Object> annotations) { + Map, Object> annotations) { Objects.requireNonNull(targetClass); Objects.requireNonNull(methodName); Objects.requireNonNull(returnType); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java index 9ad5038e21cb8..6787386862f9a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MetricAggScriptPlugin.java @@ -19,10 +19,6 @@ package org.elasticsearch.search.aggregations.metrics; -import org.elasticsearch.script.MockScriptPlugin; -import org.elasticsearch.search.lookup.LeafDocLookup; -import org.elasticsearch.test.ESTestCase; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -30,6 +26,10 @@ import java.util.function.BiFunction; import java.util.function.Function; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.search.lookup.LeafDocLookup; +import org.elasticsearch.test.ESTestCase; + /** * Provides a number of dummy scripts for tests. * diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 9113897304f2e..3069efbebb451 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -586,7 +586,7 @@ public void setScorer(Scorable scorer) { class MockAggregationScript implements AggregationScript.Factory, ScriptFactory { private final MockDeterministicScript script; - public MockAggregationScript(MockDeterministicScript script) { this.script = script; } + MockAggregationScript(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @Override @@ -617,7 +617,7 @@ public boolean needs_score() { class MockSignificantTermsHeuristicScoreScript implements SignificantTermsHeuristicScoreScript.Factory, ScriptFactory { private final MockDeterministicScript script; - public MockSignificantTermsHeuristicScoreScript(MockDeterministicScript script) { this.script = script; } + MockSignificantTermsHeuristicScoreScript(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @Override @@ -633,7 +633,7 @@ public double execute(Map vars) { class MockFieldScriptFactory implements FieldScript.Factory, ScriptFactory { private final MockDeterministicScript script; - public MockFieldScriptFactory(MockDeterministicScript script) { this.script = script; } + MockFieldScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @Override @@ -652,7 +652,7 @@ public Object execute() { class MockStringSortScriptFactory implements StringSortScript.Factory, ScriptFactory { private final MockDeterministicScript script; - public MockStringSortScriptFactory(MockDeterministicScript script) { this.script = script; } + MockStringSortScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @Override From b808b39d375c9eb589d8be84fa1d09533d61615e Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 17 Dec 2019 14:38:16 -0700 Subject: [PATCH 44/44] Mark Calendar and Clock methods nondeterministic --- .../org/elasticsearch/painless/spi/java.time.txt | 10 +++++----- .../org/elasticsearch/painless/spi/java.util.txt | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.time.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.time.txt index 0cedc849a6838..38c6e8a4f575e 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.time.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.time.txt @@ -26,11 +26,11 @@ class java.time.Clock { Clock fixed(Instant,ZoneId) - ZoneId getZone() - Instant instant() - long millis() - Clock offset(Clock,Duration) - Clock tick(Clock,Duration) + ZoneId getZone() @nondeterministic + Instant instant() @nondeterministic + long millis() @nondeterministic + Clock offset(Clock,Duration) @nondeterministic + Clock tick(Clock,Duration) @nondeterministic } class java.time.Duration { diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt index 39b52ee104c75..12da1f679422c 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/java.util.txt @@ -490,9 +490,9 @@ class java.util.Calendar { Map getDisplayNames(int,int,Locale) int getFirstDayOfWeek() int getGreatestMinimum(int) - Calendar getInstance() - Calendar getInstance(TimeZone) - Calendar getInstance(TimeZone,Locale) + Calendar getInstance() @nondeterministic + Calendar getInstance(TimeZone) @nondeterministic + Calendar getInstance(TimeZone,Locale) @nondeterministic int getLeastMaximum(int) int getMaximum(int) int getMinimalDaysInFirstWeek()