From 3d3dd7185d7638e572309b2e630d22234fc2659c Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Oct 2016 11:11:42 -0700 Subject: [PATCH] Add support for booleans in scripts (#20950) * Scripting: Add support for booleans in scripts Since 2.0, booleans have been represented as numeric fields (longs). However, in scripts, this is odd, since you expect doing a comparison against a boolean to work. While languages like groovy will auto convert between booleans and longs, painless does not. This changes the doc values accessor for boolean fields in scripts to return Boolean objects instead of Long objects. closes #20949 * Make Booleans final and remove wrapping of `this` for getValues() --- .../index/fielddata/ScriptDocValues.java | 34 +++++++++++++++++ .../fielddata/plain/AtomicLongFieldData.java | 38 +++++-------------- .../plain/SortedNumericDVIndexFieldData.java | 6 +-- .../painless/org.elasticsearch.txt | 6 +++ .../rest-api-spec/test/plan_a/30_search.yaml | 19 ++++++++-- 5 files changed, 69 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/core/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 3991a37a8bf..403a1290546 100644 --- a/core/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/core/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -291,4 +291,38 @@ public interface ScriptDocValues extends List { return geohashDistance(geohash); } } + + final class Booleans extends AbstractList implements ScriptDocValues { + + private final SortedNumericDocValues values; + + public Booleans(SortedNumericDocValues values) { + this.values = values; + } + + @Override + public void setNextDocId(int docId) { + values.setDocument(docId); + } + + @Override + public List getValues() { + return this; + } + + public boolean getValue() { + return values.count() != 0 && values.valueAt(0) == 1; + } + + @Override + public Boolean get(int index) { + return values.valueAt(index) == 1; + } + + @Override + public int size() { + return values.count(); + } + + } } diff --git a/core/src/main/java/org/elasticsearch/index/fielddata/plain/AtomicLongFieldData.java b/core/src/main/java/org/elasticsearch/index/fielddata/plain/AtomicLongFieldData.java index b3b0604e9e2..c52ccb90bed 100644 --- a/core/src/main/java/org/elasticsearch/index/fielddata/plain/AtomicLongFieldData.java +++ b/core/src/main/java/org/elasticsearch/index/fielddata/plain/AtomicLongFieldData.java @@ -19,28 +19,24 @@ package org.elasticsearch.index.fielddata.plain; -import org.apache.lucene.index.DocValues; -import org.apache.lucene.index.SortedNumericDocValues; -import org.apache.lucene.util.Accountable; import org.elasticsearch.index.fielddata.AtomicNumericFieldData; import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; -import java.util.Collection; -import java.util.Collections; - - /** * Specialization of {@link AtomicNumericFieldData} for integers. */ abstract class AtomicLongFieldData implements AtomicNumericFieldData { private final long ramBytesUsed; + /** True if this numeric data is for a boolean field, and so only has values 0 and 1. */ + private final boolean isBoolean; - AtomicLongFieldData(long ramBytesUsed) { + AtomicLongFieldData(long ramBytesUsed, boolean isBoolean) { this.ramBytesUsed = ramBytesUsed; + this.isBoolean = isBoolean; } @Override @@ -50,7 +46,11 @@ abstract class AtomicLongFieldData implements AtomicNumericFieldData { @Override public final ScriptDocValues getScriptValues() { - return new ScriptDocValues.Longs(getLongValues()); + if (isBoolean) { + return new ScriptDocValues.Booleans(getLongValues()); + } else { + return new ScriptDocValues.Longs(getLongValues()); + } } @Override @@ -63,24 +63,6 @@ abstract class AtomicLongFieldData implements AtomicNumericFieldData { return FieldData.castToDouble(getLongValues()); } - public static AtomicNumericFieldData empty(final int maxDoc) { - return new AtomicLongFieldData(0) { - - @Override - public SortedNumericDocValues getLongValues() { - return DocValues.emptySortedNumeric(maxDoc); - } - - @Override - public Collection getChildResources() { - return Collections.emptyList(); - } - - }; - } - @Override - public void close() { - } - + public void close() {} } diff --git a/core/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericDVIndexFieldData.java b/core/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericDVIndexFieldData.java index be877b9c68a..cf1fccabee0 100644 --- a/core/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericDVIndexFieldData.java +++ b/core/src/main/java/org/elasticsearch/index/fielddata/plain/SortedNumericDVIndexFieldData.java @@ -96,7 +96,7 @@ public class SortedNumericDVIndexFieldData extends DocValuesIndexFieldData imple case DOUBLE: return new SortedNumericDoubleFieldData(reader, field); default: - return new SortedNumericLongFieldData(reader, field); + return new SortedNumericLongFieldData(reader, field, numericType == NumericType.BOOLEAN); } } @@ -117,8 +117,8 @@ public class SortedNumericDVIndexFieldData extends DocValuesIndexFieldData imple final LeafReader reader; final String field; - SortedNumericLongFieldData(LeafReader reader, String field) { - super(0L); + SortedNumericLongFieldData(LeafReader reader, String field, boolean isBoolean) { + super(0L, isBoolean); this.reader = reader; this.field = field; } diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.txt index 3757e4fb76c..19af8204f28 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.txt @@ -101,6 +101,12 @@ class org.elasticsearch.index.fielddata.ScriptDocValues.GeoPoints -> org.elastic double geohashDistanceWithDefault(String,double) } +class org.elasticsearch.index.fielddata.ScriptDocValues.Booleans -> org.elasticsearch.index.fielddata.ScriptDocValues$Booleans extends List,Collection,Iterable,Object { + Boolean get(int) + boolean getValue() + List getValues() +} + # for testing. # currently FeatureTest exposes overloaded constructor, field load store, and overloaded static methods class org.elasticsearch.painless.FeatureTest -> org.elasticsearch.painless.FeatureTest extends Object { diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/30_search.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/30_search.yaml index f1051ba7106..d92c0e41e6c 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/30_search.yaml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/30_search.yaml @@ -6,19 +6,19 @@ index: test type: test id: 1 - body: { "test": "value beck", "num1": 1.0 } + body: { "test": "value beck", "num1": 1.0, "bool": true } - do: index: index: test type: test id: 2 - body: { "test": "value beck", "num1": 2.0 } + body: { "test": "value beck", "num1": 2.0, "bool": false } - do: index: index: test type: test id: 3 - body: { "test": "value beck", "num1": 3.0 } + body: { "test": "value beck", "num1": 3.0, "bool": true } - do: indices.refresh: {} @@ -95,6 +95,19 @@ - match: { hits.hits.1.fields.sNum1.0: 2.0 } - match: { hits.hits.2.fields.sNum1.0: 3.0 } + - do: + index: test + search: + body: + query: + script: + script: + inline: "doc['bool'].value == false" + lang: painless + + - match: { hits.total: 1 } + - match: { hits.hits.0._id: "2" } + --- "Custom Script Boost":