Skip to content

Commit cba3128

Browse files
authored
Ensure integer sorts are rewritten to long sorts for BWC indexes (#139293)
Older indexes used long sorts for integer fields, and we need to ensure that sorts against these fields do not conflict. Recent refactoring in IndexNumericSortField missed a case where this could happen. Closes #139127 Closes #139128
1 parent a0ee98c commit cba3128

File tree

6 files changed

+87
-31
lines changed

6 files changed

+87
-31
lines changed

docs/changelog/139293.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 139293
2+
summary: Ensure integer sorts are rewritten to long sorts for BWC indexes
3+
area: Search
4+
type: bug
5+
issues:
6+
- 139127
7+
- 139128

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,6 @@ tests:
433433
- class: org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT
434434
method: test {p0=search.highlight/50_synthetic_source/text multi unified from vectors}
435435
issue: https://github.com/elastic/elasticsearch/issues/139066
436-
- class: org.elasticsearch.upgrades.IndexSortUpgradeIT
437-
method: testIndexSortForNumericTypes {upgradedNodes=1}
438-
issue: https://github.com/elastic/elasticsearch/issues/139127
439-
- class: org.elasticsearch.upgrades.IndexSortUpgradeIT
440-
method: testIndexSortForNumericTypes {upgradedNodes=2}
441-
issue: https://github.com/elastic/elasticsearch/issues/139128
442436
- class: org.elasticsearch.xpack.search.AsyncSearchConcurrentStatusIT
443437
method: testConcurrentStatusFetchWhileTaskCloses
444438
issue: https://github.com/elastic/elasticsearch/issues/138543

server/src/main/java/org/elasticsearch/common/lucene/Lucene.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ public static void writeSortType(StreamOutput out, SortField.Type sortType) thro
636636
* Returns the generic version of the provided {@link SortField} that
637637
* can be used to merge documents coming from different shards.
638638
*/
639-
private static SortField rewriteMergeSortField(SortField sortField) {
639+
public static SortField rewriteMergeSortField(SortField sortField) {
640640
if (sortField.getClass() == GEO_DISTANCE_SORT_TYPE_CLASS) {
641641
SortField newSortField = new SortField(sortField.getField(), SortField.Type.DOUBLE);
642642
newSortField.setMissingValue(sortField.getMissingValue());
@@ -651,6 +651,13 @@ private static SortField rewriteMergeSortField(SortField sortField) {
651651
return newSortField;
652652
} else if (sortField.getClass() == ShardDocSortField.class) {
653653
return new SortField(sortField.getField(), SortField.Type.LONG, sortField.getReverse());
654+
} else if (sortField.getComparatorSource() instanceof IndexFieldData.XFieldComparatorSource fcs) {
655+
SortField newSortField = new SortField(sortField.getField(), fcs.reducedType(), sortField.getReverse());
656+
Object missingValue = fcs.missingValue(sortField.getReverse());
657+
if (missingValue != null) {
658+
newSortField.setMissingValue(missingValue);
659+
}
660+
return newSortField;
654661
} else {
655662
return sortField;
656663
}
@@ -659,15 +666,8 @@ private static SortField rewriteMergeSortField(SortField sortField) {
659666
static void writeSortField(StreamOutput out, SortField sortField) throws IOException {
660667
sortField = rewriteMergeSortField(sortField);
661668
out.writeOptionalString(sortField.getField());
662-
if (sortField.getComparatorSource() != null) {
663-
IndexFieldData.XFieldComparatorSource comparatorSource = (IndexFieldData.XFieldComparatorSource) sortField
664-
.getComparatorSource();
665-
writeSortType(out, comparatorSource.reducedType());
666-
writeMissingValue(out, comparatorSource.missingValue(sortField.getReverse()));
667-
} else {
668-
writeSortType(out, sortField.getType());
669-
writeMissingValue(out, sortField.getMissingValue());
670-
}
669+
writeSortType(out, sortField.getType());
670+
writeMissingValue(out, sortField.getMissingValue());
671671
out.writeBoolean(sortField.getReverse());
672672
}
673673

server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ public SortField sortField(
157157
boolean reverse
158158
) {
159159
SortField sortField = sortField(indexSort, getNumericType(), missingValue, sortMode, nested, reverse);
160+
160161
if (getNumericType() == NumericType.DATE_NANOSECONDS
161162
&& indexCreatedVersion.before(IndexVersions.V_7_14_0)
162163
&& missingValue.equals("_last")
@@ -166,35 +167,42 @@ public SortField sortField(
166167
// open the index.
167168
sortField.setMissingValue(Long.MIN_VALUE);
168169
return sortField;
169-
} else if (getNumericType().sortFieldType != SortField.Type.INT
170+
}
171+
172+
if (getNumericType().sortFieldType != SortField.Type.INT
170173
// we introduced INT sort type in 8.19 and from 9.1
171174
|| indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE)
172175
|| indexCreatedVersion.between(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19, UPGRADE_TO_LUCENE_10_0_0)) {
173-
return sortField;
174-
}
175-
if ((sortField instanceof SortedNumericSortField) == false) {
176176
return sortField;
177177
}
178+
178179
// if the index was created before 8.19, or in 9.0
179180
// we need to rewrite the sort field to use LONG sort type
180-
181-
// Rewrite INT sort to LONG sort.
182181
// Before indices used TYPE.LONG for index sorting on integer field,
183182
// and this is stored in their index writer config on disk and can't be modified.
184183
// Now sortField() returns TYPE.INT when sorting on integer field,
185184
// but to support sorting on old indices, we need to rewrite this sort to TYPE.LONG.
186-
SortedNumericSortField numericSortField = (SortedNumericSortField) sortField;
187-
SortedNumericSortField rewrittenSortField = new SortedNumericSortField(
188-
sortField.getField(),
189-
SortField.Type.LONG,
190-
sortField.getReverse(),
191-
numericSortField.getSelector()
192-
);
185+
193186
XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested);
187+
if (sortField instanceof SortedNumericSortField snsf) {
188+
SortedNumericSortField rewrittenSortField = new SortedNumericSortField(
189+
snsf.getField(),
190+
SortField.Type.LONG,
191+
snsf.getReverse(),
192+
snsf.getSelector()
193+
);
194+
rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse));
195+
// we don't optimize sorting on int field for old indices
196+
rewrittenSortField.setOptimizeSortWithPoints(false);
197+
return rewrittenSortField;
198+
}
199+
200+
SortField rewrittenSortField = new SortField(sortField.getField(), SortField.Type.LONG, reverse);
194201
rewrittenSortField.setMissingValue(longSource.missingObject(missingValue, reverse));
195202
// we don't optimize sorting on int field for old indices
196203
rewrittenSortField.setOptimizeSortWithPoints(false);
197204
return rewrittenSortField;
205+
198206
}
199207

200208
/**

server/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,22 @@ public void testEqualsAndHashcode() {
183183
}
184184

185185
protected final SearchExecutionContext createMockSearchExecutionContext() {
186-
return createMockSearchExecutionContext(null);
186+
return createMockSearchExecutionContext(null, IndexVersion.current());
187+
}
188+
189+
protected final SearchExecutionContext createMockSearchExecutionContext(IndexVersion indexVersion) {
190+
return createMockSearchExecutionContext(null, indexVersion);
187191
}
188192

189193
protected final SearchExecutionContext createMockSearchExecutionContext(IndexSearcher searcher) {
194+
return createMockSearchExecutionContext(searcher, IndexVersion.current());
195+
}
196+
197+
protected final SearchExecutionContext createMockSearchExecutionContext(IndexSearcher searcher, IndexVersion indexVersion) {
190198
Index index = new Index(randomAlphaOfLengthBetween(1, 10), "_na_");
191199
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(
192200
index,
193-
Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()).build()
201+
Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, indexVersion).build()
194202
);
195203
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(idxSettings, mock(BitsetFilterCache.Listener.class));
196204
BiFunction<MappedFieldType, FieldDataContext, IndexFieldData<?>> indexFieldDataLookup = (fieldType, fdc) -> {

server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import org.apache.lucene.tests.index.RandomIndexWriter;
3030
import org.apache.lucene.tests.search.AssertingIndexSearcher;
3131
import org.apache.lucene.util.BytesRef;
32+
import org.elasticsearch.common.lucene.Lucene;
33+
import org.elasticsearch.index.IndexVersion;
34+
import org.elasticsearch.index.IndexVersions;
3235
import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource;
3336
import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
3437
import org.elasticsearch.index.mapper.DateFieldMapper;
@@ -46,6 +49,7 @@
4649
import org.elasticsearch.search.MultiValueMode;
4750
import org.elasticsearch.search.SearchSortValuesAndFormats;
4851
import org.elasticsearch.search.builder.SearchSourceBuilder;
52+
import org.elasticsearch.test.index.IndexVersionUtils;
4953
import org.elasticsearch.xcontent.XContentParseException;
5054
import org.elasticsearch.xcontent.XContentParser;
5155
import org.elasticsearch.xcontent.json.JsonXContent;
@@ -684,4 +688,39 @@ public void testIsBottomSortShardDisjoint() throws Exception {
684688
protected FieldSortBuilder fromXContent(XContentParser parser, String fieldName) throws IOException {
685689
return FieldSortBuilder.fromXContent(parser, fieldName);
686690
}
691+
692+
public void testIntRewritesToLong() throws IOException {
693+
assertIntegerSortRewrite(
694+
IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19),
695+
SortField.Type.LONG
696+
);
697+
assertIntegerSortRewrite(
698+
IndexVersionUtils.randomVersionBetween(
699+
random(),
700+
IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19,
701+
IndexVersionUtils.getPreviousVersion(IndexVersions.UPGRADE_TO_LUCENE_10_0_0)
702+
),
703+
SortField.Type.INT
704+
);
705+
assertIntegerSortRewrite(
706+
IndexVersionUtils.randomVersionBetween(
707+
random(),
708+
IndexVersions.UPGRADE_TO_LUCENE_10_0_0,
709+
IndexVersionUtils.getPreviousVersion(IndexVersions.INDEX_INT_SORT_INT_TYPE)
710+
),
711+
SortField.Type.LONG
712+
);
713+
assertIntegerSortRewrite(
714+
IndexVersionUtils.randomVersionBetween(random(), IndexVersions.INDEX_INT_SORT_INT_TYPE, IndexVersion.current()),
715+
SortField.Type.INT
716+
);
717+
assertIntegerSortRewrite(IndexVersion.current(), SortField.Type.INT);
718+
}
719+
720+
private void assertIntegerSortRewrite(IndexVersion version, SortField.Type expectedType) throws IOException {
721+
FieldSortBuilder builder = new FieldSortBuilder("custom-integer");
722+
SortFieldAndFormat sff = builder.build(createMockSearchExecutionContext(version));
723+
SortField sf = Lucene.rewriteMergeSortField(sff.field());
724+
assertThat(sf.getType(), equalTo(expectedType));
725+
}
687726
}

0 commit comments

Comments
 (0)