Fix hashCode values of aggregations' BytesValues.

This commit removes FilterBytesValues which is very trappy as the default
implementation forwards all method calls to the delegate. So if you do any
non-trivial modification to the terms or to the order of the terms, you need
to remember to override currentValueHash, copyShared, and this is very
error-prone.

FieldDataSource.WithScript.BytesValues and ScriptBytesValues now return correct
hash codes, future bugs here would be catched by the new assertion in
SortedUniqueBytesValues.

This bug was causing performance issues with scripts as all terms were assumed
to have the same hash code.

Close #5004

src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java

@@ -287,31 +287,34 @@ public org.elasticsearch.index.fielddata.BytesValues bytesValues() {
                 return bytesValues;
             }
 
-            static class SortedUniqueBytesValues extends FilterBytesValues {
+            static class SortedUniqueBytesValues extends BytesValues {
 
-                final BytesRef spare;
+                final BytesValues delegate;
                 int[] sortedIds;
                 final BytesRefHash bytes;
                 int numUniqueValues;
                 int pos = Integer.MAX_VALUE;
 
                 public SortedUniqueBytesValues(BytesValues delegate) {
-                    super(delegate);
+                    super(delegate.isMultiValued());
+                    this.delegate = delegate;
                     bytes = new BytesRefHash();
-                    spare = new BytesRef();
                 }
 
                 @Override
                 public int setDocument(int docId) {
-                    final int numValues = super.setDocument(docId);
+                    final int numValues = delegate.setDocument(docId);
                     if (numValues == 0) {
                         sortedIds = null;
                         return 0;
                     }
                     bytes.clear();
                     bytes.reinit();
                     for (int i = 0; i < numValues; ++i) {
-                        bytes.add(super.nextValue(), super.currentValueHash());
+                        final BytesRef next = delegate.nextValue();
+                        final int hash = delegate.currentValueHash();
+                        assert hash == next.hashCode();
+                        bytes.add(next, hash);
                     }
                     numUniqueValues = bytes.size();
                     sortedIds = bytes.sort(BytesRef.getUTF8SortedAsUnicodeComparator());
@@ -321,13 +324,8 @@ public int setDocument(int docId) {
 
                 @Override
                 public BytesRef nextValue() {
-                    bytes.get(sortedIds[pos++], spare);
-                    return spare;
-                }
-
-                @Override
-                public int currentValueHash() {
-                    return spare.hashCode();
+                    bytes.get(sortedIds[pos++], scratch);
+                    return scratch;
                 }
 
                 @Override
@@ -738,13 +736,11 @@ static class BytesValues extends org.elasticsearch.index.fielddata.BytesValues {
 
             private final FieldDataSource source;
             private final SearchScript script;
-            private final BytesRef scratch;
 
             public BytesValues(FieldDataSource source, SearchScript script) {
                 super(true);
                 this.source = source;
                 this.script = script;
-                scratch = new BytesRef();
             }
 
             @Override

src/main/java/org/elasticsearch/search/aggregations/support/bytes/ScriptBytesValues.java

@@ -37,7 +37,6 @@ public class ScriptBytesValues extends BytesValues implements ScriptValues {
 
     private Iterator<?> iter;
     private Object value;
-    private BytesRef scratch = new BytesRef();
 
     public ScriptBytesValues(SearchScript script) {
         super(true); // assume multi-valued

src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java

@@ -0,0 +1,141 @@
+/*
+ * 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.search.aggregations.support;
+
+import org.apache.lucene.index.AtomicReaderContext;
+import org.apache.lucene.search.Scorer;
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.index.fielddata.BytesValues;
+import org.elasticsearch.script.SearchScript;
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.junit.Test;
+
+import java.util.Map;
+
+public class FieldDataSourceTests extends ElasticsearchTestCase {
+
+    private static BytesValues randomBytesValues() {
+        final boolean multiValued = randomBoolean();
+        return new BytesValues(multiValued) {
+            @Override
+            public int setDocument(int docId) {
+                return randomInt(multiValued ? 10 : 1);
+            }
+            @Override
+            public BytesRef nextValue() {
+                scratch.copyChars(randomAsciiOfLength(10));
+                return scratch;
+            }
+
+        };
+    }
+
+    private static SearchScript randomScript() {
+        return new SearchScript() {
+
+            @Override
+            public void setNextVar(String name, Object value) {
+            }
+
+            @Override
+            public Object run() {
+                return randomAsciiOfLength(5);
+            }
+
+            @Override
+            public Object unwrap(Object value) {
+                return value;
+            }
+
+            @Override
+            public void setNextReader(AtomicReaderContext reader) {
+            }
+
+            @Override
+            public void setScorer(Scorer scorer) {
+            }
+
+            @Override
+            public void setNextDocId(int doc) {
+            }
+
+            @Override
+            public void setNextSource(Map<String, Object> source) {
+            }
+
+            @Override
+            public void setNextScore(float score) {
+            }
+
+            @Override
+            public float runAsFloat() {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public long runAsLong() {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public double runAsDouble() {
+                throw new UnsupportedOperationException();
+            }
+
+        };
+    }
+
+    private static void assertConsistent(BytesValues values) {
+        for (int i = 0; i < 10; ++i) {
+            final int valueCount = values.setDocument(i);
+            for (int j = 0; j < valueCount; ++j) {
+                final BytesRef term = values.nextValue();
+                assertEquals(term.hashCode(), values.currentValueHash());
+                assertTrue(term.bytesEquals(values.copyShared()));
+            }
+        }
+    }
+
+    @Test
+    public void bytesValuesWithScript() {
+        final BytesValues values = randomBytesValues();
+        FieldDataSource source = new FieldDataSource.Bytes() {
+
+            @Override
+            public BytesValues bytesValues() {
+                return values;
+            }
+
+            @Override
+            public MetaData metaData() {
+                throw new UnsupportedOperationException();
+            }
+
+        };
+        SearchScript script = randomScript();
+        assertConsistent(new FieldDataSource.WithScript.BytesValues(source, script));
+    }
+
+    @Test
+    public void sortedUniqueBytesValues() {
+        assertConsistent(new FieldDataSource.Bytes.SortedAndUnique.SortedUniqueBytesValues(randomBytesValues()));
+    }
+
+}