Handle NPE due to a weird configuration

[ebuildy]

Prevent java.lang.NullPointerException. To reproduce: set max_query_terms to 0 and you get:

"Caused by: java.lang.NullPointerException",
"at org.elasticsearch.common.lucene.search.XMoreLikeThis.createQueue(XMoreLikeThis.java:766) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.common.lucene.search.XMoreLikeThis.createQueue(XMoreLikeThis.java:716) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.common.lucene.search.XMoreLikeThis.like(XMoreLikeThis.java:630) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.common.lucene.search.MoreLikeThisQuery.createQuery(MoreLikeThisQuery.java:172) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.common.lucene.search.MoreLikeThisQuery.rewrite(MoreLikeThisQuery.java:156) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.apache.lucene.search.IndexSearcher.rewrite(IndexSearcher.java:667) ~[lucene-core-8.1.0.jar:8.1.0 dbe5ed0b2f17677ca6c904ebae919363f2d36a0a - ishan - 2019-05-09 19:34:03]",
"at org.elasticsearch.search.internal.ContextIndexSearcher.rewrite(ContextIndexSearcher.java:110) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.DefaultSearchContext.preProcess(DefaultSearchContext.java:263) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.query.QueryPhase.preProcess(QueryPhase.java:91) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.SearchService.createContext(SearchService.java:603) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.SearchService.createAndPutContext(SearchService.java:550) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:353) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.search.SearchService.lambda$executeQueryPhase$1(SearchService.java:340) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.action.ActionListener.lambda$map$2(ActionListener.java:145) ~[elasticsearch-7.3.1.jar:7.3.1]",
"at org.elasticsearch.action.ActionListener$1.onResponse(ActionListener.java:62) ~[elasticsearch-7.3.1.jar:7.3.1]",
"... 8 more"] }

• Have you signed the contributor license agreement?
• Have you followed the contributor guidelines?
• If submitting code, have you built your formula locally prior to submission with gradle check?
• If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
• If submitting code, have you checked that your submission is for an OS and architecture that we support?
• If you are submitting this code for a class then read our policy for that.

[williamrandolph]

@elasticmachine test this please

[elasticmachine]

Pinging @elastic/es-search (:Search/Search)

[cbuescher]

Hi @ebuildy,

thanks for the PR, I'd like to understand how you ran into this particular NPE and would rather catch it earlier on by checking the input parameters and add vaidation checks / throw IAE in the builders and query itself before we even get to constructing a query. Does this make sense or am I missing that would make max_query_terms of 0 (or even negative) a setting we should to support?

[cbuescher]

Rather than checking for a particular value this late, I'd rather try improving the input validation for the max_query_terms parameter, both in the MoreLikeThisQueryBuilder setter and also maybe in MoreLikeThisQuery. I think we should reject negative or zero values here, since at least to my understanding it indicates a wrong usage of the API. Or are there cases you know where this parameter setting would make sense?

[ebuildy]

I agree there is no sense to set max_query_terms = 0, maybe the API should validate this parameter instead.

[cbuescher]

Are you interested in changing this PR to add validation to the MoreLikeThisQueryBuilder and MoreLikeThisQuery setters for the max query terms? I think we should reject all values <= 0 there already with an IllegalArgumentException and a fitting message. Adding these checkes should also be tests in the corresponding unit tests. If you don't want to do this just let me know, I'll open an issue so we can fix it later.

[ebuildy]

Sure! thanks you

[cbuescher]

Hi @ebuildy, just checking in to see if you are still interested in working on this PR. If you are busy and cannot continue working on it we can also open an issue and somebody else can pick it up.

[ebuildy]

hello @cbuescher I am quite busy by this end of the year, if someone can work on it this would be perfect, sorry for my late.

[cbuescher]

@ebuildy no problem, thanks for getting back. I opened #49927 and will close this PR then, but feel free to pick it up once you have time if it is still open.