From 4f91152b3de6fd60936ece1d39e96498e2dc3aa7 Mon Sep 17 00:00:00 2001 From: kimchy Date: Tue, 27 Apr 2010 12:33:24 +0300 Subject: [PATCH] change numeric term or query parser field query to use range filter/query and not encoded Term query --- .../index/mapper/FieldMapper.java | 6 +- .../index/mapper/json/JsonAllFieldMapper.java | 2 +- .../index/mapper/json/JsonFieldMapper.java | 16 ++-- .../mapper/json/JsonNumberFieldMapper.java | 24 +++++- .../index/query/json/TermJsonQueryParser.java | 2 +- .../query/support/MapperQueryParser.java | 9 +- .../json/SimpleJsonIndexQueryParserTests.java | 84 +++++++++++-------- .../index/query/json/disMax.json | 4 +- .../index/query/json/field1.json | 2 +- 9 files changed, 93 insertions(+), 56 deletions(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index a2eeedb6c01..601ec7946a4 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -162,15 +162,15 @@ public interface FieldMapper { String indexedValue(T value); /** - * Should the field query {@link #termQuery(String)} be used when detecting this + * Should the field query {@link #fieldQuery(String)} be used when detecting this * field in query string. */ - boolean useTermQueryWithQueryString(); + boolean useFieldQueryWithQueryString(); /** * A field query for the specified value. */ - Query termQuery(String value); + Query fieldQuery(String value); /** * A term query to use when parsing a query string. Can return null. diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonAllFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonAllFieldMapper.java index 02c2aecd8f7..e0315b6e177 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonAllFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonAllFieldMapper.java @@ -110,7 +110,7 @@ public class JsonAllFieldMapper extends JsonFieldMapper implements AllFiel return new AllTermQuery(term); } - @Override public Query termQuery(String value) { + @Override public Query fieldQuery(String value) { return new AllTermQuery(new Term(names.indexName(), value)); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java index 11fe7db48d6..892fd6ba6e1 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java @@ -319,18 +319,18 @@ public abstract class JsonFieldMapper implements FieldMapper, JsonMapper { return value.toString(); } - @Override public boolean useTermQueryWithQueryString() { - return false; - } - - @Override public Query termQuery(String value) { - return new TermQuery(new Term(names.indexName(), indexedValue(value))); - } - @Override public Query queryStringTermQuery(Term term) { return null; } + @Override public boolean useFieldQueryWithQueryString() { + return false; + } + + @Override public Query fieldQuery(String value) { + return new TermQuery(new Term(names.indexName(), indexedValue(value))); + } + @Override public Filter fieldFilter(String value) { return new TermFilter(new Term(names.indexName(), indexedValue(value))); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonNumberFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonNumberFieldMapper.java index 2a700ff0c71..3e310d3c9a4 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonNumberFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonNumberFieldMapper.java @@ -23,6 +23,8 @@ import org.apache.lucene.analysis.NumericTokenStream; import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.document.Field; import org.apache.lucene.document.Fieldable; +import org.apache.lucene.search.Filter; +import org.apache.lucene.search.Query; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.util.ThreadLocals; @@ -115,10 +117,30 @@ public abstract class JsonNumberFieldMapper extends JsonFieldM /** * Use the field query created here when matching on numbers. */ - @Override public boolean useTermQueryWithQueryString() { + @Override public boolean useFieldQueryWithQueryString() { return true; } + /** + * Numeric field level query are basically range queries with same value and included. That's the recommended + * way to execute it. + */ + @Override public Query fieldQuery(String value) { + return rangeQuery(value, value, true, true); + } + + /** + * Numeric field level filter are basically range queries with same value and included. That's the recommended + * way to execute it. + */ + @Override public Filter fieldFilter(String value) { + return rangeFilter(value, value, true, true); + } + + @Override public abstract Query rangeQuery(String lowerTerm, String upperTerm, boolean includeLower, boolean includeUpper); + + @Override public abstract Filter rangeFilter(String lowerTerm, String upperTerm, boolean includeLower, boolean includeUpper); + /** * Override the default behavior (to return the string, and return the actual Number instance). */ diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/json/TermJsonQueryParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/json/TermJsonQueryParser.java index 5a586c3c6dc..884089c6349 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/json/TermJsonQueryParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/json/TermJsonQueryParser.java @@ -93,7 +93,7 @@ public class TermJsonQueryParser extends AbstractIndexComponent implements JsonQ MapperService.SmartNameFieldMappers smartNameFieldMappers = parseContext.smartFieldMappers(fieldName); if (smartNameFieldMappers != null) { if (smartNameFieldMappers.hasMapper()) { - query = smartNameFieldMappers.mapper().termQuery(value); + query = smartNameFieldMappers.mapper().fieldQuery(value); } } if (query == null) { diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/support/MapperQueryParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/support/MapperQueryParser.java index 9be49b95582..3523f4865ec 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/support/MapperQueryParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/support/MapperQueryParser.java @@ -81,10 +81,11 @@ public class MapperQueryParser extends QueryParser { if (fieldMappers != null) { currentMapper = fieldMappers.fieldMappers().mapper(); if (currentMapper != null) { - Query query; - if (currentMapper.useTermQueryWithQueryString()) { - query = currentMapper.termQuery(queryText); - } else { + Query query = null; + if (currentMapper.useFieldQueryWithQueryString()) { + query = currentMapper.fieldQuery(queryText); + } + if (query == null) { query = super.getFieldQuery(currentMapper.names().indexName(), queryText); } return wrapSmartNameQuery(query, fieldMappers, indexCache); diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/SimpleJsonIndexQueryParserTests.java b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/SimpleJsonIndexQueryParserTests.java index 874db173c57..355c24c62d8 100644 --- a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/SimpleJsonIndexQueryParserTests.java +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/SimpleJsonIndexQueryParserTests.java @@ -158,7 +158,7 @@ public class SimpleJsonIndexQueryParserTests { @Test public void testDisMaxBuilder() throws Exception { IndexQueryParser queryParser = newQueryParser(); - Query parsedQuery = queryParser.parse(disMaxQuery().boost(1.2f).tieBreaker(0.7f).add(termQuery("age", 34)).add(termQuery("age", 35))); + Query parsedQuery = queryParser.parse(disMaxQuery().boost(1.2f).tieBreaker(0.7f).add(termQuery("name.first", "first")).add(termQuery("name.last", "last"))); assertThat(parsedQuery, instanceOf(DisjunctionMaxQuery.class)); DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) parsedQuery; assertThat((double) disjunctionMaxQuery.getBoost(), closeTo(1.2, 0.01)); @@ -168,11 +168,11 @@ public class SimpleJsonIndexQueryParserTests { Query firstQ = disjuncts.get(0); assertThat(firstQ, instanceOf(TermQuery.class)); - assertThat(((TermQuery) firstQ).getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(((TermQuery) firstQ).getTerm(), equalTo(new Term("name.first", "first"))); Query secondsQ = disjuncts.get(1); assertThat(secondsQ, instanceOf(TermQuery.class)); - assertThat(((TermQuery) secondsQ).getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(35)))); + assertThat(((TermQuery) secondsQ).getTerm(), equalTo(new Term("name.last", "last"))); } @Test public void testDisMax() throws Exception { @@ -188,49 +188,57 @@ public class SimpleJsonIndexQueryParserTests { Query firstQ = disjuncts.get(0); assertThat(firstQ, instanceOf(TermQuery.class)); - assertThat(((TermQuery) firstQ).getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(((TermQuery) firstQ).getTerm(), equalTo(new Term("name.first", "first"))); Query secondsQ = disjuncts.get(1); assertThat(secondsQ, instanceOf(TermQuery.class)); - assertThat(((TermQuery) secondsQ).getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(35)))); + assertThat(((TermQuery) secondsQ).getTerm(), equalTo(new Term("name.last", "last"))); } @Test public void testTermQueryBuilder() throws IOException { IndexQueryParser queryParser = newQueryParser(); Query parsedQuery = queryParser.parse(termQuery("age", 34).buildAsBytes()); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); } @Test public void testTermQuery() throws IOException { IndexQueryParser queryParser = newQueryParser(); String query = copyToStringFromClasspath("/org/elasticsearch/index/query/json/term.json"); Query parsedQuery = queryParser.parse(query); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); } @Test public void testFieldQueryBuilder1() throws IOException { IndexQueryParser queryParser = newQueryParser(); Query parsedQuery = queryParser.parse(fieldQuery("age", 34).buildAsBytes()); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); } @Test public void testFieldQuery1() throws IOException { IndexQueryParser queryParser = newQueryParser(); String query = copyToStringFromClasspath("/org/elasticsearch/index/query/json/field1.json"); Query parsedQuery = queryParser.parse(query); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); } @Test public void testFieldQuery2() throws IOException { @@ -251,31 +259,37 @@ public class SimpleJsonIndexQueryParserTests { String query = copyToStringFromClasspath("/org/elasticsearch/index/query/json/field3.json"); Query parsedQuery = queryParser.parse(query); assertThat((double) parsedQuery.getBoost(), closeTo(2.0, 0.01)); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); } @Test public void testTermWithBoostQueryBuilder() throws IOException { IndexQueryParser queryParser = newQueryParser(); Query parsedQuery = queryParser.parse(termQuery("age", 34).boost(2.0f)); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); - assertThat((double) termQuery.getBoost(), closeTo(2.0, 0.01)); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); + assertThat((double) fieldQuery.getBoost(), closeTo(2.0, 0.01)); } @Test public void testTermWithBoostQuery() throws IOException { IndexQueryParser queryParser = newQueryParser(); String query = copyToStringFromClasspath("/org/elasticsearch/index/query/json/term-with-boost.json"); Query parsedQuery = queryParser.parse(query); - assertThat(parsedQuery, instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) parsedQuery; - // since age is automatically registered in data, we encode it as numeric - assertThat(termQuery.getTerm(), equalTo(new Term("age", NumericUtils.longToPrefixCoded(34)))); - assertThat((double) termQuery.getBoost(), closeTo(2.0, 0.01)); + assertThat(parsedQuery, instanceOf(NumericRangeQuery.class)); + NumericRangeQuery fieldQuery = (NumericRangeQuery) parsedQuery; + assertThat(fieldQuery.getMin().intValue(), equalTo(34)); + assertThat(fieldQuery.getMax().intValue(), equalTo(34)); + assertThat(fieldQuery.includesMax(), equalTo(true)); + assertThat(fieldQuery.includesMin(), equalTo(true)); + assertThat((double) fieldQuery.getBoost(), closeTo(2.0, 0.01)); } @Test public void testPrefixQueryBuilder() throws IOException { diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/disMax.json b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/disMax.json index a4e89deb1c7..03d33f10b96 100644 --- a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/disMax.json +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/disMax.json @@ -4,10 +4,10 @@ boost: 1.2, queries : [ { - term : { age : 34 } + term : { "name.first" : "first" } }, { - term : { age : 35 } + term : { "name.last" : "last" } } ] } diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/field1.json b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/field1.json index 55749a4db83..e33a7cb2d94 100644 --- a/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/field1.json +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/query/json/field1.json @@ -1,3 +1,3 @@ { - field : { age : 34 } + "field" : { "age" : 34 } } \ No newline at end of file