Skip to content

Commit

Permalink
Handle missing values in painless (#32207)
Browse files Browse the repository at this point in the history
Throw an exception for doc['field'].value
if this document is missing a value for the field.

After deprecation changes have been backported to 6.x,
make this a default behaviour in 7.0

Closes #29286
  • Loading branch information
mayya-sharipova authored Jul 19, 2018
1 parent 9ae6905 commit 4c68dfe
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 266 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,6 @@ class BuildPlugin implements Plugin<Project> {
systemProperty 'tests.task', path
systemProperty 'tests.security.manager', 'true'
systemProperty 'jna.nosys', '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<String, String> property : System.properties.entrySet()) {
Expand Down
17 changes: 2 additions & 15 deletions docs/painless/painless-getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,8 @@ GET hockey/_search
[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 {ref}/jvm-options.html[`jvm.option`]
`-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.
`doc['field'].value` throws an exception if
the field is missing in a document.

To check if a document is missing a value, you can call
`doc['field'].size() == 0`.
Expand Down
10 changes: 0 additions & 10 deletions server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,6 @@ if (isEclipse) {
compileJava.options.compilerArgs << "-Xlint:-cast,-deprecation,-rawtypes,-try,-unchecked"
compileTestJava.options.compilerArgs << "-Xlint:-cast,-deprecation,-rawtypes,-try,-unchecked"

// TODO: remove ScriptDocValuesMissingV6BehaviourTests in 7.0
additionalTest('testScriptDocValuesMissingV6Behaviour'){
include '**/ScriptDocValuesMissingV6BehaviourTests.class'
systemProperty 'es.scripting.exception_for_missing_value', 'false'
}
test {
// these are tested explicitly in separate test tasks
exclude '**/*ScriptDocValuesMissingV6BehaviourTests.class'
}

forbiddenPatterns {
exclude '**/*.json'
exclude '**/*.jmx'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
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;
Expand Down Expand Up @@ -126,11 +125,8 @@ 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[<field>].size()==0 to check if a document is missing a field!");
}
return 0L;
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return values[0];
}
Expand Down Expand Up @@ -172,11 +168,8 @@ 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[<field>].size()==0 to check if a document is missing a field!");
}
return EPOCH;
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return get(0);
}
Expand Down Expand Up @@ -277,11 +270,8 @@ 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[<field>].size()==0 to check if a document is missing a field!");
}
return 0d;
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return values[0];
}
Expand Down Expand Up @@ -337,11 +327,8 @@ 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! " +
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return null;
}
return values[0];
}
Expand Down Expand Up @@ -454,11 +441,8 @@ protected void resize(int newSize) {

public boolean 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[<field>].size()==0 to check if a document is missing a field!");
}
return false;
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return values[0];
}
Expand Down Expand Up @@ -544,11 +528,8 @@ public String get(int index) {

public String getValue() {
if (count == 0) {
if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) {
throw new IllegalStateException("A document doesn't have a value for a field! " +
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return null;
}
return get(0);
}
Expand All @@ -572,11 +553,8 @@ public BytesRef get(int index) {

public BytesRef getValue() {
if (count == 0) {
if (ScriptModule.EXCEPTION_FOR_MISSING_VALUE) {
throw new IllegalStateException("A document doesn't have a value for a field! " +
throw new IllegalStateException("A document doesn't have a value for a field! " +
"Use doc[<field>].size()==0 to check if a document is missing a field!");
}
return new BytesRef();
}
return get(0);
}
Expand Down
13 changes: 1 addition & 12 deletions server/src/main/java/org/elasticsearch/script/ScriptModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@
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}.
Expand Down Expand Up @@ -64,11 +62,6 @@ public class ScriptModule {
).collect(Collectors.toMap(c -> c.name, Function.identity()));
}

public static final boolean EXCEPTION_FOR_MISSING_VALUE =
Booleans.parseBoolean(System.getProperty("es.scripting.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<ScriptPlugin> scriptPlugins) {
Expand All @@ -92,10 +85,6 @@ public ScriptModule(Settings settings, List<ScriptPlugin> scriptPlugins) {
}
}
}
if (EXCEPTION_FOR_MISSING_VALUE == false)
DEPRECATION_LOGGER.deprecated("Script: returning default values for missing document values is deprecated. " +
"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));
}

Expand Down

This file was deleted.

0 comments on commit 4c68dfe

Please sign in to comment.