Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
  • Loading branch information
@reta
reta committed Jan 31, 2024
1 parent 7c0ad5a commit b8ec474
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 57 deletions.
122 changes: 65 additions & 57 deletions server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,8 @@ public void testIssue2991() throws InterruptedException {
}

public void testSimpleSorts() throws Exception {
final int docs = 50;

Random random = random();
assertAcked(
prepareCreate("test").setMapping(
Expand Down Expand Up @@ -623,9 +625,9 @@ public void testSimpleSorts() throws Exception {
)
);
ensureGreen();
BigInteger UNSIGNED_LONG_BASE = Numbers.MAX_UNSIGNED_LONG_VALUE.subtract(BigInteger.valueOf(100000));
BigInteger UNSIGNED_LONG_BASE = Numbers.MAX_UNSIGNED_LONG_VALUE.subtract(BigInteger.valueOf(10000 * docs));
List<IndexRequestBuilder> builders = new ArrayList<>();
for (int i = 0; i < 10; i++) {
for (int i = 0; i < docs; i++) {
IndexRequestBuilder builder = client().prepareIndex("test")
.setId(Integer.toString(i))
.setSource(
Expand Down Expand Up @@ -660,13 +662,13 @@ public void testSimpleSorts() throws Exception {
indexRandomForConcurrentSearch("test");

// STRING
int size = 1 + random.nextInt(10);
int size = 1 + random.nextInt(docs);
SearchResponse searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.setSize(size)
.addSort("str_value", SortOrder.ASC)
.get();
assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
Expand All @@ -675,192 +677,198 @@ public void testSimpleSorts() throws Exception {
equalTo(new String(new char[] { (char) (97 + i), (char) (97 + i) }))
);
}
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("str_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(
searchResponse.getHits().getAt(i).getSortValues()[0].toString(),
equalTo(new String(new char[] { (char) (97 + (9 - i)), (char) (97 + (9 - i)) }))
equalTo(new String(new char[] { (char) (97 + (docs - 1 - i)), (char) (97 + (docs - 1 - i)) }))
);
}

assertThat(searchResponse.toString(), not(containsString("error")));

// BYTE
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("byte_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).byteValue(), equalTo((byte) i));
}
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("byte_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).byteValue(), equalTo((byte) (9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).byteValue(), equalTo((byte) (docs - 1 - i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));

// SHORT
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("short_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).shortValue(), equalTo((short) i));
}
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("short_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).shortValue(), equalTo((short) (9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).shortValue(), equalTo((short) (docs - 1 - i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));

// INTEGER
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("integer_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).intValue(), equalTo(i));
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("integer_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).intValue(), equalTo((9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).intValue(), equalTo((docs - 1 - i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));

// LONG
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("long_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).longValue(), equalTo((long) i));
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("long_value", SortOrder.DESC).get();
assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, 10);

assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).longValue(), equalTo((long) (9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).longValue(), equalTo((long) (docs - 1 - i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));

// FLOAT
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("float_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(), closeTo(0.1d * i, 0.000001d));
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("float_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(), closeTo(0.1d * (9 - i), 0.000001d));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(
((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(),
closeTo(0.1d * (docs - 1 - i), 0.000001d)
);
}

assertThat(searchResponse.toString(), not(containsString("error")));

// HALF_FLOAT
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("half_float_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("half_float_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
}

assertThat(searchResponse.toString(), not(containsString("error")));

// DOUBLE
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("double_value", SortOrder.ASC).get();

assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(), closeTo(0.1d * i, 0.000001d));
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setSize(size).addSort("double_value", SortOrder.DESC).get();

assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(), closeTo(0.1d * (9 - i), 0.000001d));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(
((Number) searchResponse.getHits().getAt(i).getSortValues()[0]).doubleValue(),
closeTo(0.1d * (docs - 1 - i), 0.000001d)
);
}

assertNoFailures(searchResponse);

// UNSIGNED_LONG
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.setSize(size)
.addSort("unsigned_long_value", SortOrder.ASC)
.get();

assertHitCount(searchResponse, 10);
assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
Expand All @@ -871,20 +879,20 @@ public void testSimpleSorts() throws Exception {
}

assertThat(searchResponse.toString(), not(containsString("error")));
size = 1 + random.nextInt(10);
size = 1 + random.nextInt(docs);
searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.setSize(size)
.addSort("unsigned_long_value", SortOrder.DESC)
.get();
assertHitCount(searchResponse, 10L);
assertHitCount(searchResponse, 10);

assertHitCount(searchResponse, docs);
assertThat(searchResponse.getHits().getHits().length, equalTo(size));
for (int i = 0; i < size; i++) {
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(9 - i)));
assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(Integer.toString(docs - 1 - i)));
assertThat(
((Number) searchResponse.getHits().getAt(i).getSortValues()[0]),
equalTo(UNSIGNED_LONG_BASE.add(BigInteger.valueOf(10000 * (9 - i))))
equalTo(UNSIGNED_LONG_BASE.add(BigInteger.valueOf(10000 * (docs - 1 - i))))
);
}

Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/org/opensearch/index/fielddata/AbstractNumericDocValues.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
* aggregations, which only use {@link #advanceExact(int)} and
* {@link #longValue()}.
*
* In case when optimizations based on point values are used, the {@link #advance(int)}
* and, optionally, {@link #cost()} have to be implemented as well.
*
* @opensearch.internal
*/
public abstract class AbstractNumericDocValues extends NumericDocValues {
Expand Down

0 comments on commit b8ec474

Please sign in to comment.