Maybe instead of fixing NumericDVSorter, we can have a ReverseSortSorter (must have a better name!) which wraps any Sorter and reverses the DocMap?

That's another option. I have no strong feeling towards any of them.

I hacked this up real quickly, so I could be missing something. Patch adds a ReverseOrderSorter which wraps a Sorter and on sort() returns a DocMap that reverses whatever the wrapped Sorter DocMap returned.

I still didn't figure out how to plug that sorter with existing tests, so it could be this approach doesn't work. Will look at it later.

Added ReverseOrderSorter to IndexSortingTest (was after all very easy), which uncovered a bug in my original implementation. It's now working and tests are happy.

I basically think this is ready, would appreciate some review.

Patch on latest trunk (previous one had issues applying).

We can add this ReverseOrderSorter, but as far as NumericDocValuesSorter is concerned, I would rather have the abstraction at the level of the DocComparator rather than the Sorter. This would allow Sorter.sort(int,DocComparator) to quickly return null without allocating (potentially lots of) memory for the doc maps if the reader is already sorted. Additionally, this would allow for more readable diagnostics (such as "DocValues(fieldName,desc)" instead of "Reverse(DocValues(fieldName,asc))".

Fair enough. It was quite easy to modify NumericDVSorter. Patch adds another ctor which takes a boolean ascending, and integrates it with the test.

This got me thinking if ascending/descending should be on the Sorter.sort API, but I think it shouldn't because someone can dangerously sort some segments in ascending and others in descending order. Better, I think, if it's a consistent decision for all segments. What do you think?

This got me thinking if ascending/descending should be on the Sorter.sort API

I think it shouldn't for the reasons you mentioned.

The patch looks good to me, +1 to commit!

I am also wondering if I should keep ReverseOrderSorter, because implementing reverse in numeric is so trivial, that ReverseOrderSorter becomes just another sorter to maintain... I think I'll remove it

It is OK for me.

Committed to trunk and 4x.

Closed after release.