From eea35944468d4cae7d6e690d0dfcad38349ebe5c Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 28 Oct 2021 15:17:38 -0500 Subject: [PATCH 1/5] Script: support boolean fields in Fields API Add support for boolean fields with the following public API: * `boolean getValue(boolean)` Return the first boolean or the given default * `boolean getValue(int, boolean)` Return the boolean at index or the given default * `boolean[] getValues()` Return a copy of the array of booleans, may return a zero length array. Refs: #79105 --- .../org.elasticsearch.script.fields.txt | 6 + .../test/painless/50_script_doc_values.yml | 106 +++++++++++++++ .../index/fielddata/ScriptDocValues.java | 47 ++----- .../fielddata/plain/LeafLongFieldData.java | 3 +- .../script/field/BooleanDocValuesField.java | 121 ++++++++++++++++++ .../mapper/BooleanScriptFieldTypeTests.java | 18 +++ 6 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt index cf8f1ff2560b5..29ea5c4fe61d7 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt @@ -23,3 +23,9 @@ class org.elasticsearch.script.field.DelegateDocValuesField @dynamic_type { def getValue(def) List getValues() } + +class org.elasticsearch.script.field.BooleanDocValuesField @dynamic_type { + boolean getValue(boolean) + boolean getValue(int, boolean) + boolean[] getValues() +} diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index 29d526a7c7187..34b519370ae32 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -58,6 +58,12 @@ setup: scaled_float: 3.14 token_count: count all these words please + - do: + index: + index: test + id: 2 + body: {} + - do: indices.refresh: {} @@ -67,6 +73,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -77,12 +84,80 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: source: "doc['boolean'].value" - match: { hits.hits.0.fields.field.0: true } + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 1 } } + script_fields: + field: + script: + source: "field('boolean').getValue(false)" + - match: { hits.hits.0.fields.field.0: true } + + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 1 } } + script_fields: + field: + script: + source: "field('boolean').getValue(false)" + - match: { hits.hits.0.fields.field.0: true } + + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 2 } } + script_fields: + field: + script: + source: "field('boolean').getValue(false)" + - match: { hits.hits.0.fields.field.0: false } + + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 1 } } + script_fields: + field: + script: + source: "field('boolean').getValue(1, false)" + - match: { hits.hits.0.fields.field.0: false } + + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 1 } } + script_fields: + field: + script: + source: "field('boolean').getValues().length" + - match: { hits.hits.0.fields.field.0: 1 } + + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 2 } } + script_fields: + field: + script: + source: "field('boolean').getValues().length" + - match: { hits.hits.0.fields.field.0: 0 } + + --- "date": - skip: @@ -92,6 +167,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -102,6 +178,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -114,6 +191,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -125,6 +203,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -136,6 +215,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: centroid: script: @@ -147,6 +227,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: bbox: script: @@ -160,6 +241,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: topLeft: script: @@ -176,6 +258,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: type: script: @@ -186,6 +269,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: width: script: @@ -202,6 +286,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -212,6 +297,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -224,6 +310,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -234,6 +321,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -249,6 +337,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -259,6 +348,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -271,6 +361,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -281,6 +372,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -293,6 +385,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -303,6 +396,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -315,6 +409,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -325,6 +420,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -337,6 +433,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -347,6 +444,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -359,6 +457,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -369,6 +468,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -381,6 +481,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -391,6 +492,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -403,6 +505,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -413,6 +516,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -425,6 +529,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: @@ -435,6 +540,7 @@ setup: search: rest_total_hits_as_int: true body: + query: { term: { _id: 1 } } script_fields: field: script: 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 af5d9038478f2..e45ef46d60da2 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -17,13 +17,13 @@ import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.time.DateUtils; import org.elasticsearch.geometry.utils.Geohash; +import org.elasticsearch.script.field.BooleanDocValuesField; import java.io.IOException; import java.time.Instant; import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.AbstractList; -import java.util.Arrays; import java.util.Comparator; import java.util.function.UnaryOperator; @@ -454,60 +454,31 @@ public GeoBoundingBox getBoundingBox() { public static final class Booleans extends ScriptDocValues { - private final SortedNumericDocValues in; - private boolean[] values = new boolean[0]; - private int count; + private final BooleanDocValuesField booleanDocValuesField; - public Booleans(SortedNumericDocValues in) { - this.in = in; + public Booleans(BooleanDocValuesField booleanDocValuesField) { + this.booleanDocValuesField = booleanDocValuesField; } @Override public void setNextDocId(int docId) throws IOException { - if (in.advanceExact(docId)) { - resize(in.docValueCount()); - for (int i = 0; i < count; i++) { - values[i] = in.nextValue() == 1; - } - } else { - resize(0); - } - } - - /** - * Set the {@link #size()} and ensure that the {@link #values} array can - * store at least that many entries. - */ - protected void resize(int newSize) { - count = newSize; - values = grow(values, count); + throw new UnsupportedOperationException(); } public boolean getValue() { + throwIfEmpty(); return get(0); } @Override public Boolean get(int index) { - if (count == 0) { - 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 values[index]; + throwIfEmpty(); + return booleanDocValuesField.getInternalValues()[index]; } @Override public int size() { - return count; - } - - private static boolean[] grow(boolean[] array, int minSize) { - assert minSize >= 0 : "size must be positive (got " + minSize + "): likely integer overflow?"; - if (array.length < minSize) { - return Arrays.copyOf(array, ArrayUtil.oversize(minSize, 1)); - } else return array; + return booleanDocValuesField.size(); } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java index 5219575befd05..fc4014f8365d4 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java @@ -16,6 +16,7 @@ import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; +import org.elasticsearch.script.field.BooleanDocValuesField; import org.elasticsearch.script.field.DelegateDocValuesField; import org.elasticsearch.script.field.DocValuesField; import org.elasticsearch.search.DocValueFormat; @@ -56,7 +57,7 @@ public final DocValuesField getScriptField(String name) { name ); case BOOLEAN: - return new DelegateDocValuesField(new ScriptDocValues.Booleans(getLongValues()), name); + return new BooleanDocValuesField(getLongValues(), name); default: return new DelegateDocValuesField(new ScriptDocValues.Longs(getLongValues()), name); } diff --git a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java new file mode 100644 index 0000000000000..10fa6fa03e23a --- /dev/null +++ b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java @@ -0,0 +1,121 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.script.field; + +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.util.ArrayUtil; +import org.elasticsearch.index.fielddata.ScriptDocValues; + +import java.io.IOException; +import java.util.Arrays; + +public class BooleanDocValuesField implements DocValuesField { + + private final SortedNumericDocValues input; + private final String name; + + private boolean[] values = new boolean[0]; + private int count; + + private ScriptDocValues.Booleans booleansSDV = null; + + public BooleanDocValuesField(SortedNumericDocValues input, String name) { + this.input = input; + this.name = name; + } + + /** + * Set the current document ID. + * + * @param docId + */ + @Override + public void setNextDocId(int docId) throws IOException { + if (input.advanceExact(docId)) { + resize(input.docValueCount()); + for (int i = 0; i < count; i++) { + values[i] = input.nextValue() == 1; + } + } else { + resize(0); + } + } + + + private void resize(int newSize) { + count = newSize; + + assert count >= 0 : "size must be positive (got " + count + "): likely integer overflow?"; + if (values.length < count) { + values = Arrays.copyOf(values, ArrayUtil.oversize(count, 1)); + } + } + + /** + * Returns a {@code ScriptDocValues} of the appropriate type for this field. + * This is used to support backwards compatibility for accessing field values + * through the {@code doc} variable. + */ + @Override + public ScriptDocValues getScriptDocValues() { + if (booleansSDV == null) { + booleansSDV = new ScriptDocValues.Booleans(this); + } + + return booleansSDV; + } + + /** + * Returns the name of this field. + */ + @Override + public String getName() { + return name; + } + + /** + * Returns {@code true} if this field has no values, otherwise {@code false}. + */ + @Override + public boolean isEmpty() { + return count == 0; + } + + /** + * Returns the number of values this field has. + */ + @Override + public int size() { + return count; + } + + public boolean[] getValues() { + if (isEmpty()) { + return new boolean[0]; + } + return Arrays.copyOf(values, count); + } + + public boolean getValue(boolean defaultValue) { + return getValue(0, defaultValue); + } + + public boolean getValue(int index, boolean defaultValue) { + if (isEmpty() || index < 0 || index >= count) { + return defaultValue; + } + + return values[index]; + } + + public boolean[] getInternalValues() { + return values; + } + +} diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java index f99b6ee3e1390..e26d35ab643da 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java @@ -38,6 +38,7 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptCompiler; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.script.field.BooleanDocValuesField; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentParser; @@ -137,6 +138,23 @@ public double execute(ExplanationHolder explanation) { }; } }, searchContext.lookup(), 2.5f, "test", 0, Version.CURRENT)), equalTo(1)); + assertThat(searcher.count(new ScriptScoreQuery(new MatchAllDocsQuery(), new Script("test"), new ScoreScript.LeafFactory() { + @Override + public boolean needs_score() { + return false; + } + + @Override + public ScoreScript newInstance(DocReader docReader) { + return new ScoreScript(Map.of(), searchContext.lookup(), docReader) { + @Override + public double execute(ExplanationHolder explanation) { + BooleanDocValuesField booleans = (BooleanDocValuesField) field("test"); + return booleans.getValues()[0] ? 3 : 0; + } + }; + } + }, searchContext.lookup(), 2.5f, "test", 0, Version.CURRENT)), equalTo(1)); } } } From 3b0bfb698ee8d1cd28497bc5ab90d406a31f8ca8 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 28 Oct 2021 15:42:28 -0500 Subject: [PATCH 2/5] remove extra line --- .../org/elasticsearch/script/field/BooleanDocValuesField.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java index 10fa6fa03e23a..619e483f8e49d 100644 --- a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java +++ b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java @@ -47,7 +47,6 @@ public void setNextDocId(int docId) throws IOException { } } - private void resize(int newSize) { count = newSize; From 709a05f917137a7c3f492d029582f9c1198821e5 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 28 Oct 2021 17:08:17 -0500 Subject: [PATCH 3/5] List instead of array, allow SDV to set next doc --- .../org.elasticsearch.script.fields.txt | 2 +- .../test/painless/50_script_doc_values.yml | 4 ++-- .../index/fielddata/ScriptDocValues.java | 2 +- .../script/field/BooleanDocValuesField.java | 22 ++++++++++++------- .../mapper/BooleanScriptFieldTypeTests.java | 2 +- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt index 29ea5c4fe61d7..2b9619f62d043 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt @@ -27,5 +27,5 @@ class org.elasticsearch.script.field.DelegateDocValuesField @dynamic_type { class org.elasticsearch.script.field.BooleanDocValuesField @dynamic_type { boolean getValue(boolean) boolean getValue(int, boolean) - boolean[] getValues() + List getValues() } diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index 34b519370ae32..76369a1e60bb2 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -143,7 +143,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValues().length" + source: "field('boolean').getValues().size()" - match: { hits.hits.0.fields.field.0: 1 } - do: @@ -154,7 +154,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValues().length" + source: "field('boolean').getValues().size()" - match: { hits.hits.0.fields.field.0: 0 } 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 e45ef46d60da2..33bd891569822 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -462,7 +462,7 @@ public Booleans(BooleanDocValuesField booleanDocValuesField) { @Override public void setNextDocId(int docId) throws IOException { - throw new UnsupportedOperationException(); + booleanDocValuesField.setNextDocId(docId); } public boolean getValue() { diff --git a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java index 619e483f8e49d..4ab2b0eac0cbd 100644 --- a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java +++ b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java @@ -13,7 +13,10 @@ import org.elasticsearch.index.fielddata.ScriptDocValues; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.List; public class BooleanDocValuesField implements DocValuesField { @@ -23,7 +26,7 @@ public class BooleanDocValuesField implements DocValuesField { private boolean[] values = new boolean[0]; private int count; - private ScriptDocValues.Booleans booleansSDV = null; + private ScriptDocValues.Booleans booleans = null; public BooleanDocValuesField(SortedNumericDocValues input, String name) { this.input = input; @@ -63,11 +66,11 @@ private void resize(int newSize) { */ @Override public ScriptDocValues getScriptDocValues() { - if (booleansSDV == null) { - booleansSDV = new ScriptDocValues.Booleans(this); + if (booleans == null) { + booleans = new ScriptDocValues.Booleans(this); } - return booleansSDV; + return booleans; } /** @@ -94,11 +97,15 @@ public int size() { return count; } - public boolean[] getValues() { + public List getValues() { if (isEmpty()) { - return new boolean[0]; + return Collections.emptyList(); } - return Arrays.copyOf(values, count); + List list = new ArrayList<>(count); + for (int i = 0; i < count; i++) { + list.add(i, values[i]); + } + return list; } public boolean getValue(boolean defaultValue) { @@ -116,5 +123,4 @@ public boolean getValue(int index, boolean defaultValue) { public boolean[] getInternalValues() { return values; } - } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java index e26d35ab643da..ce0bc954e3b7a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java @@ -150,7 +150,7 @@ public ScoreScript newInstance(DocReader docReader) { @Override public double execute(ExplanationHolder explanation) { BooleanDocValuesField booleans = (BooleanDocValuesField) field("test"); - return booleans.getValues()[0] ? 3 : 0; + return booleans.getValues().get(0) ? 3 : 0; } }; } From 335328596454d9a94a4e843e240078ed16aeff83 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 3 Nov 2021 15:14:26 -0500 Subject: [PATCH 4/5] Update API to match DocValuesField --- .../test/painless/50_script_doc_values.yml | 31 +++++++++--- .../index/fielddata/ScriptDocValues.java | 2 +- .../script/field/BooleanDocValuesField.java | 50 ++++++++++++------- .../mapper/BooleanScriptFieldTypeTests.java | 2 +- .../index/mapper/MapperTestCase.java | 16 +++--- 5 files changed, 67 insertions(+), 34 deletions(-) diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index a2e15e0eaae6e..42d3f6f10a5d9 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -64,6 +64,13 @@ setup: id: 2 body: {} + - do: + index: + index: test + id: 3 + body: + boolean: [true, false, true] + - do: indices.refresh: {} @@ -99,7 +106,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValue(false)" + source: "field('boolean').get(false)" - match: { hits.hits.0.fields.field.0: true } - do: @@ -108,9 +115,10 @@ setup: body: query: { term: { _id: 1 } } script_fields: + field: script: - source: "field('boolean').getValue(false)" + source: "field('boolean').get(false)" - match: { hits.hits.0.fields.field.0: true } - do: @@ -121,7 +129,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValue(false)" + source: "field('boolean').get(false)" - match: { hits.hits.0.fields.field.0: false } - do: @@ -132,7 +140,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValue(1, false)" + source: "field('boolean').get(1, false)" - match: { hits.hits.0.fields.field.0: false } - do: @@ -143,9 +151,20 @@ setup: script_fields: field: script: - source: "field('boolean').getValues().size()" + source: "int total = 0; for (boolean b : field('boolean')) { total += b ? 1 : 0; } total;" - match: { hits.hits.0.fields.field.0: 1 } + - do: + search: + rest_total_hits_as_int: true + body: + query: { term: { _id: 3 } } + script_fields: + field: + script: + source: "int total = 0; for (boolean b : field('boolean')) { total += b ? 1 : 0; } total + field('boolean').size();" + - match: { hits.hits.0.fields.field.0: 5 } + - do: search: rest_total_hits_as_int: true @@ -154,7 +173,7 @@ setup: script_fields: field: script: - source: "field('boolean').getValues().size()" + source: "field('boolean').size()" - match: { hits.hits.0.fields.field.0: 0 } 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 6289afdb6de2c..9003a32db09f0 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -474,7 +474,7 @@ public boolean getValue() { @Override public Boolean get(int index) { throwIfEmpty(); - return booleanDocValuesField.getInternalValues()[index]; + return booleanDocValuesField.getInternal(index); } @Override diff --git a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java index 4ab2b0eac0cbd..6eed4eef37e3a 100644 --- a/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java +++ b/server/src/main/java/org/elasticsearch/script/field/BooleanDocValuesField.java @@ -13,12 +13,11 @@ import org.elasticsearch.index.fielddata.ScriptDocValues; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; -import java.util.List; +import java.util.Iterator; +import java.util.NoSuchElementException; -public class BooleanDocValuesField implements DocValuesField { +public class BooleanDocValuesField implements DocValuesField { private final SortedNumericDocValues input; private final String name; @@ -97,22 +96,36 @@ public int size() { return count; } - public List getValues() { - if (isEmpty()) { - return Collections.emptyList(); - } - List list = new ArrayList<>(count); - for (int i = 0; i < count; i++) { - list.add(i, values[i]); - } - return list; + /** + * Returns an iterator over elements of type {@code T}. + * + * @return an Iterator. + */ + @Override + public Iterator iterator() { + return new Iterator() { + private int index = 0; + + @Override + public boolean hasNext() { + return index < count; + } + + @Override + public Boolean next() { + if (hasNext() == false) { + throw new NoSuchElementException(); + } + return values[index++]; + } + }; } - public boolean getValue(boolean defaultValue) { - return getValue(0, defaultValue); + public boolean get(boolean defaultValue) { + return get(0, defaultValue); } - public boolean getValue(int index, boolean defaultValue) { + public boolean get(int index, boolean defaultValue) { if (isEmpty() || index < 0 || index >= count) { return defaultValue; } @@ -120,7 +133,8 @@ public boolean getValue(int index, boolean defaultValue) { return values[index]; } - public boolean[] getInternalValues() { - return values; + // this method is required to support the old-style "doc" access in ScriptDocValues + public boolean getInternal(int index) { + return values[index]; } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java index ce0bc954e3b7a..746635718a927 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanScriptFieldTypeTests.java @@ -150,7 +150,7 @@ public ScoreScript newInstance(DocReader docReader) { @Override public double execute(ExplanationHolder explanation) { BooleanDocValuesField booleans = (BooleanDocValuesField) field("test"); - return booleans.getValues().get(0) ? 3 : 0; + return booleans.getInternal(0) ? 3 : 0; } }; } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 1ee397de644c5..152f2fb60458a 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -31,6 +31,7 @@ import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.script.field.DocValuesField; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.lookup.LeafStoredFieldsLookup; import org.elasticsearch.search.lookup.SearchLookup; @@ -666,24 +667,23 @@ public final void testIndexTimeFieldData() throws IOException { LeafReaderContext ctx = ir.leaves().get(0); - ScriptDocValues fieldData = fieldType.fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) + DocValuesField docValuesField = fieldType.fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) .load(ctx) - .getScriptField("test") - .getScriptDocValues(); + .getScriptField("test"); - fieldData.setNextDocId(0); + docValuesField.setNextDocId(0); DocumentLeafReader reader = new DocumentLeafReader(doc.rootDoc(), Collections.emptyMap()); - ScriptDocValues indexData = fieldType.fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) + DocValuesField indexData = fieldType.fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) .load(reader.getContext()) - .getScriptField("test") - .getScriptDocValues(); + .getScriptField("test"); + indexData.setNextDocId(0); // compare index and search time fielddata - assertThat(fieldData, equalTo(indexData)); + assertThat(docValuesField.getScriptDocValues(), equalTo(indexData.getScriptDocValues())); }); } From 11da58386743bde108193d6dc69d1e4bffdd51c3 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 3 Nov 2021 15:19:06 -0500 Subject: [PATCH 5/5] spotless --- .../main/java/org/elasticsearch/index/mapper/MapperTestCase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 152f2fb60458a..7da507d04430a 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -28,7 +28,6 @@ import org.elasticsearch.core.CheckedConsumer; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.fielddata.IndexFieldDataCache; -import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.script.field.DocValuesField;