Allow binary sort values.

[jpountz]

The ip field uses a binary representation internally. This breaks when
rendering sort values in search responses since elasticsearch tries to write a
binary byte[] as an utf8 json string. This commit extends the DocValueFormat
API in order to give fields a chance to choose how to render values.

Closes #6077

Relates to #17971

[rjernst]

Is this really the best place for this? Seems it has nothing to do with actual networking, but just with the ip mapping, so it should probably go there? And does it really need to be public or can it be an impl detail of the mapper?

[rjernst]

I'm a little concerned that this change just propagates craziness through the internal api that was a hack before (turning BytesRef into Text). We have ipfield mapper using sorted doc values, so why can't we sort on byte[]?

[jpountz]

We do sort on byte[]. This change is only about how we expose the sort values to the user since I don't think BytesRef should be exposed in the client API (I chose to go with String but byte[] would work too, even though it is not straightforward since json does not support binary values).

I did it the change this way because I think this is the option that would play best in terms of backward compatibility. But we have other options:

• returning sort values for all fields that use binary/sorted doc values using eg. hexadecimal. This would be a break for string fields since sort values used to be the string value that was used at index time and it would now be an hexadecimal string.
• not exposing sort values in SearchResponse anymore and only using them internally to merge results coming from different shards.

[rjernst]

Why isn't how doc values are formatted for response separate from how they are transferred for distributed sorting?

[jpountz]

Just had a quick discussion with Ryan about it: one source of confusion here is due to the fact that mappings might not be available on the coordinating node, so shards have to get information about how to render sort values and then serialize it back to the coordinating node where the rendering will happen.

[jpountz]

I opened #17965 to discuss the general issue. However I don't think it should block this PR.

[rjernst]

@jpountz Instead of needing to pass along how to format to the coordinating node, could we pass along the formatted value (formatted on each shard using the mapper), but to the coordinating node that is just a black box string that is read, and inserted in the results for the docs which are chosen in top N?

[rjernst]

And that would mean also passing along the binary encoded values and sorting based on the binary value. So kind of a variation of your first option you proposed above.

[rjernst]

Note that it would also mean we would not need to render strings as sortable. eg we could keep ::1 because it would only be used for display, not for actual sorting on the coordinating node.

[clintongormley]

@jpountz don't forget that sort values should be reusable in their rendered form by the seach_after feature

[jpountz]

could we pass along the formatted value (formatted on each shard using the mapper), but to the coordinating node that is just a black box string that is read, and inserted in the results for the docs which are chosen in top N?

Will do!

Note that it would also mean we would not need to render strings as sortable. eg we could keep ::1 because it would only be used for display, not for actual sorting on the coordinating node.

Actually it is not needed in the current PR either. I first thought that we should aim at returning strings that sort in the same order as the underlying binary values but I'm starting to think this is not worth the trouble. I'll remove it.

[jpountz]

@jpountz don't forget that sort values should be reusable in their rendered form by the seach_after feature

Right, this is why I had to add the DocValuesFormat.parseBytesRef(String) method so that we can get back to the binary representation from the string value. This will likely also be needed for parsing include/exclude lists of terms aggregations, see #17705.

[jpountz]

I pushed a new commit that removes the ability to render sort values as sortable.

Instead of needing to pass along how to format to the coordinating node, could we pass along the formatted value (formatted on each shard using the mapper), but to the coordinating node that is just a black box string that is read

I tried to do this but this ended up making merging top docs on the coordinating node more complicated, since each shard would have its TopDocs and Object[][] for sort values (one Object[] per ScoreDoc), then we would call Lucene's TopDocs.merge to compute the top hits, and then we would have to associate each ScoreDoc object back to the rigth Object[]. Since it was making things more complicated, I gave up on this idea, what do you think?

[jpountz]

@rjernst Any opinions about the comment above?

[rjernst]

This looks good, thanks for the the change to printable sort values, I think the inet addresses look much better with the minimized format. We can revisit whether/how to simplify (ie removing SortAndFormats) depending on what happens with #17965.