Honor topvalue while determining isMissingvalueCompetitive in case bottom is not set

[gashutos]

Description

There is a bug introduced with #12334 when field contains missing value and if those missing values are competitive.
isMissingValueCompetitive() implementation gets invoked comparing missingValue with bottom when bottom is not even set when After is set.

We should check with topValue in cases when bottom is not set. This PR adds that check.
Added tests for this scenario as well.

Lucene issue

#12521

[gashutos]

This was observed here -> opensearch-project/OpenSearch#9537

[jpountz]

Thanks for catching this bug, the change looks correct to me.

[backslasht]

Looks good, can you add a test for missing bottom minimum value (for desc sort) as well?

[gashutos]

Looks good, can you add a test for missing bottom value as well?

Thanks @backslasht .
Yes the missing value comparison with bottom (as missing value) is in this test already.

lucene/lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java

Line 219 in 34523ec

{ // test that optimization is not run when missing value setting of SortField is competitive

The test missing was, missing value comparison with top value when bottom is not set and top value is coming as missing value which I added.

[backslasht]

Looks good, can you add a test for missing bottom minimum value for desc sort as well?

Thanks @backslasht . Yes the missing value comparison with bottom (as missing value) is in this test already.

lucene/lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java

Line 219 in 34523ec

{ // test that optimization is not run when missing value setting of SortField is competitive

The test missing was, missing value comparison with top value when bottom is not set and top value is coming as missing value which I added.

Sorry, I jumbled the terminologies, I meant a test with missing minimum value for desc sort.

[gashutos]

Looks good, can you add a test for missing bottom minimum value for desc sort as well?

Thanks @backslasht . Yes the missing value comparison with bottom (as missing value) is in this test already.

lucene/lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java

Line 219 in 34523ec

{ // test that optimization is not run when missing value setting of SortField is competitive

The test missing was, missing value comparison with top value when bottom is not set and top value is coming as missing value which I added.

Sorry, I jumbled the terminologies, I meant a test with missing minimum value for desc sort.

Got it, makes sense. Added the test. I kept missing value as Long.MAX_VALUE only for this, it doesnt really matter for our logic for checking its competitiveness.

[backslasht]

Thanks @gashutos for making the changes.

[jpountz]

I'll merge next week if nobody beats me to it.

[jpountz]

Thanks @gashutos !