diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryWrappingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryWrappingQueryBuilder.java index 7946aa0d0f509..06fadb9a67dc8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryWrappingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryWrappingQueryBuilder.java @@ -29,7 +29,7 @@ * Doesn't support conversion to {@link org.elasticsearch.common.xcontent.XContent} via {@link #doXContent(XContentBuilder, Params)}. */ //norelease to be removed once all queries support separate fromXContent and toQuery methods. Make AbstractQueryBuilder#toQuery final as well then. -public class QueryWrappingQueryBuilder extends AbstractQueryBuilder { +public class QueryWrappingQueryBuilder extends AbstractQueryBuilder implements MultiTermQueryBuilder { private Query query; diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java index 17ae1b4f00338..2248fca41b659 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilder.java @@ -18,18 +18,38 @@ */ package org.elasticsearch.index.query; +import org.apache.lucene.search.MultiTermQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; - import java.io.IOException; +import java.util.Objects; +/** + * Query that allows wraping a {@link MultiTermQueryBuilder} (one of wildcard, fuzzy, prefix, term, range or regexp query) + * as a {@link SpanQueryBuilder} so it can be nested. + */ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { public static final String NAME = "span_multi"; - private MultiTermQueryBuilder multiTermQueryBuilder; - static final SpanMultiTermQueryBuilder PROTOTYPE = new SpanMultiTermQueryBuilder(null); + private final MultiTermQueryBuilder multiTermQueryBuilder; + static final SpanMultiTermQueryBuilder PROTOTYPE = new SpanMultiTermQueryBuilder(); public SpanMultiTermQueryBuilder(MultiTermQueryBuilder multiTermQueryBuilder) { - this.multiTermQueryBuilder = multiTermQueryBuilder; + this.multiTermQueryBuilder = Objects.requireNonNull(multiTermQueryBuilder); + } + + /** + * only used for prototype + */ + private SpanMultiTermQueryBuilder() { + this.multiTermQueryBuilder = null; + } + + public MultiTermQueryBuilder multiTermQueryBuilder() { + return this.multiTermQueryBuilder; } @Override @@ -41,6 +61,42 @@ protected void doXContent(XContentBuilder builder, Params params) builder.endObject(); } + @Override + protected Query doToQuery(QueryParseContext parseContext) throws IOException { + Query subQuery = multiTermQueryBuilder.toQuery(parseContext); + if (subQuery instanceof MultiTermQuery == false) { + throw new UnsupportedOperationException("unsupported inner query, should be " + MultiTermQuery.class.getName() +" but was " + + subQuery.getClass().getName()); + } + return new SpanMultiTermQueryWrapper<>((MultiTermQuery) subQuery); + } + + @Override + public QueryValidationException validate() { + return validateInnerQuery(multiTermQueryBuilder, null); + } + + @Override + protected SpanMultiTermQueryBuilder doReadFrom(StreamInput in) throws IOException { + MultiTermQueryBuilder multiTermBuilder = in.readNamedWriteable(); + return new SpanMultiTermQueryBuilder(multiTermBuilder); + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(multiTermQueryBuilder); + } + + @Override + protected int doHashCode() { + return Objects.hash(multiTermQueryBuilder); + } + + @Override + protected boolean doEquals(SpanMultiTermQueryBuilder other) { + return Objects.equals(multiTermQueryBuilder, other.multiTermQueryBuilder); + } + @Override public String getName() { return NAME; diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryParser.java index 14841cb6df742..902031b4f7cc0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanMultiTermQueryParser.java @@ -18,9 +18,6 @@ */ package org.elasticsearch.index.query; -import org.apache.lucene.search.MultiTermQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,7 +28,7 @@ /** * */ -public class SpanMultiTermQueryParser extends BaseQueryParserTemp { +public class SpanMultiTermQueryParser extends BaseQueryParser { public static final String MATCH_NAME = "match"; @@ -45,7 +42,7 @@ public String[] names() { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); Token token = parser.nextToken(); @@ -58,13 +55,13 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars throw new QueryParsingException(parseContext, "spanMultiTerm must have [" + MATCH_NAME + "] multi term query clause"); } - Query subQuery = parseContext.parseInnerQuery(); - if (!(subQuery instanceof MultiTermQuery)) { + QueryBuilder subQuery = parseContext.parseInnerQueryBuilder(); + if (subQuery instanceof MultiTermQueryBuilder == false) { throw new QueryParsingException(parseContext, "spanMultiTerm [" + MATCH_NAME + "] must be of type multi term query"); } parser.nextToken(); - return new SpanMultiTermQueryWrapper<>((MultiTermQuery) subQuery); + return new SpanMultiTermQueryBuilder((MultiTermQueryBuilder) subQuery); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java b/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java index e355d9226184a..e86a0ecbfae21 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java +++ b/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import com.carrotsearch.randomizedtesting.generators.RandomInts; +import com.carrotsearch.randomizedtesting.generators.RandomStrings; import java.util.Random; @@ -36,7 +37,7 @@ public class RandomQueryBuilder { * @return a random {@link QueryBuilder} */ public static QueryBuilder createQuery(Random r) { - switch (RandomInts.randomIntBetween(r, 0, 3)) { + switch (RandomInts.randomIntBetween(r, 0, 4)) { case 0: return new MatchAllQueryBuilderTest().createTestQueryBuilder(); case 1: @@ -44,12 +45,29 @@ public static QueryBuilder createQuery(Random r) { case 2: return new IdsQueryBuilderTest().createTestQueryBuilder(); case 3: + return createMultiTermQuery(r); + case 4: return EmptyQueryBuilder.PROTOTYPE; default: throw new UnsupportedOperationException(); } } + /** + * Create a new multi term query of a random type + * @param r random seed + * @return a random {@link MultiTermQueryBuilder} + */ + public static MultiTermQueryBuilder createMultiTermQuery(Random r) { + // for now, only use String Rangequeries for MultiTerm test, numeric and date makes little sense + // see issue #12123 for discussion + // Prefix / Fuzzy / RegEx / Wildcard can go here later once refactored and they have random query generators + RangeQueryBuilder query = new RangeQueryBuilder(BaseQueryTestCase.STRING_FIELD_NAME); + query.from("a" + RandomStrings.randomAsciiOfLengthBetween(r, 1, 10)); + query.to("z" + RandomStrings.randomAsciiOfLengthBetween(r, 1, 10)); + return query; + } + /** * Create a new invalid query of a random type * @param r random seed diff --git a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java index abf611f5136e9..94466d2d2d570 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTest.java @@ -45,32 +45,42 @@ public class RangeQueryBuilderTest extends BaseQueryTestCase protected RangeQueryBuilder doCreateTestQueryBuilder() { RangeQueryBuilder query; // switch between numeric and date ranges - if (randomBoolean()) { - if (randomBoolean()) { - // use mapped integer field for numeric range queries - query = new RangeQueryBuilder(INT_FIELD_NAME); - query.from(randomIntBetween(1, 100)); - query.to(randomIntBetween(101, 200)); - } else { - // use unmapped field for numeric range queries - query = new RangeQueryBuilder(randomAsciiOfLengthBetween(1, 10)); - query.from(0.0 - randomDouble()); - query.to(randomDouble()); - } - } else { - // use mapped date field, using date string representation - query = new RangeQueryBuilder(DATE_FIELD_NAME); - query.from(new DateTime(System.currentTimeMillis() - randomIntBetween(0, 1000000), DateTimeZone.UTC).toString()); - query.to(new DateTime(System.currentTimeMillis() + randomIntBetween(0, 1000000), DateTimeZone.UTC).toString()); - // Create timestamp option only then we have a date mapper, otherwise we could trigger exception. - if (createContext().mapperService().smartNameFieldType(DATE_FIELD_NAME) != null) { + switch (randomIntBetween(0, 2)) { + case 0: if (randomBoolean()) { - query.timeZone(TIMEZONE_IDS.get(randomIntBetween(0, TIMEZONE_IDS.size() - 1))); + // use mapped integer field for numeric range queries + query = new RangeQueryBuilder(INT_FIELD_NAME); + query.from(randomIntBetween(1, 100)); + query.to(randomIntBetween(101, 200)); + } else { + // use unmapped field for numeric range queries + query = new RangeQueryBuilder(randomAsciiOfLengthBetween(1, 10)); + query.from(0.0 - randomDouble()); + query.to(randomDouble()); } - if (randomBoolean()) { - query.format("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); + break; + case 1: + // use mapped date field, using date string representation + query = new RangeQueryBuilder(DATE_FIELD_NAME); + query.from(new DateTime(System.currentTimeMillis() - randomIntBetween(0, 1000000), DateTimeZone.UTC).toString()); + query.to(new DateTime(System.currentTimeMillis() + randomIntBetween(0, 1000000), DateTimeZone.UTC).toString()); + // Create timestamp option only then we have a date mapper, + // otherwise we could trigger exception. + if (createContext().mapperService().smartNameFieldType(DATE_FIELD_NAME) != null) { + if (randomBoolean()) { + query.timeZone(TIMEZONE_IDS.get(randomIntBetween(0, TIMEZONE_IDS.size() - 1))); + } + if (randomBoolean()) { + query.format("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); + } } - } + break; + case 2: + default: + query = new RangeQueryBuilder(STRING_FIELD_NAME); + query.from("a" + randomAsciiOfLengthBetween(1, 10)); + query.to("z" + randomAsciiOfLengthBetween(1, 10)); + break; } query.includeLower(randomBoolean()).includeUpper(randomBoolean()); if (randomBoolean()) { diff --git a/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java new file mode 100644 index 0000000000000..c63b3e057ff09 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/SpanMultiTermQueryBuilderTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.search.MultiTermQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper; +import org.junit.Test; + +import java.io.IOException; + +public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase { + + @Override + protected Query doCreateExpectedQuery(SpanMultiTermQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException { + Query multiTermQuery = testQueryBuilder.multiTermQueryBuilder().toQuery(context); + return new SpanMultiTermQueryWrapper<>((MultiTermQuery) multiTermQuery); + } + + @Override + protected SpanMultiTermQueryBuilder doCreateTestQueryBuilder() { + MultiTermQueryBuilder multiTermQueryBuilder = RandomQueryBuilder.createMultiTermQuery(random()); + return new SpanMultiTermQueryBuilder(multiTermQueryBuilder); + } + + @Test + public void testValidate() { + int totalExpectedErrors = 0; + MultiTermQueryBuilder multiTermQueryBuilder; + if (randomBoolean()) { + multiTermQueryBuilder = new RangeQueryBuilder(""); + totalExpectedErrors++; + } else { + multiTermQueryBuilder = new RangeQueryBuilder("field"); + } + SpanMultiTermQueryBuilder queryBuilder = new SpanMultiTermQueryBuilder(multiTermQueryBuilder); + assertValidate(queryBuilder, totalExpectedErrors); + } + + @Test(expected = NullPointerException.class) + public void testInnerQueryNull() { + new SpanMultiTermQueryBuilder(null); + } + + /** + * test checks that we throw an {@link UnsupportedOperationException} if the query wrapped + * by {@link SpanMultiTermQueryBuilder} does not generate a lucene {@link MultiTermQuery}. + * This is currently the case for {@link RangeQueryBuilder} when the target field is mapped + * to a date. + */ + @Test + public void testUnsupportedInnerQueryType() throws IOException { + QueryParseContext parseContext = createContext(); + // test makes only sense if date field is mapped + if (parseContext.fieldMapper(DATE_FIELD_NAME) != null) { + try { + RangeQueryBuilder query = new RangeQueryBuilder(DATE_FIELD_NAME); + new SpanMultiTermQueryBuilder(query).toQuery(createContext()); + fail("Exception expected, range query on date fields should not generate a lucene " + MultiTermQuery.class.getName()); + } catch (UnsupportedOperationException e) { + assert(e.getMessage().contains("unsupported inner query, should be " + MultiTermQuery.class.getName())); + } + } + } +}