Significant text aggregation nullpointers if subject field does not exist.

[markharwood]

This bug looks to have crept in since version 7-something.
Given a query like:

{
	"size": 0,
	"aggs": {
		"s": {
			"significant_text": {
				"field": "thisDoesNotExist"
			}
		}
	}
}

Version 6.6 responds with

"Aggregation [s] cannot process field [thisDoesNotExist] since it is not present"

But 7.10 onwards errors with:

{
  "error": {
	"root_cause": [
	  {
		"type": "null_pointer_exception",
		"reason": "Cannot invoke \"org.elasticsearch.index.mapper.MappedFieldType.indexAnalyzer()\" because the return value of \"org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource.access$400(org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource)\" is null"
	  }
	],
	"type": "search_phase_execution_exception",
	"reason": "all shards failed",
	"phase": "query",

Stack trace is

Caused by: java.lang.NullPointerException: Cannot invoke "org.elasticsearch.index.mapper.MappedFieldType.indexAnalyzer()" because the return value of "org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource.access$400(org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource)" is null
		at org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource$1.collectFromSource(SignificantTextAggregatorFactory.java:219) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]
		at org.elasticsearch.search.aggregations.bucket.terms.SignificantTextAggregatorFactory$SignificantTextCollectorSource$1.collect(SignificantTextAggregatorFactory.java:189) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]
		at org.elasticsearch.search.aggregations.LeafBucketCollectorBase.collect(LeafBucketCollectorBase.java:60) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]
		at org.elasticsearch.search.aggregations.bucket.terms.MapStringTermsAggregator$SignificantTermsResults$1.collect(MapStringTermsAggregator.java:472) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]
		at org.elasticsearch.search.aggregations.LeafBucketCollector.collect(LeafBucketCollector.java:107) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]

[elasticmachine]

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

[markharwood]

@jtibshirani - this may be going back a bit but it looks like null field types became acceptable with field alias work in 15ff3da . Can you offer any insight on when might be the best point to validate the field name is not resolved?

[nik9000]

@not-napoleon is who I'd turn to to ask about this sort of thing in aggs. I don't know this code super duper well so I don't have a strong opinion other than to bring @not-napoleon into the conversation.

[romseygeek]

I think the change in behaviour actually came in 91813ea when we merged sigterms and sigtext; that reworked the collectFromSource method that had previously been passed a MappedFieldType and so could check whether or not it was null.

I think the simplest fix is to enforce field existence in the SignificantTextAggregatorFactory constructor?

[markharwood]

I think the simplest fix is to enforce field existence in the SignificantTextAggregatorFactory constructor?

It was this comment from 15ff3da that made me hold off from that:

    // Note that if the field is unmapped (its field type is null), we don't fail,
    // and just use the given field name as a placeholder.

[nik9000]

Oh boy! Past Nik is at it again, breaking stuff.

[jtibshirani]

this may be going back a bit but it looks like null field types became acceptable with field alias work in 15ff3da

Reviewing the change where I added this comment, it seems I was just trying to maintain the current logic (which didn't explicitly check for a null field type), and didn't mean to assert that we should support significant_text on unmapped fields.

[markharwood]

Thanks y'all for the feedback.
I added a null test to the SignificantTextAggregatorFactory constructor