Aggregation key can't be number

[rightaway]

Elasticsearch version (bin/elasticsearch --version): 7.6.2

Plugins installed: []

JVM version (java -version): 13.0.2

OS version (uname -a if on a Unix-like system): archlinux

Description of the problem including expected versus actual behavior:

If you use a numeric key for a range aggregation you get an error. But string key works fine. Numeric keys should be accepted and turned into strings instead of failing.

Steps to reproduce:

aggs: {
  agg1: {
    range: {
      field: "agg1",
      ranges: [{ key: 1, from 1 }]
    }
  }
}

Provide logs (if relevant):

      type: 'x_content_parse_exception',
      reason: '[1:641] [range] failed to parse field [ranges]',
      caused_by: {                                                  
        type: 'parsing_exception',                                  
        reason: 'Failed to parse object: unknown field [key] found',
        line: 1,                                                    
        col: 641                                                    
      }

[elasticmachine]

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

[javanna]

Please could somebody @elastic/es-analytics-geo have a look and verify whether limiting range keys to strings is a bug or rather a conscious decision?

[nik9000]

@polyfractal what do you think about this? I don't know enough about keys to have an opinion.

[polyfractal]

I believe this is just a quirk of the old-style pull parser that Range still uses. If the token is a string, we check if it is potentially a key. If the token is a numeric value, we don't check for a key and thus throw the error.

It would be fairly trivial to cast a numeric key into a string and use that.

Note: We'll still need to return the key as a string, because default keys include the range (*-100), as well as BWC reasons. But I don't think there's a reason we can't accept numeric keys and cast to strings... that just feels like an oversight to me.

[nik9000]

But I don't think there's a reason we can't accept numeric keys and cast to strings... that just feels like an oversight to me.

Gotcha!

[nik9000]

@rightaway I've landed this change in 7.x which will become 7.9.0.