Fix field mapping updates with similarity (#33634)

This change fixes a bug introduced in 6.3 that prevents fields with an explicit
similarity to be updated. It also adds a test that checks this case for similarities
but also for analyzers since they could suffer from the same problem.

Closes #33611
This commit is contained in:
Jim Ferenczi 2018-09-13 09:21:27 +02:00 committed by GitHub
parent 5a3fd8e4e7
commit 6ca36bba15
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 50 additions and 12 deletions

View File

@ -111,17 +111,6 @@ public abstract class MappedFieldType extends FieldType {
public boolean equals(Object o) {
if (!super.equals(o)) return false;
MappedFieldType fieldType = (MappedFieldType) o;
// check similarity first because we need to check the name, and it might be null
// TODO: SimilarityProvider should have equals?
if (similarity == null || fieldType.similarity == null) {
if (similarity != fieldType.similarity) {
return false;
}
} else {
if (Objects.equals(similarity.name(), fieldType.similarity.name()) == false) {
return false;
}
}
return boost == fieldType.boost &&
docValues == fieldType.docValues &&
@ -131,7 +120,8 @@ public abstract class MappedFieldType extends FieldType {
Objects.equals(searchQuoteAnalyzer(), fieldType.searchQuoteAnalyzer()) &&
Objects.equals(eagerGlobalOrdinals, fieldType.eagerGlobalOrdinals) &&
Objects.equals(nullValue, fieldType.nullValue) &&
Objects.equals(nullValueAsString, fieldType.nullValueAsString);
Objects.equals(nullValueAsString, fieldType.nullValueAsString) &&
Objects.equals(similarity, fieldType.similarity);
}
@Override

View File

@ -21,6 +21,8 @@ package org.elasticsearch.index.similarity;
import org.apache.lucene.search.similarities.Similarity;
import java.util.Objects;
/**
* Wrapper around a {@link Similarity} and its name.
*/
@ -48,4 +50,28 @@ public final class SimilarityProvider {
return similarity;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
SimilarityProvider that = (SimilarityProvider) o;
/**
* We check <code>name</code> only because the <code>similarity</code> is
* re-created for each new instance and they don't implement equals.
* This is not entirely correct though but we only use equality checks
* for similarities inside the same index and names are unique in this case.
**/
return Objects.equals(name, that.name);
}
@Override
public int hashCode() {
/**
* We use <code>name</code> only because the <code>similarity</code> is
* re-created for each new instance and they don't implement equals.
* This is not entirely correct though but we only use equality checks
* for similarities a single index and names are unique in this case.
**/
return Objects.hash(name);
}
}

View File

@ -89,6 +89,17 @@ public abstract class FieldTypeTestCase extends ESTestCase {
other.setIndexAnalyzer(new NamedAnalyzer("foo", AnalyzerScope.INDEX, new StandardAnalyzer()));
}
},
// check that we can update if the analyzer is unchanged
new Modifier("analyzer", true) {
@Override
public void modify(MappedFieldType ft) {
ft.setIndexAnalyzer(new NamedAnalyzer("foo", AnalyzerScope.INDEX, new StandardAnalyzer()));
}
@Override
public void normalizeOther(MappedFieldType other) {
other.setIndexAnalyzer(new NamedAnalyzer("foo", AnalyzerScope.INDEX, new StandardAnalyzer()));
}
},
new Modifier("search_analyzer", true) {
@Override
public void modify(MappedFieldType ft) {
@ -137,6 +148,17 @@ public abstract class FieldTypeTestCase extends ESTestCase {
other.setSimilarity(new SimilarityProvider("bar", new BM25Similarity()));
}
},
// check that we can update if the similarity is unchanged
new Modifier("similarity", true) {
@Override
public void modify(MappedFieldType ft) {
ft.setSimilarity(new SimilarityProvider("foo", new BM25Similarity()));
}
@Override
public void normalizeOther(MappedFieldType other) {
other.setSimilarity(new SimilarityProvider("foo", new BM25Similarity()));
}
},
new Modifier("eager_global_ordinals", true) {
@Override
public void modify(MappedFieldType ft) {