Ensure integer sorts are rewritten to long sorts for BWC indexes by romseygeek · Pull Request #139293 · elastic/elasticsearch

romseygeek · 2025-12-10T11:28:23Z

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

…ibility

elasticsearchmachine · 2025-12-10T11:28:47Z

Pinging @elastic/es-search (Team:Search)

john-wagster · 2025-12-10T14:04:14Z

Fixes: #139293

elasticsearchmachine · 2025-12-10T16:15:49Z

Hi @romseygeek, I've created a changelog YAML for you.

benwtrent · 2025-12-10T16:17:25Z

@romseygeek I think this might also fix this issue: #139275

We saw this NPE on an index created before (thus sorting on long) and then sorting with a new index (sorting on int).

benwtrent · 2025-12-10T16:17:52Z

Consequently, what do you think about backporting to 9.2.x (and maybe 9.1.x?)?

romseygeek · 2025-12-10T16:49:56Z

This specific bug is only in 9.3 I think, although the fix may also clean up other issues. There have been several changes in IndexNumericFieldData which means this won't be trivial to backport.

romseygeek · 2025-12-10T16:51:16Z

which means this won't be trivial to backport.

To be clear, that doesn't mean we shouldn't do it! Just that it probably needs to be a separate PR and a different test setup.

mayya-sharipova · 2025-12-10T17:18:52Z

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

        return FieldSortBuilder.fromXContent(parser, fieldName);
    }
+
+    public void testIntRewritesToLong() throws IOException {


very nice test!

mayya-sharipova · 2025-12-10T17:22:01Z

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

-        );
+
        XFieldComparatorSource longSource = comparatorSource(NumericType.LONG, missingValue, sortMode, nested);
+        if (sortField instanceof SortedNumericSortField snsf) {


For my education, under what condition index sort is not instance of SortedNumericSortField?

Is it something new?

Index sorts are always SortedNumericSortField instances, but query-time sorts now generally use custom comparators to pick up some performance improvements that are not in a released lucene version, so they are plain SortField instances.

mayya-sharipova

@romseygeek Thanks for a fix!

mayya-sharipova · 2025-12-10T17:25:21Z

I think this might also fix this issue: #139275

This specific issue originally involved scroll, so I am not sure.
I will need to check this.

romseygeek · 2025-12-11T09:17:20Z

I'll backport the test to 9.2 and see if it surfaces any issues.

romseygeek · 2025-12-11T10:24:31Z

I opened #139358 to backport the tests, which seem to pass at the moment so I think #139275 must be a different issue.

* upstream/main: (79 commits) Mute org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT test {p0=search/140_pre_filter_search_shards/prefilter on non-indexed date fields} elastic#139381 Adjust error bounds for bfloat16 value checks (elastic#139371) Unmute some vector CSS tests (elastic#139370) Do not allow `project_routing` as a query param (elastic#139206) Unmute HalfFloat...Tests#testSynthesizeArrayRandom (elastic#139341) Remove leniency in LinkedProjectConfig builder methods (elastic#139012) EQL: fix project_routing (elastic#139366) Add patch version for 9.2 index version constant (elastic#139362) Mute org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT test {p0=search.vectors/200_dense_vector_docvalue_fields/dense_vector docvalues with bfloat16} elastic#139368 ES|QL: Enable CCS tests for FORK (elastic#139302) Restructuring the semantic_text field type page (elastic#138571) AggregateMetricDouble fields should not build BKD indexes (elastic#138724) Feature/count by trunc with filter (elastic#138765) ESQL: Convert TS 500 error to 400 (elastic#139360) [CI] Rerun failing tests for periodic build pipelines (elastic#139200) revert muting saml test (elastic#139327) Add TDigest histogram as metric (elastic#139247) Links solved bugs to class cast exception changelog and unmutes errors (elastic#139340) Ensure integer sorts are rewritten to long sorts for BWC indexes (elastic#139293) Integrate stored fields format bloom filter with synthetic _id (elastic#138515) ...

Backports tests from #139293

Sorts against integer fields in pre-8.19 indexes are rewritten to use LongComparators to stay consistent with pre-written index metadata. These also need to always expect multi-valued fields. elastic#139293 incorrectly built single-valued sorts in certain circumstances, which would then throw runtime errors.

Sorts against integer fields in pre-8.19 indexes are rewritten to use LongComparators to stay consistent with pre-written index metadata. These also need to always expect multi-valued fields. #139293 incorrectly built single-valued sorts in certain circumstances, which would then throw runtime errors.

…39538) Sorts against integer fields in pre-8.19 indexes are rewritten to use LongComparators to stay consistent with pre-written index metadata. These also need to always expect multi-valued fields. elastic#139293 incorrectly built single-valued sorts in certain circumstances, which would then throw runtime errors.

…139700) Sorts against integer fields in pre-8.19 indexes are rewritten to use LongComparators to stay consistent with pre-written index metadata. These also need to always expect multi-valued fields. #139293 incorrectly built single-valued sorts in certain circumstances, which would then throw runtime errors.

Ensure integer sorts are rewritten to long sorts for backwards compat…

f680d10

…ibility

romseygeek self-assigned this Dec 10, 2025

romseygeek added >non-issue :Search/Search Search-related issues that do not fall into other categories v9.3.0 labels Dec 10, 2025

elasticsearchmachine added the Team:Search Meta label for search team label Dec 10, 2025

romseygeek and others added 5 commits December 10, 2025 11:30

unmute

b718f14

spotless

23f11e6

Handle STRING null missing value

e220c86

edge cases

26b69ac

[CI] Auto commit changes from spotless

576487d

romseygeek requested a review from mayya-sharipova December 10, 2025 13:12

benwtrent added >bug and removed >non-issue labels Dec 10, 2025

Update docs/changelog/139293.yaml

3c49fb9

mayya-sharipova reviewed Dec 10, 2025

View reviewed changes

mayya-sharipova approved these changes Dec 10, 2025

View reviewed changes

Merge branch 'main' into bug/sort-rewrite-int-to-long

8fb71c6

romseygeek added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Dec 11, 2025

romseygeek mentioned this pull request Dec 11, 2025

Add test for int-to-long sort BWC conversion #139358

Merged

elasticsearchmachine merged commit cba3128 into elastic:main Dec 11, 2025
34 checks passed

romseygeek deleted the bug/sort-rewrite-int-to-long branch December 11, 2025 10:24

romseygeek added a commit that referenced this pull request Dec 12, 2025

Add test for int-to-long sort BWC conversion (#139358)

5738aa9

Backports tests from #139293

romseygeek mentioned this pull request Dec 15, 2025

Rewritten integer sorts need to use SortedNumericSortField #139538

Merged

romseygeek mentioned this pull request Dec 17, 2025

Rewritten integer sorts need to use SortedNumericSortField (#139538) #139700

Merged

thecoop mentioned this pull request Jan 15, 2026

Using LONG vs INT for Sorting Breaks Across Index Versions #139275

Closed

Conversation

romseygeek commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Dec 10, 2025

Uh oh!

john-wagster commented Dec 10, 2025

Uh oh!

elasticsearchmachine commented Dec 10, 2025

Uh oh!

benwtrent commented Dec 10, 2025

Uh oh!

benwtrent commented Dec 10, 2025

Uh oh!

romseygeek commented Dec 10, 2025

Uh oh!

romseygeek commented Dec 10, 2025

Uh oh!

mayya-sharipova Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

mayya-sharipova Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

romseygeek Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

mayya-sharipova left a comment

Choose a reason for hiding this comment

Uh oh!

mayya-sharipova commented Dec 10, 2025

Uh oh!

romseygeek commented Dec 11, 2025

Uh oh!

Uh oh!

romseygeek commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

romseygeek commented Dec 10, 2025 •

edited

Loading