Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken backward compatibility from 2.7 for IndexSorted field indices #10045

Merged
merged 13 commits into from
Sep 15, 2023
Merged
Prev Previous commit
Next Next commit
Correcting typos and adding more tests
Signed-off-by: gashutos <gashutos@amazon.com>
  • Loading branch information
gashutos committed Sep 15, 2023
commit 61d355ed9949bd430cf14b1582d0343762c4ee4b
Original file line number Diff line number Diff line change
Expand Up @@ -860,9 +860,9 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY));
defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE);
/* There was unintentional breaking change got introduced with [OpenSearch-6424](https://github.com/opensearch-project/OpenSearch/pull/6424) (version 2.7).
* For indices created prior version (prior to 2.7) which has IndexSort type, they used to type caste the SortField.Type
* For indices created prior version (prior to 2.7) which has IndexSort type, they used to type cast the SortField.Type
* to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to
* up caste the SortField to gain some sort query optimizations.
* up cast the SortField to gain some sort query optimizations.
* Now this sortField (IndexSort) is stored in SegmentInfo and we need to maintain backward compatibility for them.
*/
widenIndexSortType = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).before(V_2_7_0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private static MultiValueMode parseMultiValueMode(String value) {

// visible for tests
final FieldSortSpec[] sortSpecs;
final boolean shouldWidenIndexSortTpe;
final boolean shouldWidenIndexSortType;

public IndexSortConfig(IndexSettings indexSettings) {
final Settings settings = indexSettings.getSettings();
Expand Down Expand Up @@ -183,7 +183,7 @@ public IndexSortConfig(IndexSettings indexSettings) {
sortSpecs[i].missingValue = missingValues.get(i);
}
}
this.shouldWidenIndexSortTpe = indexSettings.shouldWidenIndexSortType();
this.shouldWidenIndexSortType = indexSettings.shouldWidenIndexSortType();
}

/**
Expand Down Expand Up @@ -232,7 +232,7 @@ public Sort buildIndexSort(
if (fieldData == null) {
throw new IllegalArgumentException("docvalues not found for index sort field:[" + sortSpec.field + "]");
}
if (this.shouldWidenIndexSortTpe == true) {
if (this.shouldWidenIndexSortType == true) {
sortFields[i] = fieldData.wideSortField(sortSpec.missingValue, mode, null, reverse);
} else {
sortFields[i] = fieldData.sortField(sortSpec.missingValue, mode, null, reverse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public final SortField wideSortField(Object missingValue, MultiValueMode sortMod
// This is to support backward compatibility, the minimum number of bytes prior to OpenSearch 2.7 were 16 bytes,
// i.e all sort fields were upcasted to Long/Double with 16 bytes.
// Now from OpenSearch 2.7, the minimum number of bytes for sort field is 8 bytes, so if it comes as SortField INT,
// we need to up caste it to LONG to support backward compatibility info stored in segment info
// we need to up cast it to LONG to support backward compatibility info stored in segment info
if (getNumericType().sortFieldType == SortField.Type.INT) {
gashutos marked this conversation as resolved.
Show resolved Hide resolved
gashutos marked this conversation as resolved.
Show resolved Hide resolved
XFieldComparatorSource source = comparatorSource(NumericType.LONG, missingValue, sortMode, nested);
SortedNumericSelector.Type selectorType = sortMode == MultiValueMode.MAX
Expand All @@ -166,7 +166,7 @@ public final SortField wideSortField(Object missingValue, MultiValueMode sortMod
sortField.setMissingValue(source.missingObject(missingValue, reverse));
return sortField;
}
// If already more than INT, up caste not needed.
// If already more than INT, up cast not needed.
return sortField(getNumericType(), missingValue, sortMode, nested, reverse);
}

Expand Down Expand Up @@ -243,7 +243,7 @@ private XFieldComparatorSource comparatorSource(
source = new IntValuesComparatorSource(this, missingValue, sortMode, nested);
}
if (targetNumericType != getNumericType()) {
source.disableSkipping(); // disable skipping logic for caste of sort field
source.disableSkipping(); // disable skipping logic for cast of sort field
}
return source;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,14 @@ public void testIndexSort() {
index = createIndex("test", settings, createTestMapping("long"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG);

// Float index sort should be remained to float sort type
index = createIndex("test", settings, createTestMapping("float"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.FLOAT);

// Double index sort should be remained to double sort type
index = createIndex("test", settings, createTestMapping("double"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.DOUBLE);

// String index sort should be remained to string sort type
index = createIndex("test", settings, createTestMapping("string"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING);
Expand All @@ -566,6 +574,14 @@ public void testIndexSortBackwardCompatible() {
index = createIndex("test", settings, createTestMapping("long"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.LONG);

// Float index sort should be remained to float sort type
index = createIndex("test", settings, createTestMapping("float"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.FLOAT);

// Double index sort should be remained to double sort type
index = createIndex("test", settings, createTestMapping("double"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.DOUBLE);

// String index sort should be remained to string sort type
index = createIndex("test", settings, createTestMapping("string"));
assertTrue(index.getIndexSortSupplier().get().getSort()[0].getType() == SortField.Type.STRING);
Expand Down