From 0657593d58e24f931c0b27f6179e0cdf938ecac2 Mon Sep 17 00:00:00 2001 From: jimczi Date: Fri, 29 Mar 2019 11:40:29 +0100 Subject: [PATCH 1/3] more_like_this query to throw an error if the like fields is not provided With the removal of the `_all` field the `mlt` query cannot infer a field name to use to analyze the provided (un)like text if the `fields` parameter is not explicitly set in the query and the `index.query.default_field` is not changed in the index settings (by default it is set to `*`). For this reason the like text is ignored and queries are only built from the provided document ids. This change fixes this bug by throwing an error if the fields option is not set and the `index.query.default_field` is equals to `*`. The error is thrown only if like or unlike texts are provided in the query. --- .../index/query/MoreLikeThisQueryBuilder.java | 7 +++ .../query/MoreLikeThisQueryBuilderTests.java | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index b90a1e60ffa0b..11530ce5f30b3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -1050,6 +1050,13 @@ protected Query doToQuery(QueryShardContext context) throws IOException { List moreLikeFields = new ArrayList<>(); if (useDefaultField) { moreLikeFields = context.defaultFields(); + if (moreLikeFields.size() == 1 + && moreLikeFields.get(0).equals("*") + && (likeTexts.length > 0 || unlikeTexts.length > 0)) { + throw new IllegalArgumentException("[more_like_this] query cannot infer the field to analyze the free text, " + + "you should update the [index.query.default_field] index setting to a field that exists in the mapping or " + + "set the [fields] option in the query."); + } } else { for (String field : fields) { MappedFieldType fieldType = context.fieldMapper(field); diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index 62613139b50fd..c4a98903ec02f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -21,20 +21,26 @@ import org.apache.lucene.analysis.core.WhitespaceAnalyzer; import org.apache.lucene.index.Fields; +import org.apache.lucene.index.Term; import org.apache.lucene.index.memory.MemoryIndex; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.BoostQuery; +import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermQuery; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.termvectors.MultiTermVectorsItemResponse; import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; import org.elasticsearch.action.termvectors.MultiTermVectorsResponse; import org.elasticsearch.action.termvectors.TermVectorsRequest; import org.elasticsearch.action.termvectors.TermVectorsResponse; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.lucene.search.MoreLikeThisQuery; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -305,6 +311,39 @@ public void testUnsupportedFields() throws IOException { assertThat(e.getMessage(), containsString("more_like_this only supports text/keyword fields")); } + public void testDefaultField() throws IOException { + QueryShardContext context = createShardContext(); + + { + MoreLikeThisQueryBuilder builder = + new MoreLikeThisQueryBuilder(new String[]{"hello world"}, null); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> builder.toQuery(context)); + assertThat(e.getMessage(), containsString("[more_like_this] query cannot infer")); + } + + { + context.getIndexSettings().updateIndexMetaData( + newIndexMeta("index", + context.getIndexSettings().getSettings(), + Settings.builder().putList("index.query.default_field", STRING_FIELD_NAME).build() + ) + ); + try { + MoreLikeThisQueryBuilder builder = new MoreLikeThisQueryBuilder(new String[]{"hello world"}, null); + builder.toQuery(context); + } finally { + // Reset the default value + context.getIndexSettings().updateIndexMetaData( + newIndexMeta("index", + context.getIndexSettings().getSettings(), + Settings.builder().putList("index.query.default_field", "*").build() + ) + ); + } + } + } + public void testMoreLikeThisBuilder() throws Exception { Query parsedQuery = parseQuery(moreLikeThisQuery(new String[]{"name.first", "name.last"}, new String[]{"something"}, null) @@ -390,4 +429,11 @@ protected QueryBuilder parseQuery(XContentParser parser) throws IOException { } return query; } + + private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { + Settings build = Settings.builder().put(oldIndexSettings) + .put(indexSettings) + .build(); + return IndexMetaData.builder(name).settings(build).build(); + } } From 8e0be9ed775befd5f7e524bcc898420e9f0c5624 Mon Sep 17 00:00:00 2001 From: jimczi Date: Fri, 29 Mar 2019 16:32:53 +0100 Subject: [PATCH 2/3] fix test to not create invalid builder --- .../index/query/MoreLikeThisQueryBuilderTests.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index c4a98903ec02f..56e7a32347265 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -21,14 +21,10 @@ import org.apache.lucene.analysis.core.WhitespaceAnalyzer; import org.apache.lucene.index.Fields; -import org.apache.lucene.index.Term; import org.apache.lucene.index.memory.MemoryIndex; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.BoostQuery; -import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermQuery; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.termvectors.MultiTermVectorsItemResponse; import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; @@ -166,13 +162,13 @@ protected MoreLikeThisQueryBuilder doCreateTestQueryBuilder() { } else { likeItems = randomLikeItems; } - if (randomBoolean()) { // for the default field - queryBuilder = new MoreLikeThisQueryBuilder(likeTexts, likeItems); + if (randomBoolean() && likeItems != null && likeItems.length > 0) { // for the default field + queryBuilder = new MoreLikeThisQueryBuilder(null, likeItems); } else { queryBuilder = new MoreLikeThisQueryBuilder(randomFields, likeTexts, likeItems); } - if (randomBoolean()) { + if (randomBoolean() && queryBuilder.fields() != null) { queryBuilder.unlike(generateRandomStringArray(5, 5, false, false)); } if (randomBoolean()) { From 186f6df4fb2ccfa934728fcc2c2a3c80ba794ccf Mon Sep 17 00:00:00 2001 From: jimczi Date: Mon, 1 Apr 2019 08:47:18 +0200 Subject: [PATCH 3/3] fix IT --- .../elasticsearch/search/morelikethis/MoreLikeThisIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java index 2e29c7c5a3815..4492353f6f15b 100644 --- a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java +++ b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java @@ -343,10 +343,10 @@ public void testNumericField() throws Exception { new MoreLikeThisQueryBuilder(new String[] {"string_value", "int_value"}, null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)), SearchPhaseExecutionException.class); - // mlt query with no field -> No results (because _all is not enabled) - searchResponse = client().prepareSearch().setQuery(moreLikeThisQuery(new String[] {"index"}).minTermFreq(1).minDocFreq(1)) - .get(); - assertHitCount(searchResponse, 0L); + // mlt query with no field -> exception because _all is not enabled) + assertThrows(client().prepareSearch() + .setQuery(moreLikeThisQuery(new String[] {"index"}).minTermFreq(1).minDocFreq(1)), + SearchPhaseExecutionException.class); // mlt query with string fields searchResponse = client().prepareSearch().setQuery(moreLikeThisQuery(new String[]{"string_value"}, new String[] {"index"}, null)