Parse Failure: NullPointerException

[mausch]

Query is pretty big and I have no idea what's causing it. I don't think that's relevant anyway, IMHO the code should check for nulls and throw a more specific exception, otherwise we clients are left in the dark about what we're doing wrong.
Here's the stack trace:

        at org.elasticsearch.search.SearchService.parseSource(SearchService.java:660)
        at org.elasticsearch.search.SearchService.createContext(SearchService.java:516)
        at org.elasticsearch.search.SearchService.createAndPutContext(SearchService.java:488)
        at org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:257)
        at org.elasticsearch.search.action.SearchServiceTransportAction$5.call(SearchServiceTransportAction.java:206)
        at org.elasticsearch.search.action.SearchServiceTransportAction$5.call(SearchServiceTransportAction.java:203)
        at org.elasticsearch.search.action.SearchServiceTransportAction$23.run(SearchServiceTransportAction.java:517)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
        at java.lang.Thread.run(Unknown Source)
Caused by: java.lang.NullPointerException
        at org.elasticsearch.search.aggregations.bucket.filter.FilterParser.parse(FilterParser.java:42)
        at org.elasticsearch.search.aggregations.AggregatorParsers.parseAggregators(AggregatorParsers.java:130)
        at org.elasticsearch.search.aggregations.AggregatorParsers.parseAggregators(AggregatorParsers.java:120)
        at org.elasticsearch.search.aggregations.AggregatorParsers.parseAggregators(AggregatorParsers.java:77)
        at org.elasticsearch.search.aggregations.AggregationParseElement.parse(AggregationParseElement.java:60)
        at org.elasticsearch.search.SearchService.parseSource(SearchService.java:644)
        ... 9 more

[clintongormley]

Query is pretty big and I have no idea what's causing it. I don't think that's relevant anyway, IMHO the code should check for nulls and throw a more specific exception, otherwise we clients are left in the dark about what we're doing wrong.

Agreed - can you tell us what you were doing when you saw this error? What does the query/agg look like?

[mausch]

It was mostly simple terms aggs but also one global agg with a filter. IIRC the query itself was that same filter being applied to the global agg.

[clintongormley]

@mausch could you provide some examples? Clearly we're not seeing this in our testing, so it would be helpful to see what you are doing different.

[mausch]

Sorry, I don't have that query any more. But I still insist that it shouldn't matter. I'd run some random-testing tool (e.g. https://github.com/pholser/junit-quickcheck ) on SearchService.parseSource and FilterParser.parse. No input ever should cause a NPE in my opinion, otherwise it's a bug.

[clintongormley]

@mausch I agree with you - we don't wilfully leave NPEs in our code. The trick is finding out what is causing it.

[vaibhavkulkar]

I am new to this community and I think I can fix this one to start with. Has some one already fixed it ?

[mausch]

@clintongormley Indeed, especially with complex code. Sorry I lost the original query. The only thing I can advice is to start applying randomized testing (if you haven't already, I don't know much about the ES codebase). It's great to find this kind of bugs. IIRC Lucene has started using it a couple of years ago.

@vaibhavkulkar unless there's already some randomized testing in place in the project this isn't trivial to fix.

[s1monw]

can you tell what filters you are using?

[mausch]

IIRC it was an And filter with a single terms filter within.

[clintongormley]

@mausch also, what version of Elasticsearch did you see this on?

[martijnvg]

The filters aggregation should in at FilterParser.java line 41 check for null. A null value is a valid return value for a filter parser (and for example the and filter does this).

[mausch]

@clintongormley version 1.3.4

[wcong]

I found the same NPE.
the version is 1.3.2.
In my case,it happened when i used FilterAggregation ,but sent no filter.
In java client,it looks like this.

        FilterBuilder fb = new AndFilterBuilder();
        FilterAggregationBuilder fab = AggregationBuilders
                .filter("filter")
                .filter(fb)
                .subAggregation(AggregationBuilders.terms("brandId").field("brand_id").size(0))
                .subAggregation(AggregationBuilders.terms("allAttr").field("all_attr").size(0));

In http query it looks like this.

[markharwood]

@martijnvg I can see where you say check for null but do you believe the correct response is then to throw a parse exception with a friendly message or to process the query using something like a MatchNoDocsFilter?

[martijnvg]

@markharwood I think if a filter parser returns null then the filters agg should interpret this as nothing is going to match, so MatchNoDocsFilter should be used.

[s1monw]

We had the same issue with bool query and what we do there is:

  if (clauses.isEmpty()) {
    return new MatchAllDocsQuery();
  }

I think bool filter should be consistent here - I am not sure which one we should change to be honest...

[markharwood]

I am not sure which one we should change to be honest...

Tough one. My gut feel on the empty filters array would be that it would mean "match none" but if we have a precedent for this on the query side which is "match all" then I guess we have these ugly options:

1. API Inconsistency (query=match all, filter = match none)
2. Consistently ALL (but empty filters matching all might feel weird)
3. Consistently NONE ( introduces a backwards compatibility issue here for change in query behaviour)
4. Parser error (avoids any ambiguity by forcing users to declare logic more explicitly)

[clintongormley]

The bool query change was made to make it easier to build such queries in tools like Kibana, ie they have a placeholder to which clauses can be added, but which doesn't fail if none are added.

If we don't specify a query (eg in filtered query) then it defaults to match_all. I think the same logic works for filters here. We're not specifying leaf filters against any fields, there is just no leaf filter. I'd lean towards matching ALL.

[s1monw]

+1 for what clint said. I thinks the deal here is no restrictions == match all so I think translating it to match all it the right thing. I personally think it's not perfectly fine for a filter parser to return null btw. I think the actual query/filter parser should handle these cases and return valid filter instead. @martijnvg what do you think?