From 8e6b2b390981bc240d7a204fb7250286c953b590 Mon Sep 17 00:00:00 2001 From: Alexander Kazakov Date: Mon, 14 Mar 2016 12:04:06 +0300 Subject: [PATCH 1/3] Check that _value is used in aggregations script before setting value to specialValue #14262 --- .../script/expression/ExpressionSearchScript.java | 12 +++++++----- .../script/expression/MoreExpressionTests.java | 11 ++++++++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java index 0f56adeea55..3944090cef2 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java @@ -112,14 +112,16 @@ class ExpressionSearchScript implements SearchScript { @Override public void setNextVar(String name, Object value) { - assert(specialValue != null); // this should only be used for the special "_value" variable used in aggregations assert(name.equals("_value")); - if (value instanceof Number) { - specialValue.setValue(((Number)value).doubleValue()); - } else { - throw new ScriptException("Cannot use expression with text variable using " + compiledScript); + // _value isn't used in script if specialValue == null + if (specialValue != null) { + if (value instanceof Number) { + specialValue.setValue(((Number)value).doubleValue()); + } else { + throw new ScriptException("Cannot use expression with text variable using " + compiledScript); + } } } }; diff --git a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java index 5246d0dc306..1260919bfab 100644 --- a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java +++ b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java @@ -383,7 +383,11 @@ public class MoreExpressionTests extends ESIntegTestCase { .script(new Script("_value * 3", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null))) .addAggregation( AggregationBuilders.stats("double_agg").field("y") - .script(new Script("_value - 1.1", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null))); + .script(new Script("_value - 1.1", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null))) + .addAggregation( + AggregationBuilders.stats("const_agg").field("x") + .script(new Script("3.0", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null)) + ); SearchResponse rsp = req.get(); assertEquals(3, rsp.getHits().getTotalHits()); @@ -395,6 +399,11 @@ public class MoreExpressionTests extends ESIntegTestCase { stats = rsp.getAggregations().get("double_agg"); assertEquals(0.7, stats.getMax(), 0.0001); assertEquals(0.1, stats.getMin(), 0.0001); + + stats = rsp.getAggregations().get("const_agg"); + assertThat(stats.getMax(), equalTo(3.0)); + assertThat(stats.getMin(), equalTo(3.0)); + assertThat(stats.getAvg(), equalTo(3.0)); } public void testStringSpecialValueVariable() throws Exception { From e4bed0c97e7e0acb293624f9601027f8be38c874 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 Mar 2016 14:57:43 +0100 Subject: [PATCH 2/3] Improve validation error message on PutMappingRequest --- .../action/admin/indices/mapping/put/PutMappingRequest.java | 5 +++-- .../admin/indices/mapping/put/PutMappingRequestTests.java | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java b/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java index 17fdd6e5f00..7b389dba25a 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java @@ -35,6 +35,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.Index; import java.io.IOException; +import java.util.Arrays; import java.util.Map; import java.util.Objects; @@ -94,8 +95,8 @@ public class PutMappingRequest extends AcknowledgedRequest im validationException = addValidationError("mapping source is empty", validationException); } if (concreteIndex != null && (indices != null && indices.length > 0)) { - validationException = addValidationError("either concreteIndices or unresolved indices can be set concrete: [" + concreteIndex - + "] and indices: " + indices , validationException); + validationException = addValidationError("either concrete index or unresolved indices can be set, concrete index: [" + + concreteIndex + "] and indices: " + Arrays.asList(indices) , validationException); } return validationException; } diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java index 65b9ff0dd22..04892b82339 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java @@ -53,6 +53,8 @@ public class PutMappingRequestTests extends ESTestCase { r.setConcreteIndex(new Index("foo", "bar")); ex = r.validate(); assertNotNull("source validation should fail", ex); - assertTrue(ex.getMessage().contains("either concreteIndices or unresolved indices can be set")); + assertEquals(ex.getMessage(), + "Validation Failed: 1: either concrete index or unresolved indices can be set," + + " concrete index: [[foo/bar]] and indices: [myindex];"); } } From dfec4547eaf01e41d39929996750abaeb2a6a59d Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 14 Mar 2016 13:19:52 -0700 Subject: [PATCH 3/3] Added one minor comment for expressions tests. --- .../elasticsearch/script/expression/MoreExpressionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java index 1260919bfab..a8856ea78b5 100644 --- a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java +++ b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java @@ -385,7 +385,7 @@ public class MoreExpressionTests extends ESIntegTestCase { AggregationBuilders.stats("double_agg").field("y") .script(new Script("_value - 1.1", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null))) .addAggregation( - AggregationBuilders.stats("const_agg").field("x") + AggregationBuilders.stats("const_agg").field("x") // specifically to test a script w/o _value .script(new Script("3.0", ScriptType.INLINE, ExpressionScriptEngineService.NAME, null)) );