Missing fields values unexpectedly return 0 in painless

[mayya-sharipova]

Describe the feature: Missing fields values are not processed as expected in painless
Elasticsearch version (bin/elasticsearch --version): 6.2.3

Steps to reproduce:

PUT /indexscore1
{
  "mappings": {
			"doc": {
				"properties": {
					"f1": {
						"type":     "text"
					},
					"f2" : {
						"type" : "integer"
					}
				}
			}
		}
}

POST/_bulk
{ "index" : { "_index" : "indexscore1", "_type" : "doc", "_id" : "4" } }
{ "type" : "doc", "f1" : "beautiful horse"}

GET /indexscore1/_search
 {
	 "query" : {
		 "function_score": {
				"script_score" : {
					"script" : {
							"source" : "if (doc.containsKey('f2')) {return doc['f2'].value;} return 1.0;"
					}    
				}	
		 }
	 }
 }

I expect that "if (doc.containsKey('f2')) {return doc['f2'].value;} return 1.0;" should return 1 for my doc, but this returns 0:

"hits": [
                     {
				"_index": "indexscore1",
				"_type": "doc",
				"_id": "4",
				"_score": 0.0,
				"_source": {
					"type": "doc",
					"f1": "beautiful horse"
				}
			}
		]

[elasticmachine]

Pinging @elastic/es-core-infra

[mayya-sharipova]

I have clarified the following information from @rjernst and @jpountz :

• We use SortedNumericDocValuesto access document's integer values with the following code:

public long getValue() {
            if (count == 0) {
                return 0L;
            }
            return values[0];
        }

which does return 0.

• Lucene used to have a contract that NumericDocValuesshould return 0 when the document doesn't have a value. But later a bitset of documents that had a value was added and now we even have iterators of docs with a value, so this must be something that can be fixed

If we want painless not to return 0 for missing values, we should correct this.
This may be something we should change in LeafDocLookup. we can make it return null in this case, by checking scriptValues.isEmpty() before returning from get

[mayya-sharipova]

@rjernst Ryan, what do you think if we add these two functions to painless:

missing(doc['field1'], <floatValue>) - returns floatValue if a current document is missing field1, otherwise returns doc's value for this field. Only applicable for numeric fields.

multi_value(doc['field1'], <stringValue>). Options for <stringValue> are avg, min, max, sum. For multi valued fields, depending on the chosen option, returns their averages, minimum, maximum, or sum. Only applicable for numeric fields.

[rjernst]

missing(doc['field1'], <floatValue>)

I think this would be unintuitive to have to know about an additional method to call (a non member method on the return of doc['field'] too). Returning null from doc['field'] is more intuitive IMO. The field missing from doc means it is not in the mappings, and returning null means the current doc has no values.

multi_value(doc['field1'], <stringValue>)

I think this would be better as member methods on ScriptDocValues.Longs (and ScriptDocValues.Doubles too if we want them for double fields). This would be similar to methods we have for geo fields.

[jdconrad]

@rjernst I'm concerned about one thing - if we do return null for missing field values in this case, once we make scripting index-aware, we do not want to return objects from fields that represent numerical types so we get the performance benefit of not boxing/unboxing. We can probably just document the differences between index-aware versus non-index aware, but will this be confusing later on?