more_like_this query to throw an error if the like fields is not provided #40632
Conversation
…ided 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.
jimczi
added
>bug
:Search/Search
|
Pinging @elastic/es-search |
| 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, " + |
There was a problem hiding this comment.
The error seems to be breaking MoreLikeThisQueryBuilderTests#testToQuery and also occasionally #testMustRewrite. Probably something in the test setup needs to be changed.
There was a problem hiding this comment.
Thanks good catch, I changed the test to not create invalid builders now that we have this restriction.
| } finally { | ||
| // Reset the default value | ||
| context.getIndexSettings().updateIndexMetaData( | ||
| newIndexMeta("index", |
There was a problem hiding this comment.
Just out of curiosity: why does the default need to get changed back? I was under the impression the context doesn't get reused? between different tests? If that is not the case, maybe the context should contain a copy of the index settings so changes to it don't affect other tests. This might be something to adress in another issue though if adressing it here is too involved.
There was a problem hiding this comment.
We update the settings for the created index that is shared among all tests in this class so we need to reset the value at the end. The query shard context is created for each test method but the index is static.
There was a problem hiding this comment.
Interesting, I think it would be good to change the test setup so this is something we don't have to worry about when writing tests. Probably worth a separate issue though.
There was a problem hiding this comment.
I think it's ok to share the same index on all tests. We can maybe restore the default value of the index settings after each test but it's very uncommon to change a setting in a query builder test so I don't think it's worth the change.
|
@elasticmachine run elasticsearch-ci/1 |
|
@elasticmachine run elasticsearch-ci/2 |
|
@cbuescher I pushed some changes to address your feedback, can you take another look ? |
…ided (#40632) 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.
…ided (elastic#40632) 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.
With the removal of the
_allfield themltquery cannot infer a field nameto use to analyze the provided (un)like text if the
fieldsparameter is notexplicitly set in the query and the
index.query.default_fieldis not changedin the index settings (by default it is set to
*). For this reason the like textis 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_fieldis equals to*. The error is thrown onlyif like or unlike texts are provided in the query.