🐛 search_after not working with undefined fields

[jgnieuwhof]

As clarified here: elastic/elasticsearch#28806 elasticsearch returns -9223372036854775808 (elasticsearch's max long) as the sort value for documents that are missing the field that they are supposed to sort on. elasticsearch (non-JS) also accepts that number as a valid search_after value.

JS evaluates -9223372036854775808 as -9223372036854776000, which when fed back into search_after fails with an error No matching token for number_type [BIG_INTEGER].

Of course we can't give elasticsearch back -9223372036854775808 as a number in JS land, but we can give it back a string version of the same, at which point we get expected behaviour. My local fix has been to filter returned sort values as shown below.

x => (Number.isInteger(x) && !Number.isSafeInteger(x)) ? `-9223372036854775808` : x

It seems like a similar fix could be incorporated into the ES-JS codebase in order to isolate consumers from this issue altogether.

thoughts?

[spalger]

Hmm, this is tricky... my thoughts on the issue:

If my research is correct -9223372036854775808 is specifically Java's Long.MIN_VALUE and equals -2^63.

JavaScript's Number.MAX_SAFE_INTEGER is 2^53-1

2^63 is very different from 2^53, which is why I'm scared about automatically converting any number between the two into a string representing -2^63.

That said, I also kind of hate the way JavaScript handles this too, and I would much prefer a more explicit failure when esjs parses numbers from Elasticsearch that it can't actually represent. I think my preferred corse of action would be:

• With BigInt support at stage 3 I'd like to checkout and try polyfills/shims and JSON parsers that support it.
• if polyfill/shim produces a major performance or usability penalty either:
  • enable it in a major version bump with an option to disable
  • add option to easily enable it
• If BigInt support is not enabled and non-safe numbers are parsed by esjs we could throw, log warnings, or otherwise inform the user that they should turn on BigInt support.

[stale]

We understand that this might be important for you, but this issue has been automatically marked as stale because it has not had recent activity either from our end or yours.
It will be closed if no further activity occurs, please write a comment if you would like to keep this going.

Note: in the past months we have built a new client, that has just landed in master. If you want to open an issue or a pr for the legacy client, you should do that in https://github.com/elastic/elasticsearch-js-legacy