Skip to content

Commit

Permalink
Internally manipulate the terms execution hint as an enum instead of …
Browse files Browse the repository at this point in the history 
…a String.

Close #5530
  • Loading branch information
@jpountz
jpountz committed Mar 25, 2014
1 parent de97b2a commit ed6dcf6
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 19 deletions.
78 changes: 63 additions & 15 deletions src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java
View file
Expand Up @@ -20,9 +20,11 @@

import org.apache.lucene.index.AtomicReaderContext;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.search.aggregations.AggregationExecutionException;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.Aggregator.BucketAggregationMode;
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude;
import org.elasticsearch.search.aggregations.support.AggregationContext;
import org.elasticsearch.search.aggregations.support.ValueSourceAggregatorFactory;
Expand All @@ -36,8 +38,55 @@
*/
public class TermsAggregatorFactory extends ValueSourceAggregatorFactory {

public static final String EXECUTION_HINT_VALUE_MAP = "map";
public static final String EXECUTION_HINT_VALUE_ORDINALS = "ordinals";
public enum ExecutionMode {
MAP(new ParseField("map")) {

@Override
Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
InternalOrder order, int requiredSize, int shardSize, long minDocCount, IncludeExclude includeExclude,
AggregationContext aggregationContext, Aggregator parent) {
return new StringTermsAggregator(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
}

},
ORDINALS(new ParseField("ordinals")) {

@Override
Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
InternalOrder order, int requiredSize, int shardSize, long minDocCount, IncludeExclude includeExclude,
AggregationContext aggregationContext, Aggregator parent) {
if (includeExclude != null) {
throw new ElasticsearchIllegalArgumentException("The `" + this + "` execution mode cannot filter terms.");
}
return new StringTermsAggregator.WithOrdinals(name, factories, (BytesValuesSource.WithOrdinals) valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, aggregationContext, parent);
}

};

public static ExecutionMode fromString(String value) {
for (ExecutionMode mode : values()) {
if (mode.parseField.match(value)) {
return mode;
}
}
throw new ElasticsearchIllegalArgumentException("Unknown `execution_hint`: [" + value + "], expected any of " + values());
}

private final ParseField parseField;

ExecutionMode(ParseField parseField) {
this.parseField = parseField;
}

abstract Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
InternalOrder order, int requiredSize, int shardSize, long minDocCount,
IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent);

@Override
public String toString() {
return parseField.getPreferredName();
}
}

private final InternalOrder order;
private final int requiredSize;
Expand Down Expand Up @@ -113,31 +162,30 @@ protected Aggregator create(ValuesSource valuesSource, long expectedBucketsCount
estimatedBucketCount = Math.min(estimatedBucketCount, 512);

if (valuesSource instanceof BytesValuesSource) {
if (executionHint != null && !executionHint.equals(EXECUTION_HINT_VALUE_MAP) && !executionHint.equals(EXECUTION_HINT_VALUE_ORDINALS)) {
throw new ElasticsearchIllegalArgumentException("execution_hint can only be '" + EXECUTION_HINT_VALUE_MAP + "' or '" + EXECUTION_HINT_VALUE_ORDINALS + "', not " + executionHint);
ExecutionMode execution = null;
if (executionHint != null) {
execution = ExecutionMode.fromString(executionHint);
}
String execution = executionHint;

// In some cases, using ordinals is just not supported: override it
if (!(valuesSource instanceof BytesValuesSource.WithOrdinals)) {
execution = EXECUTION_HINT_VALUE_MAP;
execution = ExecutionMode.MAP;
} else if (includeExclude != null) {
execution = EXECUTION_HINT_VALUE_MAP;
execution = ExecutionMode.MAP;
}

if (execution == null) {
// Let's try to use a good default
if ((valuesSource instanceof BytesValuesSource.WithOrdinals)
&& shouldUseOrdinals(parent, valuesSource, aggregationContext)) {
execution = EXECUTION_HINT_VALUE_ORDINALS;
execution = ExecutionMode.ORDINALS;
} else {
execution = EXECUTION_HINT_VALUE_MAP;
execution = ExecutionMode.MAP;
}
}
assert execution != null;

if (execution.equals(EXECUTION_HINT_VALUE_ORDINALS)) {
assert includeExclude == null;
return new StringTermsAggregator.WithOrdinals(name, factories, (BytesValuesSource.WithOrdinals) valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, aggregationContext, parent);
} else {
return new StringTermsAggregator(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
}
return execution.create(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
}

if (includeExclude != null) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/elasticsearch/search/aggregations/RandomTests.java
View file
Expand Up @@ -191,8 +191,8 @@ public void testDuelTerms() throws Exception {
SearchResponse resp = client().prepareSearch("idx")
.addAggregation(terms("long").field("long_values").size(maxNumTerms).subAggregation(min("min").field("num")))
.addAggregation(terms("double").field("double_values").size(maxNumTerms).subAggregation(max("max").field("num")))
.addAggregation(terms("string_map").field("string_values").executionHint(TermsAggregatorFactory.EXECUTION_HINT_VALUE_MAP).size(maxNumTerms).subAggregation(stats("stats").field("num")))
.addAggregation(terms("string_ordinals").field("string_values").executionHint(TermsAggregatorFactory.EXECUTION_HINT_VALUE_ORDINALS).size(maxNumTerms).subAggregation(extendedStats("stats").field("num"))).execute().actionGet();
.addAggregation(terms("string_map").field("string_values").executionHint(TermsAggregatorFactory.ExecutionMode.MAP.toString()).size(maxNumTerms).subAggregation(stats("stats").field("num")))
.addAggregation(terms("string_ordinals").field("string_values").executionHint(TermsAggregatorFactory.ExecutionMode.ORDINALS.toString()).size(maxNumTerms).subAggregation(extendedStats("stats").field("num"))).execute().actionGet();
assertEquals(0, resp.getFailedShards());

final Terms longTerms = resp.getAggregations().get("long");
Expand Down
3 changes: 1 addition & 2 deletions src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java
View file
Expand Up @@ -38,7 +38,6 @@
import org.junit.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.regex.Pattern;
Expand All @@ -61,7 +60,7 @@ public class StringTermsTests extends ElasticsearchIntegrationTest {
private static final String MULTI_VALUED_FIELD_NAME = "s_values";

public static String randomExecutionHint() {
return randomFrom(Arrays.asList(null, TermsAggregatorFactory.EXECUTION_HINT_VALUE_MAP, TermsAggregatorFactory.EXECUTION_HINT_VALUE_ORDINALS));
return randomBoolean() ? null : randomFrom(TermsAggregatorFactory.ExecutionMode.values()).toString();
}

@Before
Expand Down

0 comments on commit ed6dcf6

Please sign in to comment.