Skip to content

Commit

Permalink
Fix IncludeExclude parsing
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
s1monw committed Dec 11, 2016
1 parent e6b10ca commit 20ff703
Showing 1 changed file with 24 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -146,21 +149,19 @@ 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
final long hashCode = BitMixer.mix64(value);
return Math.floorMod(hashCode, incNumPartitions) == incZeroBasedPartition;
}
}


public static class SetBackedLongFilter extends LongFilter {
private LongSet valids;
private LongSet invalids;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -361,7 +362,7 @@ public IncludeExclude(SortedSet<BytesRef> includeValues, SortedSet<BytesRef> exc
this.include = null;
this.exclude = null;
this.incZeroBasedPartition = 0;
this.incNumPartitions = 0;
this.incNumPartitions = 0;
this.includeValues = includeValues;
this.excludeValues = excludeValues;
}
Expand All @@ -388,11 +389,11 @@ public IncludeExclude(int partition, int numPartitions) {
this.exclude = null;
this.includeValues = null;
this.excludeValues = null;

}



/**
* Read from a stream.
*/
Expand Down Expand Up @@ -580,7 +581,7 @@ public boolean isRegexBased() {
public boolean isPartitionBased() {
return incNumPartitions > 0;
}

private Automaton toAutomaton() {
Automaton a = null;
if (include != null) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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());
Expand Down

0 comments on commit 20ff703

Please sign in to comment.