From d1d631794d4f76171c56319c2e4f3e27e32898f4 Mon Sep 17 00:00:00 2001 From: kimchy Date: Thu, 26 May 2011 11:17:20 +0300 Subject: [PATCH] Query DSL: Ids Filter / Query - allow to execute it with no type defined / several types, closes #969. --- .../index/mapper/MapperService.java | 5 ++++ .../index/query/xcontent/FilterBuilders.java | 10 ++++--- .../query/xcontent/IdsFilterBuilder.java | 18 ++++++++++--- .../index/query/xcontent/IdsFilterParser.java | 27 ++++++++++++++----- .../index/query/xcontent/IdsQueryBuilder.java | 18 ++++++++++--- .../index/query/xcontent/IdsQueryParser.java | 27 ++++++++++++++----- .../index/query/xcontent/QueryBuilders.java | 10 ++++--- .../elasticsearch/index/search/UidFilter.java | 24 ++++++++--------- .../search/query/SimpleQueryTests.java | 12 +++++++++ 9 files changed, 110 insertions(+), 41 deletions(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 606996483be..da67b54b660 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -54,6 +54,7 @@ import java.io.InputStreamReader; import java.io.Reader; import java.net.MalformedURLException; import java.net.URL; +import java.util.Collection; import java.util.Map; import java.util.Set; @@ -234,6 +235,10 @@ public class MapperService extends AbstractIndexComponent implements Iterable types() { + return mappers.keySet(); + } + public DocumentMapper documentMapper(String type) { return mappers.get(type); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/FilterBuilders.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/FilterBuilders.java index dd0d8f5ae1f..fb5a7b0713f 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/FilterBuilders.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/FilterBuilders.java @@ -19,6 +19,8 @@ package org.elasticsearch.index.query.xcontent; +import org.elasticsearch.common.Nullable; + /** * A static factory for simple "import static" usage. * @@ -34,12 +36,12 @@ public abstract class FilterBuilders { } /** - * Creates a new ids filter with the provided doc/mapping type. + * Creates a new ids filter with the provided doc/mapping types. * - * @param type The type + * @param types The types to match the ids against. */ - public static IdsFilterBuilder idsFilter(String type) { - return new IdsFilterBuilder(type); + public static IdsFilterBuilder idsFilter(@Nullable String... types) { + return new IdsFilterBuilder(types); } /** diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterBuilder.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterBuilder.java index 34392f80f90..566a69499cc 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterBuilder.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterBuilder.java @@ -31,7 +31,7 @@ import java.util.List; */ public class IdsFilterBuilder extends BaseFilterBuilder { - private String type; + private final List types; private List values = new ArrayList(); @@ -40,8 +40,8 @@ public class IdsFilterBuilder extends BaseFilterBuilder { /** * Create an ids filter based on the type. */ - public IdsFilterBuilder(String type) { - this.type = type; + public IdsFilterBuilder(String... types) { + this.types = types == null ? null : Arrays.asList(types); } /** @@ -69,7 +69,17 @@ public class IdsFilterBuilder extends BaseFilterBuilder { @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(IdsFilterParser.NAME); - builder.field("type", type); + if (types != null) { + if (types.size() == 1) { + builder.field("type", types.get(0)); + } else { + builder.startArray("types"); + for (Object type : types) { + builder.value(type); + } + builder.endArray(); + } + } builder.startArray("values"); for (Object value : values) { builder.value(value); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterParser.java index bf447d777f3..67bbfe5d773 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsFilterParser.java @@ -20,6 +20,8 @@ package org.elasticsearch.index.query.xcontent; import org.apache.lucene.search.Filter; +import org.elasticsearch.common.collect.ImmutableList; +import org.elasticsearch.common.collect.Iterables; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,6 +33,7 @@ import org.elasticsearch.index.settings.IndexSettings; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.List; public class IdsFilterParser extends AbstractIndexComponent implements XContentFilterParser { @@ -49,7 +52,7 @@ public class IdsFilterParser extends AbstractIndexComponent implements XContentF XContentParser parser = parseContext.parser(); List ids = new ArrayList(); - String type = null; + Collection types = null; String filterName = null; String currentFieldName = null; XContentParser.Token token; @@ -65,24 +68,36 @@ public class IdsFilterParser extends AbstractIndexComponent implements XContentF } ids.add(value); } + } else if ("types".equals(currentFieldName) || "type".equals(currentFieldName)) { + types = new ArrayList(); + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + String value = parser.textOrNull(); + if (value == null) { + throw new QueryParsingException(index, "No type specified for term filter"); + } + types.add(value); + } } } else if (token.isValue()) { if ("type".equals(currentFieldName) || "_type".equals(currentFieldName)) { - type = parser.text(); + types = ImmutableList.of(parser.text()); } else if ("_name".equals(currentFieldName)) { filterName = parser.text(); } } } - if (type == null) { - throw new QueryParsingException(index, "[ids] filter, no type provided"); - } if (ids.size() == 0) { throw new QueryParsingException(index, "[ids] filter, no ids values provided"); } - UidFilter filter = new UidFilter(type, ids, parseContext.indexCache().bloomCache()); + if (types == null || types.isEmpty()) { + types = parseContext.mapperService().types(); + } else if (types.size() == 1 && Iterables.getFirst(types, null).equals("_all")) { + types = parseContext.mapperService().types(); + } + + UidFilter filter = new UidFilter(types, ids, parseContext.indexCache().bloomCache()); if (filterName != null) { parseContext.addNamedFilter(filterName, filter); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryBuilder.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryBuilder.java index 9cb94aa98ca..bebe549338c 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryBuilder.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryBuilder.java @@ -31,14 +31,14 @@ import java.util.List; */ public class IdsQueryBuilder extends BaseQueryBuilder { - private String type; + private final List types; private List values = new ArrayList(); private float boost = -1; - public IdsQueryBuilder(String type) { - this.type = type; + public IdsQueryBuilder(String... types) { + this.types = types == null ? null : Arrays.asList(types); } /** @@ -67,7 +67,17 @@ public class IdsQueryBuilder extends BaseQueryBuilder { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(IdsQueryParser.NAME); - builder.field("type", type); + if (types != null) { + if (types.size() == 1) { + builder.field("type", types.get(0)); + } else { + builder.startArray("types"); + for (Object type : types) { + builder.value(type); + } + builder.endArray(); + } + } builder.startArray("values"); for (Object value : values) { builder.value(value); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryParser.java index 0825dd17cf9..d24279ec19e 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/IdsQueryParser.java @@ -21,6 +21,8 @@ package org.elasticsearch.index.query.xcontent; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.collect.ImmutableList; +import org.elasticsearch.common.collect.Iterables; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; @@ -32,6 +34,7 @@ import org.elasticsearch.index.settings.IndexSettings; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.List; /** @@ -53,7 +56,7 @@ public class IdsQueryParser extends AbstractIndexComponent implements XContentQu XContentParser parser = parseContext.parser(); List ids = new ArrayList(); - String type = null; + Collection types = null; String currentFieldName = null; float boost = 1.0f; XContentParser.Token token; @@ -69,24 +72,36 @@ public class IdsQueryParser extends AbstractIndexComponent implements XContentQu } ids.add(value); } + } else if ("types".equals(currentFieldName) || "type".equals(currentFieldName)) { + types = new ArrayList(); + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + String value = parser.textOrNull(); + if (value == null) { + throw new QueryParsingException(index, "No type specified for term filter"); + } + types.add(value); + } } } else if (token.isValue()) { if ("type".equals(currentFieldName) || "_type".equals(currentFieldName)) { - type = parser.text(); + types = ImmutableList.of(parser.text()); } else if ("boost".equals(currentFieldName)) { boost = parser.floatValue(); } } } - if (type == null) { - throw new QueryParsingException(index, "[ids] query, no type provided"); - } if (ids.size() == 0) { throw new QueryParsingException(index, "[ids] query, no ids values provided"); } - UidFilter filter = new UidFilter(type, ids, parseContext.indexCache().bloomCache()); + if (types == null || types.isEmpty()) { + types = parseContext.mapperService().types(); + } else if (types.size() == 1 && Iterables.getFirst(types, null).equals("_all")) { + types = parseContext.mapperService().types(); + } + + UidFilter filter = new UidFilter(types, ids, parseContext.indexCache().bloomCache()); // no need for constant score filter, since we don't cache the filter, and it always takes deletes into account ConstantScoreQuery query = new ConstantScoreQuery(filter); query.setBoost(boost); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/QueryBuilders.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/QueryBuilders.java index 94852d2f870..e8b81d3f447 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/QueryBuilders.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/xcontent/QueryBuilders.java @@ -19,6 +19,8 @@ package org.elasticsearch.index.query.xcontent; +import org.elasticsearch.common.Nullable; + /** * A static factory for simple "import static" usage. * @@ -73,12 +75,12 @@ public abstract class QueryBuilders { } /** - * Constructs a query that will match only specific ids within a type. + * Constructs a query that will match only specific ids within types. * - * @param type The mapping/doc type + * @param types The mapping/doc type */ - public static IdsQueryBuilder idsQuery(String type) { - return new IdsQueryBuilder(type); + public static IdsQueryBuilder idsQuery(@Nullable String... types) { + return new IdsQueryBuilder(types); } /** diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/UidFilter.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/UidFilter.java index 5cb23939e39..399468155d3 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/UidFilter.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/UidFilter.java @@ -33,22 +33,24 @@ import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.UidFieldMapper; import java.io.IOException; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; public class UidFilter extends Filter { - private final Term[] uids; + private final List uids; private final BloomCache bloomCache; - public UidFilter(String type, List ids, BloomCache bloomCache) { + public UidFilter(Collection types, List ids, BloomCache bloomCache) { this.bloomCache = bloomCache; - uids = new Term[ids.size()]; - for (int i = 0; i < ids.size(); i++) { - uids[i] = new Term(UidFieldMapper.NAME, Uid.createUid(type, ids.get(i))); + this.uids = new ArrayList(types.size() * ids.size()); + for (String type : types) { + for (String id : ids) { + uids.add(new Term(UidFieldMapper.NAME, Uid.createUid(type, id))); + } } - Arrays.sort(uids); } // TODO Optimizations @@ -86,15 +88,11 @@ public class UidFilter extends Filter { @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - UidFilter uidFilter = (UidFilter) o; - - if (!Arrays.equals(uids, uidFilter.uids)) return false; - - return true; + return !uids.equals(uidFilter.uids); } @Override public int hashCode() { - return uids != null ? Arrays.hashCode(uids) : 0; + return uids.hashCode(); } } \ No newline at end of file diff --git a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java index bea4014c6d1..2e2b5d018a2 100644 --- a/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java +++ b/modules/test/integration/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java @@ -174,11 +174,23 @@ public class SimpleQueryTests extends AbstractNodesTests { assertThat(searchResponse.hits().getAt(0).id(), anyOf(equalTo("1"), equalTo("3"))); assertThat(searchResponse.hits().getAt(1).id(), anyOf(equalTo("1"), equalTo("3"))); + // no type + searchResponse = client.prepareSearch().setQuery(constantScoreQuery(idsFilter().ids("1", "3"))).execute().actionGet(); + assertThat(searchResponse.hits().totalHits(), equalTo(2l)); + assertThat(searchResponse.hits().getAt(0).id(), anyOf(equalTo("1"), equalTo("3"))); + assertThat(searchResponse.hits().getAt(1).id(), anyOf(equalTo("1"), equalTo("3"))); + searchResponse = client.prepareSearch().setQuery(idsQuery("type1").ids("1", "3")).execute().actionGet(); assertThat(searchResponse.hits().totalHits(), equalTo(2l)); assertThat(searchResponse.hits().getAt(0).id(), anyOf(equalTo("1"), equalTo("3"))); assertThat(searchResponse.hits().getAt(1).id(), anyOf(equalTo("1"), equalTo("3"))); + // no type + searchResponse = client.prepareSearch().setQuery(idsQuery().ids("1", "3")).execute().actionGet(); + assertThat(searchResponse.hits().totalHits(), equalTo(2l)); + assertThat(searchResponse.hits().getAt(0).id(), anyOf(equalTo("1"), equalTo("3"))); + assertThat(searchResponse.hits().getAt(1).id(), anyOf(equalTo("1"), equalTo("3"))); + searchResponse = client.prepareSearch().setQuery(idsQuery("type1").ids("7", "10")).execute().actionGet(); assertThat(searchResponse.hits().totalHits(), equalTo(0l)); }