From ca9f0a58fc04e691fc8ef3b8a38079effe2f9b57 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 29 May 2018 15:36:25 -0400 Subject: [PATCH 1/4] Handle missing values in painless Throw an exception for `doc['field'].value` if this document is missing a value for the `field`. For 7.0: This is the default behaviour from 7.0 For 6.x: To enable this behavior from 6.x, a user can set a jvm.option: `-Des.script.exception_for_missing_value=true` on a node. If a user does not enable this behavior, a deprecation warning is logged on start up. Closes #29286 --- .../painless-getting-started.asciidoc | 21 +++++++++ modules/lang-painless/build.gradle | 11 ++++- .../test/painless/20_scriptfield.yml | 3 +- modules/mapper-extras/build.gradle | 3 ++ modules/parent-join/build.gradle | 3 ++ modules/percolator/build.gradle | 4 ++ server/build.gradle | 4 ++ .../index/fielddata/ScriptDocValues.java | 44 +++++++++++++++++-- .../elasticsearch/script/ScriptModule.java | 12 +++++ .../fielddata/ScriptDocValuesDatesTests.java | 8 +++- .../fielddata/ScriptDocValuesLongsTests.java | 23 +++------- test/framework/build.gradle | 1 + x-pack/plugin/security/build.gradle | 1 + 13 files changed, 115 insertions(+), 23 deletions(-) diff --git a/docs/painless/painless-getting-started.asciidoc b/docs/painless/painless-getting-started.asciidoc index 2cf91666ba48d..90c6664d8a1fc 100644 --- a/docs/painless/painless-getting-started.asciidoc +++ b/docs/painless/painless-getting-started.asciidoc @@ -119,6 +119,27 @@ GET hockey/_search ---------------------------------------------------------------- // CONSOLE + +[float] +===== Missing values + +If you request the value from a field `field` that isn’t in +the document, `doc['field'].value` for this document returns: + +- `0` if a `field` has a numeric datatype (long, double etc.) +- `false` is a `field` has a boolean datatype +- epoch date if a `field` has a date datatype +- `null` if a `field` has a string datatype +- `null` if a `field` has a geo datatype +- `""` if a `field` has a binary datatype + +IMPORTANT: Starting in 7.0, `doc['field'].value` throws an exception if +the field is missing in a document. To enable this behavior now, +set a <> +`-Des.script.exception_for_missing_value=true` on a node. If you do not enable +this behavior, a deprecation warning is logged on start up. + + [float] ==== Updating Fields with Painless diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle index d287d7ee02378..4ccb877113485 100644 --- a/modules/lang-painless/build.gradle +++ b/modules/lang-painless/build.gradle @@ -17,7 +17,7 @@ * under the License. */ -import org.apache.tools.ant.types.Path +import org.elasticsearch.gradle.test.RestIntegTestTask esplugin { description 'An easy, safe and fast scripting language for Elasticsearch' @@ -28,6 +28,15 @@ integTestCluster { module project.project(':modules:mapper-extras') } +Task additionalClusterTest = tasks.create( + name: "additionalClusterTest", type: RestIntegTestTask) +additionalClusterTestCluster { + systemProperty 'es.script.exception_for_missing_value', 'true' + distribution = 'integ-test-zip' + module project // add the lang-painless module itself + module project.project(':modules:mapper-extras') +} + dependencies { compile 'org.antlr:antlr4-runtime:4.5.3' compile 'org.ow2.asm:asm-debug-all:5.1' diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml index 02c17ce0e3714..53c2fc024d5ef 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml @@ -88,14 +88,13 @@ setup: --- "Scripted Field with a null safe dereference (null)": - # Change this to ?: once we have it implemented - do: search: body: script_fields: bar: script: - source: "(doc['missing'].value?.length() ?: 0) + params.x;" + source: "(doc['missing'].size() == 0 ? 0 : doc['missing'].value?.length()) + params.x;" params: x: 5 diff --git a/modules/mapper-extras/build.gradle b/modules/mapper-extras/build.gradle index 7831de3a68e94..01764fcf09deb 100644 --- a/modules/mapper-extras/build.gradle +++ b/modules/mapper-extras/build.gradle @@ -21,3 +21,6 @@ esplugin { description 'Adds advanced field mappers' classname 'org.elasticsearch.index.mapper.MapperExtrasPlugin' } +test.configure { + systemProperty 'es.script.exception_for_missing_value', 'true' +} diff --git a/modules/parent-join/build.gradle b/modules/parent-join/build.gradle index 67bcc9d54e8e7..dd736d79b56ed 100644 --- a/modules/parent-join/build.gradle +++ b/modules/parent-join/build.gradle @@ -22,3 +22,6 @@ esplugin { classname 'org.elasticsearch.join.ParentJoinPlugin' hasClientJar = true } +test.configure { + systemProperty 'es.script.exception_for_missing_value', 'true' +} \ No newline at end of file diff --git a/modules/percolator/build.gradle b/modules/percolator/build.gradle index db4a716af6513..2bed596fee8d7 100644 --- a/modules/percolator/build.gradle +++ b/modules/percolator/build.gradle @@ -36,3 +36,7 @@ dependencyLicenses { compileJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" + +test.configure { + systemProperty 'es.script.exception_for_missing_value', 'true' +} \ No newline at end of file diff --git a/server/build.gradle b/server/build.gradle index 7e880e0dae4d2..bdfd6a7ed137f 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -338,3 +338,7 @@ if (isEclipse == false || project.path == ":server-tests") { check.dependsOn integTest integTest.mustRunAfter test } + +test.configure { + systemProperty 'es.script.exception_for_missing_value', 'true' +} \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index dd85e921e4ac5..47cfc465cb189 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.ESLoggerFactory; +import org.elasticsearch.script.ScriptModule; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.MutableDateTime; @@ -128,6 +129,10 @@ protected void resize(int newSize) { public long getValue() { if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } return 0L; } return values[0]; @@ -170,6 +175,10 @@ public Dates(SortedNumericDocValues in) { */ public ReadableDateTime getValue() { if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } return EPOCH; } return get(0); @@ -271,6 +280,10 @@ public SortedNumericDoubleValues getInternalValues() { public double getValue() { if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } return 0d; } return values[0]; @@ -327,6 +340,10 @@ protected void resize(int newSize) { public GeoPoint getValue() { if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } return null; } return values[0]; @@ -439,7 +456,14 @@ protected void resize(int newSize) { } public boolean getValue() { - return count != 0 && values[0]; + if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } + return false; + } + return values[0]; } @Override @@ -522,7 +546,14 @@ public String get(int index) { } public String getValue() { - return count == 0 ? null : get(0); + if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } + return null; + } + return get(0); } } @@ -543,7 +574,14 @@ public BytesRef get(int index) { } public BytesRef getValue() { - return count == 0 ? new BytesRef() : get(0); + if (count == 0) { + if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) { + throw new IllegalStateException("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!"); + } + return new BytesRef(); + } + return get(0); } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptModule.java b/server/src/main/java/org/elasticsearch/script/ScriptModule.java index 7074d3ad9fe44..b85ad494f47f5 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptModule.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptModule.java @@ -31,6 +31,9 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.search.aggregations.pipeline.movfn.MovingFunctionScript; +import org.elasticsearch.common.Booleans; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; /** * Manages building {@link ScriptService}. @@ -57,6 +60,11 @@ public class ScriptModule { ).collect(Collectors.toMap(c -> c.name, Function.identity())); } + public static final boolean EXCEPTION_FOR_MISSING_VALUE = + Booleans.parseBoolean(System.getProperty("es.script.exception_for_missing_value", "false")); + + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ScriptModule.class)); + private final ScriptService scriptService; public ScriptModule(Settings settings, List scriptPlugins) { @@ -80,6 +88,10 @@ public ScriptModule(Settings settings, List scriptPlugins) { } } } + if (EXCEPTION_FOR_MISSING_VALUE == false) + DEPRECATION_LOGGER.deprecated("Script: returning default values for missing document values is deprecated. " + + "Set system property '-Des.script.exception_for_missing_value=true' " + + "to make behaviour compatible with future major versions."); scriptService = new ScriptService(settings, Collections.unmodifiableMap(engines), Collections.unmodifiableMap(contexts)); } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java index 12eb69bef39d8..0f6bb7aa50ad9 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesDatesTests.java @@ -54,7 +54,13 @@ public void test() throws IOException { for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); dates.setNextDocId(d); - assertEquals(expectedDates[d].length > 0 ? expectedDates[d][0] : new DateTime(0, DateTimeZone.UTC), dates.getValue()); + if (expectedDates[d].length > 0) { + assertEquals(expectedDates[d][0] , dates.getValue()); + } else { + Exception e = expectThrows(IllegalStateException.class, () -> dates.getValue()); + assertEquals("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!", e.getMessage()); + } assertEquals(values[d].length, dates.size()); for (int i = 0; i < values[d].length; i++) { diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java index 5fd33da27e399..c22cb4919677a 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesLongsTests.java @@ -21,22 +21,8 @@ import org.elasticsearch.index.fielddata.ScriptDocValues.Longs; import org.elasticsearch.test.ESTestCase; -import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; -import org.joda.time.ReadableDateTime; import java.io.IOException; -import java.security.AccessControlContext; -import java.security.AccessController; -import java.security.PermissionCollection; -import java.security.Permissions; -import java.security.PrivilegedAction; -import java.security.ProtectionDomain; -import java.util.HashSet; -import java.util.Set; -import java.util.function.Consumer; - -import static org.hamcrest.Matchers.containsInAnyOrder; public class ScriptDocValuesLongsTests extends ESTestCase { public void testLongs() throws IOException { @@ -52,8 +38,13 @@ public void testLongs() throws IOException { for (int round = 0; round < 10; round++) { int d = between(0, values.length - 1); longs.setNextDocId(d); - assertEquals(values[d].length > 0 ? values[d][0] : 0, longs.getValue()); - + if (values[d].length > 0) { + assertEquals(values[d][0], longs.getValue()); + } else { + Exception e = expectThrows(IllegalStateException.class, () -> longs.getValue()); + assertEquals("A document doesn't have a value for a field! " + + "Use doc[].size()==0 to check if a document is missing a field!", e.getMessage()); + } assertEquals(values[d].length, longs.size()); assertEquals(values[d].length, longs.getValues().size()); for (int i = 0; i < values[d].length; i++) { diff --git a/test/framework/build.gradle b/test/framework/build.gradle index 193fcb30988c6..d1bfdb941a621 100644 --- a/test/framework/build.gradle +++ b/test/framework/build.gradle @@ -73,4 +73,5 @@ precommit.dependsOn namingConventionsMain test.configure { systemProperty 'tests.gradle_index_compat_versions', bwcVersions.indexCompatible.join(',') systemProperty 'tests.gradle_wire_compat_versions', bwcVersions.wireCompatible.join(',') + systemProperty 'es.script.exception_for_missing_value', 'true' } diff --git a/x-pack/plugin/security/build.gradle b/x-pack/plugin/security/build.gradle index 12533a389b5f1..47597bd302fa6 100644 --- a/x-pack/plugin/security/build.gradle +++ b/x-pack/plugin/security/build.gradle @@ -225,6 +225,7 @@ test { * other if we allow them to set the number of available processors as it's set-once in Netty. */ systemProperty 'es.set.netty.runtime.available.processors', 'false' + systemProperty 'es.script.exception_for_missing_value', 'true' } // xpack modules are installed in real clusters as the meta plugin, so From 58beb2a00fe0ec2646d1c1aa61ef29c445b0a6ad Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Thu, 28 Jun 2018 15:22:04 -0400 Subject: [PATCH 2/4] Addressing feedback - add additional tests --- .../elasticsearch/gradle/BuildPlugin.groovy | 1 + modules/lang-painless/build.gradle | 11 +- .../test/painless/20_scriptfield.yml | 2 +- modules/mapper-extras/build.gradle | 5 +- modules/parent-join/build.gradle | 3 - modules/percolator/build.gradle | 6 +- server/build.gradle | 12 +- ...criptDocValuesMissingV6BehaviourTests.java | 195 ++++++++++++++++++ test/framework/build.gradle | 1 - x-pack/plugin/security/build.gradle | 1 - 10 files changed, 209 insertions(+), 28 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index eb3cd1dc8c6da..c7d987a5799d2 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -673,6 +673,7 @@ class BuildPlugin implements Plugin { systemProperty 'tests.task', path systemProperty 'tests.security.manager', 'true' systemProperty 'jna.nosys', 'true' + systemProperty 'es.script.exception_for_missing_value', 'true' // TODO: remove setting logging level via system property systemProperty 'tests.logger.level', 'WARN' for (Map.Entry property : System.properties.entrySet()) { diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle index 4ccb877113485..fb1ea441a9dd5 100644 --- a/modules/lang-painless/build.gradle +++ b/modules/lang-painless/build.gradle @@ -17,7 +17,7 @@ * under the License. */ -import org.elasticsearch.gradle.test.RestIntegTestTask + esplugin { description 'An easy, safe and fast scripting language for Elasticsearch' @@ -28,15 +28,6 @@ integTestCluster { module project.project(':modules:mapper-extras') } -Task additionalClusterTest = tasks.create( - name: "additionalClusterTest", type: RestIntegTestTask) -additionalClusterTestCluster { - systemProperty 'es.script.exception_for_missing_value', 'true' - distribution = 'integ-test-zip' - module project // add the lang-painless module itself - module project.project(':modules:mapper-extras') -} - dependencies { compile 'org.antlr:antlr4-runtime:4.5.3' compile 'org.ow2.asm:asm-debug-all:5.1' diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml index 53c2fc024d5ef..2914e8a916ec6 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/20_scriptfield.yml @@ -94,7 +94,7 @@ setup: script_fields: bar: script: - source: "(doc['missing'].size() == 0 ? 0 : doc['missing'].value?.length()) + params.x;" + source: "(doc['missing'].size() == 0 ? 0 : doc['missing'].value.length()) + params.x;" params: x: 5 diff --git a/modules/mapper-extras/build.gradle b/modules/mapper-extras/build.gradle index 01764fcf09deb..31d5483a30571 100644 --- a/modules/mapper-extras/build.gradle +++ b/modules/mapper-extras/build.gradle @@ -20,7 +20,4 @@ esplugin { description 'Adds advanced field mappers' classname 'org.elasticsearch.index.mapper.MapperExtrasPlugin' -} -test.configure { - systemProperty 'es.script.exception_for_missing_value', 'true' -} +} \ No newline at end of file diff --git a/modules/parent-join/build.gradle b/modules/parent-join/build.gradle index dd736d79b56ed..1b7a5ac6ae9cb 100644 --- a/modules/parent-join/build.gradle +++ b/modules/parent-join/build.gradle @@ -21,7 +21,4 @@ esplugin { description 'This module adds the support parent-child queries and aggregations' classname 'org.elasticsearch.join.ParentJoinPlugin' hasClientJar = true -} -test.configure { - systemProperty 'es.script.exception_for_missing_value', 'true' } \ No newline at end of file diff --git a/modules/percolator/build.gradle b/modules/percolator/build.gradle index 2bed596fee8d7..e958548f89081 100644 --- a/modules/percolator/build.gradle +++ b/modules/percolator/build.gradle @@ -35,8 +35,4 @@ dependencyLicenses { } compileJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" -compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" - -test.configure { - systemProperty 'es.script.exception_for_missing_value', 'true' -} \ No newline at end of file +compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" \ No newline at end of file diff --git a/server/build.gradle b/server/build.gradle index bdfd6a7ed137f..e857a7480d321 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -339,6 +339,12 @@ if (isEclipse == false || project.path == ":server-tests") { integTest.mustRunAfter test } -test.configure { - systemProperty 'es.script.exception_for_missing_value', 'true' -} \ No newline at end of file +// TODO: remove ScriptDocValuesMissingV6BehaviourTests in 7.0 +additionalTest('testScriptDocValuesMissingV6Behaviour'){ + include '**/ScriptDocValuesMissingV6BehaviourTests.class' + systemProperty 'es.script.exception_for_missing_value', 'false' +} +test { + // these are tested explicitly in separate test tasks + exclude '**/*ScriptDocValuesMissingV6BehaviourTests.class' +} diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java new file mode 100644 index 0000000000000..7df58dbe3fdfd --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java @@ -0,0 +1,195 @@ +/* + * 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.index.fielddata; + +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.fielddata.ScriptDocValues.Longs; +import org.elasticsearch.index.fielddata.ScriptDocValues.Dates; +import org.elasticsearch.index.fielddata.ScriptDocValues.Booleans; +import org.elasticsearch.plugins.ScriptPlugin; +import org.elasticsearch.script.MockScriptEngine; +import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.script.ScriptModule; +import org.elasticsearch.test.ESTestCase; + +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.ReadableDateTime; +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +import static java.util.Collections.singletonList; + +public class ScriptDocValuesMissingV6BehaviourTests extends ESTestCase { + + public void testScriptMissingValuesWarning(){ + new ScriptModule(Settings.EMPTY, singletonList(new ScriptPlugin() { + @Override + public ScriptEngine getScriptEngine(Settings settings, Collection> contexts) { + return new MockScriptEngine(MockScriptEngine.NAME, Collections.singletonMap("1", script -> "1")); + } + })); + assertWarnings("Script: returning default values for missing document values is deprecated. " + + "Set system property '-Des.script.exception_for_missing_value=true' " + + "to make behaviour compatible with future major versions."); + } + + public void testZeroForMissingValueLong() throws IOException { + long[][] values = new long[between(3, 10)][]; + for (int d = 0; d < values.length; d++) { + values[d] = new long[0]; + } + Longs longs = wrap(values); + for (int round = 0; round < 10; round++) { + int d = between(0, values.length - 1); + longs.setNextDocId(d); + assertEquals(0, longs.getValue()); + } + } + + public void testEpochForMissingValueDate() throws IOException { + final ReadableDateTime EPOCH = new DateTime(0, DateTimeZone.UTC); + long[][] values = new long[between(3, 10)][]; + for (int d = 0; d < values.length; d++) { + values[d] = new long[0]; + } + Dates dates = wrapDates(values); + for (int round = 0; round < 10; round++) { + int d = between(0, values.length - 1); + dates.setNextDocId(d); + assertEquals(EPOCH, dates.getValue()); + } + } + + public void testFalseForMissingValueBoolean() throws IOException { + long[][] values = new long[between(3, 10)][]; + for (int d = 0; d < values.length; d++) { + values[d] = new long[0]; + } + Booleans bools = wrapBooleans(values); + for (int round = 0; round < 10; round++) { + int d = between(0, values.length - 1); + bools.setNextDocId(d); + assertEquals(false, bools.getValue()); + } + } + + public void testNullForMissingValueGeo() throws IOException{ + final MultiGeoPointValues values = wrap(new GeoPoint[0]); + final ScriptDocValues.GeoPoints script = new ScriptDocValues.GeoPoints(values); + script.setNextDocId(0); + assertEquals(null, script.getValue()); + } + + + private Longs wrap(long[][] values) { + return new Longs(new AbstractSortedNumericDocValues() { + long[] current; + int i; + @Override + public boolean advanceExact(int doc) { + i = 0; + current = values[doc]; + return current.length > 0; + } + @Override + public int docValueCount() { + return current.length; + } + @Override + public long nextValue() { + return current[i++]; + } + }); + } + + private Booleans wrapBooleans(long[][] values) { + return new Booleans(new AbstractSortedNumericDocValues() { + long[] current; + int i; + @Override + public boolean advanceExact(int doc) { + i = 0; + current = values[doc]; + return current.length > 0; + } + @Override + public int docValueCount() { + return current.length; + } + @Override + public long nextValue() { + return current[i++]; + } + }); + } + + private Dates wrapDates(long[][] values) { + return new Dates(new AbstractSortedNumericDocValues() { + long[] current; + int i; + @Override + public boolean advanceExact(int doc) { + current = values[doc]; + i = 0; + return current.length > 0; + } + @Override + public int docValueCount() { + return current.length; + } + @Override + public long nextValue() { + return current[i++]; + } + }); + } + + + private static MultiGeoPointValues wrap(final GeoPoint... points) { + return new MultiGeoPointValues() { + int docID = -1; + int i; + @Override + public GeoPoint nextValue() { + if (docID != 0) { + fail(); + } + return points[i++]; + } + @Override + public boolean advanceExact(int docId) { + docID = docId; + return points.length > 0; + } + @Override + public int docValueCount() { + if (docID != 0) { + return 0; + } + return points.length; + } + }; + } + +} diff --git a/test/framework/build.gradle b/test/framework/build.gradle index afd938dd708a0..5f1bc524da599 100644 --- a/test/framework/build.gradle +++ b/test/framework/build.gradle @@ -74,5 +74,4 @@ precommit.dependsOn namingConventionsMain test.configure { systemProperty 'tests.gradle_index_compat_versions', bwcVersions.indexCompatible.join(',') systemProperty 'tests.gradle_wire_compat_versions', bwcVersions.wireCompatible.join(',') - systemProperty 'es.script.exception_for_missing_value', 'true' } diff --git a/x-pack/plugin/security/build.gradle b/x-pack/plugin/security/build.gradle index 47597bd302fa6..12533a389b5f1 100644 --- a/x-pack/plugin/security/build.gradle +++ b/x-pack/plugin/security/build.gradle @@ -225,7 +225,6 @@ test { * other if we allow them to set the number of available processors as it's set-once in Netty. */ systemProperty 'es.set.netty.runtime.available.processors', 'false' - systemProperty 'es.script.exception_for_missing_value', 'true' } // xpack modules are installed in real clusters as the meta plugin, so From 25c0a570755df6223c16639c3f47454484728415 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 3 Jul 2018 13:47:58 -0400 Subject: [PATCH 3/4] Small edits --- .../main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy | 2 +- docs/painless/painless-getting-started.asciidoc | 5 ++++- server/build.gradle | 2 +- .../src/main/java/org/elasticsearch/script/ScriptModule.java | 4 ++-- .../fielddata/ScriptDocValuesMissingV6BehaviourTests.java | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index ae0f4e111c9bb..3c9894138b2d1 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -683,7 +683,7 @@ class BuildPlugin implements Plugin { systemProperty 'tests.task', path systemProperty 'tests.security.manager', 'true' systemProperty 'jna.nosys', 'true' - systemProperty 'es.script.exception_for_missing_value', 'true' + systemProperty 'es.scripting.exception_for_missing_value', 'true' // TODO: remove setting logging level via system property systemProperty 'tests.logger.level', 'WARN' for (Map.Entry property : System.properties.entrySet()) { diff --git a/docs/painless/painless-getting-started.asciidoc b/docs/painless/painless-getting-started.asciidoc index 90c6664d8a1fc..22ec2d1fcb059 100644 --- a/docs/painless/painless-getting-started.asciidoc +++ b/docs/painless/painless-getting-started.asciidoc @@ -136,9 +136,12 @@ the document, `doc['field'].value` for this document returns: IMPORTANT: Starting in 7.0, `doc['field'].value` throws an exception if the field is missing in a document. To enable this behavior now, set a <> -`-Des.script.exception_for_missing_value=true` on a node. If you do not enable +`-Des.scripting.exception_for_missing_value=true` on a node. If you do not enable this behavior, a deprecation warning is logged on start up. +To check if a document is missing a value, you can call +`doc['field'].size() == 0`. + [float] ==== Updating Fields with Painless diff --git a/server/build.gradle b/server/build.gradle index 236ac2c7ee650..85c5f590979de 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -342,7 +342,7 @@ if (isEclipse == false || project.path == ":server-tests") { // TODO: remove ScriptDocValuesMissingV6BehaviourTests in 7.0 additionalTest('testScriptDocValuesMissingV6Behaviour'){ include '**/ScriptDocValuesMissingV6BehaviourTests.class' - systemProperty 'es.script.exception_for_missing_value', 'false' + systemProperty 'es.scripting.exception_for_missing_value', 'false' } test { // these are tested explicitly in separate test tasks diff --git a/server/src/main/java/org/elasticsearch/script/ScriptModule.java b/server/src/main/java/org/elasticsearch/script/ScriptModule.java index 746ec7b0b5149..042953117c5a5 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptModule.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptModule.java @@ -65,7 +65,7 @@ public class ScriptModule { } public static final boolean EXCEPTION_FOR_MISSING_VALUE = - Booleans.parseBoolean(System.getProperty("es.script.exception_for_missing_value", "false")); + Booleans.parseBoolean(System.getProperty("es.scripting.exception_for_missing_value", "false")); private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ScriptModule.class)); @@ -94,7 +94,7 @@ public ScriptModule(Settings settings, List scriptPlugins) { } if (EXCEPTION_FOR_MISSING_VALUE == false) DEPRECATION_LOGGER.deprecated("Script: returning default values for missing document values is deprecated. " + - "Set system property '-Des.script.exception_for_missing_value=true' " + + "Set system property '-Des.scripting.exception_for_missing_value=true' " + "to make behaviour compatible with future major versions."); scriptService = new ScriptService(settings, Collections.unmodifiableMap(engines), Collections.unmodifiableMap(contexts)); } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java index 7df58dbe3fdfd..1dc836874d847 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesMissingV6BehaviourTests.java @@ -50,7 +50,7 @@ public ScriptEngine getScriptEngine(Settings settings, Collection Date: Fri, 6 Jul 2018 17:47:00 -0400 Subject: [PATCH 4/4] Fixing test failure --- modules/mapper-extras/build.gradle | 2 +- modules/parent-join/build.gradle | 2 +- modules/percolator/build.gradle | 2 +- server/build.gradle | 2 +- .../search/functionscore/FunctionScoreIT.java | 9 +++------ 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/modules/mapper-extras/build.gradle b/modules/mapper-extras/build.gradle index 31d5483a30571..7831de3a68e94 100644 --- a/modules/mapper-extras/build.gradle +++ b/modules/mapper-extras/build.gradle @@ -20,4 +20,4 @@ esplugin { description 'Adds advanced field mappers' classname 'org.elasticsearch.index.mapper.MapperExtrasPlugin' -} \ No newline at end of file +} diff --git a/modules/parent-join/build.gradle b/modules/parent-join/build.gradle index 1b7a5ac6ae9cb..67bcc9d54e8e7 100644 --- a/modules/parent-join/build.gradle +++ b/modules/parent-join/build.gradle @@ -21,4 +21,4 @@ esplugin { description 'This module adds the support parent-child queries and aggregations' classname 'org.elasticsearch.join.ParentJoinPlugin' hasClientJar = true -} \ No newline at end of file +} diff --git a/modules/percolator/build.gradle b/modules/percolator/build.gradle index e958548f89081..db4a716af6513 100644 --- a/modules/percolator/build.gradle +++ b/modules/percolator/build.gradle @@ -35,4 +35,4 @@ dependencyLicenses { } compileJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" -compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" \ No newline at end of file +compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes" diff --git a/server/build.gradle b/server/build.gradle index 85c5f590979de..da60bca5a3e81 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -329,7 +329,7 @@ if (isEclipse == false || project.path == ":server-tests") { task integTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP, description: 'Multi-node tests', - dependsOn: test.dependsOn) { + dependsOn: test.dependsOn.collect()) { configure(BuildPlugin.commonTestConfig(project)) classpath = project.test.classpath testClassesDirs = project.test.testClassesDirs diff --git a/server/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreIT.java b/server/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreIT.java index 0e92aba2a8552..12e48a3ae4f0a 100644 --- a/server/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreIT.java +++ b/server/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreIT.java @@ -134,15 +134,12 @@ public void testScriptScoresWithAgg() throws IOException { } public void testMinScoreFunctionScoreBasic() throws IOException { - index(INDEX, TYPE, jsonBuilder().startObject().field("num", 2).endObject()); - refresh(); float score = randomFloat(); float minScore = randomFloat(); - index(INDEX, TYPE, jsonBuilder().startObject() - .field("num", 2) - .field("random_score", score) // Pass the random score as a document field so that it can be extracted in the script - .endObject()); + .field("num", 2) + .field("random_score", score) // Pass the random score as a document field so that it can be extracted in the script + .endObject()); refresh(); ensureYellow();