Browse Source

[8.19.1] Fix default missing index sort value of data_nanos pre 7.14 (#132172)

This commit fixes an issue whereby indices created prior to 7.14 that have an index sort on a date_nanos field can no longer be opened (with versions >= 7.14). When opening an index the configured index sort, derived from the index settings, must match exactly that of the sort encoded in the index itself. A change to fix a bug back in 7.14 changed this for date_nanos fields whose index.sort.missing value is absent, see #74760. Specifically, the default minimum value changed from Long.MIN_VALUE to 0L.

The change in this commit restores the default minimum value for indices prior to 7.14.
Chris Hegarty 2 months ago
parent
commit
cab22b373c

+ 6 - 0
docs/changelog/132162.yaml

@@ -0,0 +1,6 @@
+pr: 132162
+summary: Fix default missing index sort value of `data_nanos` pre 7.14
+area: Search
+type: bug
+issues:
+ - 132040

+ 28 - 0
server/src/internalClusterTest/java/org/elasticsearch/index/IndexSortIT.java

@@ -11,6 +11,7 @@ package org.elasticsearch.index;
 
 
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.SortField;
+import org.apache.lucene.search.SortedNumericSelector;
 import org.apache.lucene.search.SortedNumericSortField;
 import org.apache.lucene.search.SortedNumericSortField;
 import org.apache.lucene.search.SortedSetSortField;
 import org.apache.lucene.search.SortedSetSortField;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.Settings;
@@ -80,6 +81,33 @@ public class IndexSortIT extends ESIntegTestCase {
         assertSortedSegments("test", indexSort);
         assertSortedSegments("test", indexSort);
     }
     }
 
 
+    public void testIndexSortDateNanos() {
+        prepareCreate("test").setSettings(
+            Settings.builder()
+                .put(indexSettings())
+                .put("index.number_of_shards", "1")
+                .put("index.number_of_replicas", "1")
+                .put("index.sort.field", "@timestamp")
+                .put("index.sort.order", "desc")
+        ).setMapping("""
+            {
+              "properties": {
+                "@timestamp": {
+                  "type": "date_nanos"
+                }
+              }
+            }
+            """).get();
+
+        flushAndRefresh();
+        ensureYellow();
+
+        SortField sf = new SortedNumericSortField("@timestamp", SortField.Type.LONG, true, SortedNumericSelector.Type.MAX);
+        sf.setMissingValue(0L);
+        Sort expectedIndexSort = new Sort(sf);
+        assertSortedSegments("test", expectedIndexSort);
+    }
+
     public void testInvalidIndexSort() {
     public void testInvalidIndexSort() {
         IllegalArgumentException exc = expectThrows(
         IllegalArgumentException exc = expectThrows(
             IllegalArgumentException.class,
             IllegalArgumentException.class,

+ 12 - 3
server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java

@@ -144,10 +144,19 @@ public abstract class IndexNumericFieldData implements IndexFieldData<LeafNumeri
         boolean reverse
         boolean reverse
     ) {
     ) {
         SortField sortField = sortField(missingValue, sortMode, nested, reverse);
         SortField sortField = sortField(missingValue, sortMode, nested, reverse);
-        if (indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19)
-            || getNumericType().sortFieldType != SortField.Type.INT) {
+        if (getNumericType() == NumericType.DATE_NANOSECONDS
+            && indexCreatedVersion.before(IndexVersions.V_7_14_0)
+            && missingValue == null
+            && Long.valueOf(0L).equals(sortField.getMissingValue())) {
+            // 7.14 changed the default missing value of sort on date_nanos, from Long.MIN_VALUE
+            // to 0L - for compatibility we require to a missing value of MIN_VALUE to allow to
+            // open the index.
+            sortField.setMissingValue(Long.MIN_VALUE);
             return sortField;
             return sortField;
-        }
+        } else if (indexCreatedVersion.onOrAfter(IndexVersions.INDEX_INT_SORT_INT_TYPE_8_19)
+            || getNumericType().sortFieldType != SortField.Type.INT) {
+                return sortField;
+            }
         if ((sortField instanceof SortedNumericSortField) == false) {
         if ((sortField instanceof SortedNumericSortField) == false) {
             return sortField;
             return sortField;
         }
         }

+ 76 - 0
server/src/test/java/org/elasticsearch/index/IndexSortSettingsTests.java

@@ -13,6 +13,7 @@ import org.apache.lucene.search.Query;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.Sort;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.index.fielddata.FieldDataContext;
 import org.elasticsearch.index.fielddata.FieldDataContext;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
@@ -30,9 +31,12 @@ import org.elasticsearch.search.MultiValueMode;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.ESTestCase;
 
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Set;
 import java.util.Set;
+import java.util.stream.Stream;
 
 
 import static org.elasticsearch.index.IndexSettingsTests.newIndexMeta;
 import static org.elasticsearch.index.IndexSettingsTests.newIndexMeta;
 import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.arrayWithSize;
@@ -174,6 +178,58 @@ public class IndexSortSettingsTests extends ESTestCase {
         );
         );
     }
     }
 
 
+    public void testSortMissingValueDateNanoFieldPre714() {
+        MappedFieldType tsField = new DateFieldMapper.DateFieldType("@timestamp", true, DateFieldMapper.Resolution.NANOSECONDS);
+        var indexSettingsBuilder = Settings.builder();
+        indexSettingsBuilder.put("index.sort.field", "@timestamp");
+        indexSettingsBuilder.put("index.sort.order", "desc");
+
+        // test with index version 7.13 and before
+        var pre714Versions = Stream.concat(Stream.of(IndexVersions.V_7_13_0), randomVersionsBefore(IndexVersions.V_7_13_0)).toList();
+        for (var version : pre714Versions) {
+            indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, version);
+            Sort sort = buildIndexSort(indexSettings(indexSettingsBuilder.build()), Map.of("@timestamp", tsField));
+            assertThat(sort.getSort(), arrayWithSize(1));
+            assertThat(sort.getSort()[0].getField(), equalTo("@timestamp"));
+            assertThat(sort.getSort()[0].getMissingValue(), equalTo(Long.MIN_VALUE));
+        }
+
+        // now test with index version 7.14 and after
+        var post713Versions = Stream.concat(Stream.of(IndexVersions.V_7_14_0), randomVersionsAfter(IndexVersions.V_7_14_0)).toList();
+        for (var version : post713Versions) {
+            indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, version);
+            Sort sort = buildIndexSort(indexSettings(indexSettingsBuilder.build()), Map.of("@timestamp", tsField));
+            assertThat(sort.getSort(), arrayWithSize(1));
+            assertThat(sort.getSort()[0].getField(), equalTo("@timestamp"));
+            assertThat(sort.getSort()[0].getMissingValue(), equalTo(0L));
+        }
+
+        // asc order has not changed behaviour in any version
+        indexSettingsBuilder.put("index.sort.order", "asc");
+        var allVersions = Stream.concat(post713Versions.stream(), pre714Versions.stream()).toList();
+        for (var version : allVersions) {
+            indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, version);
+            Sort sort = buildIndexSort(indexSettings(indexSettingsBuilder.build()), Map.of("@timestamp", tsField));
+            assertThat(sort.getSort(), arrayWithSize(1));
+            assertThat(sort.getSort()[0].getField(), equalTo("@timestamp"));
+            assertThat(sort.getSort()[0].getMissingValue(), equalTo(DateUtils.MAX_NANOSECOND));
+        }
+
+        // ensure no change in behaviour when a missing value is set
+        indexSettingsBuilder.put("index.sort.missing", "_first");
+        for (var version : allVersions) {
+            indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, version);
+            Sort sort = buildIndexSort(indexSettings(indexSettingsBuilder.build()), Map.of("@timestamp", tsField));
+            assertThat(sort.getSort()[0].getMissingValue(), equalTo(0L));
+        }
+        indexSettingsBuilder.put("index.sort.missing", "_last");
+        for (var version : allVersions) {
+            indexSettingsBuilder.put(IndexMetadata.SETTING_VERSION_CREATED, version);
+            Sort sort = buildIndexSort(indexSettings(indexSettingsBuilder.build()), Map.of("@timestamp", tsField));
+            assertThat(sort.getSort()[0].getMissingValue(), equalTo(Long.MAX_VALUE));
+        }
+    }
+
     public void testTimeSeriesMode() {
     public void testTimeSeriesMode() {
         IndexSettings indexSettings = indexSettings(
         IndexSettings indexSettings = indexSettings(
             Settings.builder()
             Settings.builder()
@@ -224,4 +280,24 @@ public class IndexSortSettingsTests extends ESTestCase {
             )
             )
         );
         );
     }
     }
+
+    /* Returns a stream of versions before the given version */
+    Stream<IndexVersion> randomVersionsBefore(IndexVersion indexVersion) {
+        var versions = IndexVersions.getAllVersions().stream().filter(v -> v.before(indexVersion)).toList();
+        List<IndexVersion> ret = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            ret.add(randomValueOtherThanMany(ret::contains, () -> randomFrom(versions)));
+        }
+        return ret.stream();
+    }
+
+    /* Returns a stream of versions after the given version */
+    Stream<IndexVersion> randomVersionsAfter(IndexVersion indexVersion) {
+        var versions = IndexVersions.getAllVersions().stream().filter(v -> v.after(indexVersion)).toList();
+        List<IndexVersion> ret = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            ret.add(randomValueOtherThanMany(ret::contains, () -> randomFrom(versions)));
+        }
+        return ret.stream();
+    }
 }
 }