honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
Adrien Grand 2014-02-04 12:14:29 +01:00
parent 56f4fcb5df
commit 9333a3c28a
4 changed files with 152 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/main/java/org/elasticsearch/index/fielddata/FilterBytesValues.java
View File

@ -1,61 +0,0 @@
/*
* 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.index.fielddata;
import org.apache.lucene.util.BytesRef;
/**
* <code>FilterBytesValues</code> contains another {@link BytesValues}, which it
* uses as its basic source of data, possibly transforming the data along the
* way or providing additional functionality.
*/
public abstract class FilterBytesValues extends BytesValues {
protected final BytesValues delegate;
protected FilterBytesValues(BytesValues delegate) {
super(delegate.isMultiValued());
this.delegate = delegate;
}
@Override
public BytesRef copyShared() {
return delegate.copyShared();
}
@Override
public int setDocument(int docId) {
return delegate.setDocument(docId);
}
@Override
public BytesRef nextValue() {
return delegate.nextValue();
}
@Override
public int currentValueHash() {
return delegate.currentValueHash();
}
@Override
public AtomicFieldData.Order getOrder() {
return delegate.getOrder();
}
}

26
src/main/java/org/elasticsearch/search/aggregations/support/FieldDataSource.java
View File

@ -287,23 +287,23 @@ public abstract class FieldDataSource {
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;
@ -311,7 +311,10 @@ public abstract class FieldDataSource {
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 abstract class FieldDataSource {
@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 @@ public abstract class FieldDataSource {
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

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

@ -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

141
src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java Normal file
View File

@ -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()));
}
}