Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore support for the include/pattern syntax. #23140

Merged
merged 4 commits into from
Feb 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static Aggregator.Parser getParser(ParseFieldRegistry<SignificanceHeurist
IncludeExclude::parseInclude, IncludeExclude.INCLUDE_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);

parser.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(b.includeExclude(), v)),
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.STRING_ARRAY);
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);

for (String name : significanceHeuristicParserRegistry.getNames()) {
parser.declareObject(SignificantTermsAggregationBuilder::significanceHeuristic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
IncludeExclude::parseInclude, IncludeExclude.INCLUDE_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);

PARSER.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(b.includeExclude(), v)),
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.STRING_ARRAY);
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);
}

public static AggregationBuilder parse(String aggregationName, QueryParseContext context) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.search.DocValueFormat;

import java.io.IOException;
Expand All @@ -61,7 +60,7 @@
public class IncludeExclude implements Writeable, ToXContent {
public static final ParseField INCLUDE_FIELD = new ParseField("include");
public static final ParseField EXCLUDE_FIELD = new ParseField("exclude");
public static final ParseField PATTERN_FIELD = new ParseField("pattern");
public static final ParseField PATTERN_FIELD = new ParseField("pattern").withAllDeprecated("Put patterns directly under the [include] or [exclude]");
public static final ParseField PARTITION_FIELD = new ParseField("partition");
public static final ParseField NUM_PARTITIONS_FIELD = new ParseField("num_partitions");
// Needed to add this seed for a deterministic term hashing policy
Expand Down Expand Up @@ -94,7 +93,7 @@ public static IncludeExclude merge(IncludeExclude include, IncludeExclude exclud
}
}

public static IncludeExclude parseInclude(XContentParser parser, QueryParseContext context) throws IOException {
public static IncludeExclude parseInclude(XContentParser parser) throws IOException {
XContentParser.Token token = parser.currentToken();
if (token == XContentParser.Token.VALUE_STRING) {
return new IncludeExclude(parser.text(), null);
Expand All @@ -103,14 +102,15 @@ public static IncludeExclude parseInclude(XContentParser parser, QueryParseConte
} else if (token == XContentParser.Token.START_OBJECT) {
String currentFieldName = null;
Integer partition = null, numPartitions = null;
String pattern = 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.*"
if (PATTERN_FIELD.match(currentFieldName)) {
return new IncludeExclude(parser.text(), null);
pattern = parser.text();
} else if (NUM_PARTITIONS_FIELD.match(currentFieldName)) {
numPartitions = parser.intValue();
} else if (PARTITION_FIELD.match(currentFieldName)) {
Expand All @@ -120,6 +120,17 @@ public static IncludeExclude parseInclude(XContentParser parser, QueryParseConte
"Unknown parameter in Include/Exclude clause: " + currentFieldName);
}
}

final boolean hasPattern = pattern != null;
final boolean hasPartition = partition != null || numPartitions != null;
if (hasPattern && hasPartition) {
throw new IllegalArgumentException("Cannot mix pattern-based and partition-based includes");
}

if (pattern != null) {
return new IncludeExclude(pattern, null);
}

if (partition == null) {
throw new IllegalArgumentException("Missing [" + PARTITION_FIELD.getPreferredName()
+ "] parameter for partition-based include");
Expand All @@ -134,12 +145,28 @@ public static IncludeExclude parseInclude(XContentParser parser, QueryParseConte
}
}

public static IncludeExclude parseExclude(XContentParser parser, QueryParseContext context) throws IOException {
public static IncludeExclude parseExclude(XContentParser parser) throws IOException {
XContentParser.Token token = parser.currentToken();
if (token == XContentParser.Token.VALUE_STRING) {
return new IncludeExclude(null, parser.text());
} else if (token == XContentParser.Token.START_ARRAY) {
return new IncludeExclude(null, new TreeSet<>(parseArrayToSet(parser)));
} else if (token == XContentParser.Token.START_OBJECT) {
String currentFieldName = null;
String pattern = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if (PATTERN_FIELD.match(currentFieldName)) {
pattern = parser.text();
} else {
throw new IllegalArgumentException("Unrecognized field [" + parser.currentName() + "]");
}
}
if (pattern == null) {
throw new IllegalArgumentException("Missing [pattern] element under [exclude]");
}
return new IncludeExclude(null, pattern);
} else {
throw new IllegalArgumentException("Unrecognized token for an exclude [" + token + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude;
import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude.OrdinalsFilter;
Expand Down Expand Up @@ -231,11 +231,10 @@ private IncludeExclude serialize(IncludeExclude incExc, ParseField field) throws
assertEquals(field.getPreferredName(), parser.currentName());
token = parser.nextToken();

QueryParseContext parseContext = new QueryParseContext(parser);
if (field.getPreferredName().equalsIgnoreCase("include")) {
return IncludeExclude.parseInclude(parser, parseContext);
return IncludeExclude.parseInclude(parser);
} else if (field.getPreferredName().equalsIgnoreCase("exclude")) {
return IncludeExclude.parseExclude(parser, parseContext);
return IncludeExclude.parseExclude(parser);
} else {
throw new IllegalArgumentException(
"Unexpected field name serialized in test: " + field.getPreferredName());
Expand Down Expand Up @@ -271,7 +270,6 @@ private IncludeExclude serializeMixedRegex(IncludeExclude incExc) throws IOExcep
builder.endObject();

XContentParser parser = createParser(builder);
QueryParseContext parseContext = new QueryParseContext(parser);
XContentParser.Token token = parser.nextToken();
assertEquals(token, XContentParser.Token.START_OBJECT);

Expand All @@ -281,10 +279,10 @@ private IncludeExclude serializeMixedRegex(IncludeExclude incExc) throws IOExcep
assertEquals(XContentParser.Token.FIELD_NAME, token);
if (IncludeExclude.INCLUDE_FIELD.match(parser.currentName())) {
token = parser.nextToken();
inc = IncludeExclude.parseInclude(parser, parseContext);
inc = IncludeExclude.parseInclude(parser);
} else if (IncludeExclude.EXCLUDE_FIELD.match(parser.currentName())) {
token = parser.nextToken();
exc = IncludeExclude.parseExclude(parser, parseContext);
exc = IncludeExclude.parseExclude(parser);
} else {
throw new IllegalArgumentException("Unexpected field name serialized in test: " + parser.currentName());
}
Expand All @@ -295,4 +293,18 @@ private IncludeExclude serializeMixedRegex(IncludeExclude incExc) throws IOExcep
return IncludeExclude.merge(inc, exc);
}

public void testMixRegexAndPartition() throws Exception {
XContentBuilder builder = JsonXContent.contentBuilder()
.startObject()
.field("pattern", "a.*")
.field("partition", 1)
.field("num_partitions", 3)
.endObject();
try (XContentParser parser = createParser(builder)) {
parser.nextToken();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> IncludeExclude.parseInclude(parser));
assertEquals("Cannot mix pattern-based and partition-based includes", e.getMessage());
assertWarnings("Deprecated field [pattern] used, replaced by [Put patterns directly under the [include] or [exclude]]");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -667,3 +667,49 @@ setup:
- match: { aggregations.double_terms.buckets.0.key: 3.5 }

- match: { aggregations.double_terms.buckets.0.doc_count: 1 }

---
"Pattern include/exclude":

- skip:
features: "warnings"

- do:
index:
index: test_1
type: test
id: 1
body: { "str" : "abc" }

- do:
index:
index: test_1
type: test
id: 2
body: { "str": "bcd" }

- do:
index:
index: test_1
type: test
id: 3
body: { "str": "cde" }

- do:
indices.refresh: {}

- do:
warnings:
- "Deprecated field [pattern] used, replaced by [Put patterns directly under the [include] or [exclude]]"
search:
body: { "size" : 0, "aggs" : { "str_terms" : { "terms" : { "field" : "str", "include" : {"pattern": ".*d.*"}, "exclude": { "pattern": ".*e.*" } } } } }

- match: { hits.total: 3 }

- length: { aggregations.str_terms.buckets: 1 }

- match: { aggregations.str_terms.buckets.0.key: "bcd" }

- is_false: aggregations.str_terms.buckets.0.key_as_string

- match: { aggregations.str_terms.buckets.0.doc_count: 1 }
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,68 @@
search:
body: { "size" : 0, "aggs" : { "ip_terms" : { "significant_terms" : { "field" : "ip", "exclude" : "127.*" } } } }


---
"Pattern include/exclude":

- skip:
features: "warnings"

- do:
indices.create:
index: test_1
body:
settings:
number_of_shards: "1"
mappings:
test:
properties:
str:
type: keyword

- do:
index:
index: test_1
type: test
id: 1
body: { "str" : "abc" }

- do:
index:
index: test_1
type: test
id: 2
body: { "str": "bcd" }

- do:
index:
index: test_1
type: test
id: 3
body: { "str": "cde" }

- do:
index:
index: test_1
type: test
id: 4
body: { }


- do:
indices.refresh: {}

- do:
warnings:
- "Deprecated field [pattern] used, replaced by [Put patterns directly under the [include] or [exclude]]"
search:
body: { "size" : 0, "query" : { "exists" : { "field" : "str" } }, "aggs" : { "str_terms" : { "significant_terms" : { "field" : "str", "min_doc_count": 1, "include" : {"pattern": ".*d.*"}, "exclude": { "pattern": ".*e.*" } } } } }

- match: { hits.total: 3 }

- length: { aggregations.str_terms.buckets: 1 }

- match: { aggregations.str_terms.buckets.0.key: "bcd" }

- is_false: aggregations.str_terms.buckets.0.key_as_string

- match: { aggregations.str_terms.buckets.0.doc_count: 1 }