From 20ff703e07c692c5e68e6b30a6e6413a34dd10a7 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Sun, 11 Dec 2016 09:55:53 +0100 Subject: [PATCH] Fix IncludeExclude parsing `include` / `exclude` in terms / sig-terms aggs seems completely broken and massively untested. This commit makes the TermsTests pass again that randomly use `include` / `exclude`. This class must be tested individually and we need real integ tests that use xcontent that use this feature. --- .../bucket/terms/support/IncludeExclude.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java index 827d661d104cf..ea4797780c55a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java @@ -103,8 +103,11 @@ public static IncludeExclude parseInclude(XContentParser parser, QueryParseConte String currentFieldName = null; Integer partition = null, numPartitions = null; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else // This "include":{"pattern":"foo.*"} syntax is undocumented since 2.0 - // Regexes should be "include":"foo.*" + // Regexes should be "include":"foo.*" if (parseFieldMatcher.match(currentFieldName, PATTERN_FIELD)) { return new IncludeExclude(parser.text(), null); } else if (parseFieldMatcher.match(currentFieldName, NUM_PARTITIONS_FIELD)) { @@ -146,12 +149,10 @@ public static IncludeExclude parseExclude(XContentParser parser, QueryParseConte // in the index. public abstract static class LongFilter { public abstract boolean accept(long value); - + } - - public class PartitionedLongFilter extends LongFilter { - private final ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES); + public class PartitionedLongFilter extends LongFilter { @Override public boolean accept(long value) { // hash the value to keep even distributions @@ -159,8 +160,8 @@ public boolean accept(long value) { return Math.floorMod(hashCode, incNumPartitions) == incZeroBasedPartition; } } - - + + public static class SetBackedLongFilter extends LongFilter { private LongSet valids; private LongSet invalids; @@ -197,8 +198,8 @@ class PartitionedStringFilter extends StringFilter { public boolean accept(BytesRef value) { return Math.floorMod(value.hashCode(), incNumPartitions) == incZeroBasedPartition; } - } - + } + static class AutomatonBackedStringFilter extends StringFilter { private final ByteRunAutomaton runAutomaton; @@ -361,7 +362,7 @@ public IncludeExclude(SortedSet includeValues, SortedSet exc this.include = null; this.exclude = null; this.incZeroBasedPartition = 0; - this.incNumPartitions = 0; + this.incNumPartitions = 0; this.includeValues = includeValues; this.excludeValues = excludeValues; } @@ -388,11 +389,11 @@ public IncludeExclude(int partition, int numPartitions) { this.exclude = null; this.includeValues = null; this.excludeValues = null; - + } - - + + /** * Read from a stream. */ @@ -580,7 +581,7 @@ public boolean isRegexBased() { public boolean isPartitionBased() { return incNumPartitions > 0; } - + private Automaton toAutomaton() { Automaton a = null; if (include != null) { @@ -629,16 +630,16 @@ public OrdinalsFilter convertToOrdinalsFilter(DocValueFormat format) { if (isPartitionBased()){ return new PartitionedOrdinalsFilter(); } - + return new TermListBackedOrdinalsFilter(parseForDocValues(includeValues, format), parseForDocValues(excludeValues, format)); } public LongFilter convertToLongFilter(DocValueFormat format) { - + if(isPartitionBased()){ return new PartitionedLongFilter(); } - + int numValids = includeValues == null ? 0 : includeValues.size(); int numInvalids = excludeValues == null ? 0 : excludeValues.size(); SetBackedLongFilter result = new SetBackedLongFilter(numValids, numInvalids); @@ -659,7 +660,7 @@ public LongFilter convertToDoubleFilter() { if(isPartitionBased()){ return new PartitionedLongFilter(); } - + int numValids = includeValues == null ? 0 : includeValues.size(); int numInvalids = excludeValues == null ? 0 : excludeValues.size(); SetBackedLongFilter result = new SetBackedLongFilter(numValids, numInvalids); @@ -682,22 +683,21 @@ public LongFilter convertToDoubleFilter() { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { if (include != null) { builder.field(INCLUDE_FIELD.getPreferredName(), include.getOriginalString()); - } - if (includeValues != null) { + } else if (includeValues != null) { builder.startArray(INCLUDE_FIELD.getPreferredName()); for (BytesRef value : includeValues) { builder.value(value.utf8ToString()); } builder.endArray(); - } - if (isPartitionBased()) { + } else if (isPartitionBased()) { + builder.startObject(INCLUDE_FIELD.getPreferredName()); builder.field(PARTITION_FIELD.getPreferredName(), incZeroBasedPartition); builder.field(NUM_PARTITIONS_FIELD.getPreferredName(), incNumPartitions); + builder.endObject(); } if (exclude != null) { builder.field(EXCLUDE_FIELD.getPreferredName(), exclude.getOriginalString()); - } - if (excludeValues != null) { + } else if (excludeValues != null) { builder.startArray(EXCLUDE_FIELD.getPreferredName()); for (BytesRef value : excludeValues) { builder.value(value.utf8ToString());