Skip to content

Commit 57bd949

Browse files
committed
Remove redundant merge validation
1 parent fa22c56 commit 57bd949

File tree

4 files changed

+19
-41
lines changed

4 files changed

+19
-41
lines changed

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapper.java

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ public static class Builder extends FieldMapper.Builder {
211211
mapper -> ((SemanticTextFieldType) mapper.fieldType()).indexOptions,
212212
XContentBuilder::field,
213213
Objects::toString
214-
).acceptsNull().setMergeValidator(SemanticTextFieldMapper::canMergeIndexOptions);
214+
).acceptsNull();
215215

216216
@SuppressWarnings("unchecked")
217217
private final Parameter<ChunkingSettings> chunkingSettings = new Parameter<>(
@@ -293,10 +293,19 @@ protected void merge(FieldMapper mergeWith, Conflicts conflicts, MapperMergeCont
293293
SemanticTextFieldMapper semanticMergeWith = (SemanticTextFieldMapper) mergeWith;
294294
semanticMergeWith = copySettings(semanticMergeWith, mapperMergeContext);
295295

296-
var context = mapperMergeContext.createChildContext(semanticMergeWith.leafName(), ObjectMapper.Dynamic.FALSE);
297-
var inferenceField = inferenceFieldBuilder.apply(context.getMapperBuilderContext());
298-
var mergedInferenceField = inferenceField.merge(semanticMergeWith.fieldType().getInferenceField(), context);
299-
inferenceFieldBuilder = c -> mergedInferenceField;
296+
// We make sure to merge the inference field first to catch any model conflicts
297+
try {
298+
var context = mapperMergeContext.createChildContext(semanticMergeWith.leafName(), ObjectMapper.Dynamic.FALSE);
299+
var inferenceField = inferenceFieldBuilder.apply(context.getMapperBuilderContext());
300+
var mergedInferenceField = inferenceField.merge(semanticMergeWith.fieldType().getInferenceField(), context);
301+
inferenceFieldBuilder = c -> mergedInferenceField;
302+
} catch (Exception e) {
303+
// Wrap errors in nicer messages that hide inference field internals
304+
String errorMessage = e.getMessage() != null
305+
? e.getMessage().replaceAll(SemanticTextField.getEmbeddingsFieldName(""), "")
306+
: "";
307+
throw new IllegalArgumentException(errorMessage, e);
308+
}
300309

301310
super.merge(semanticMergeWith, conflicts, mapperMergeContext);
302311
conflicts.check();
@@ -1237,30 +1246,6 @@ private static boolean canMergeModelSettings(MinimalServiceSettings previous, Mi
12371246
return false;
12381247
}
12391248

1240-
private static boolean canMergeIndexOptions(SemanticTextIndexOptions previous, SemanticTextIndexOptions current, Conflicts conflicts) {
1241-
if (Objects.equals(previous, current)) {
1242-
return true;
1243-
}
1244-
1245-
if (previous == null || current == null) {
1246-
return true;
1247-
}
1248-
1249-
if (previous.type() == SemanticTextIndexOptions.SupportedIndexOptions.DENSE_VECTOR) {
1250-
DenseVectorFieldMapper.DenseVectorIndexOptions previousDenseOptions = (DenseVectorFieldMapper.DenseVectorIndexOptions) previous
1251-
.indexOptions();
1252-
DenseVectorFieldMapper.DenseVectorIndexOptions currentDenseOptions = (DenseVectorFieldMapper.DenseVectorIndexOptions) current
1253-
.indexOptions();
1254-
boolean updatable = previousDenseOptions.updatableTo(currentDenseOptions);
1255-
if (updatable == false) {
1256-
conflicts.addConflict(INDEX_OPTIONS_FIELD, "Incompatible index options");
1257-
}
1258-
return updatable;
1259-
}
1260-
1261-
return true;
1262-
}
1263-
12641249
private static SemanticTextIndexOptions parseIndexOptionsFromMap(String fieldName, Object node, IndexVersion indexVersion) {
12651250

12661251
if (node == null) {

x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/mapper/SemanticTextFieldMapperTests.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -587,14 +587,7 @@ public void testUpdateModelSettings() throws IOException {
587587
)
588588
)
589589
);
590-
assertThat(
591-
exc.getMessage(),
592-
containsString(
593-
"Cannot update parameter [model_settings] "
594-
+ "from [service=null, task_type=sparse_embedding] "
595-
+ "to [service=null, task_type=text_embedding, dimensions=10, similarity=cosine, element_type=float]"
596-
)
597-
);
590+
assertThat(exc.getMessage(), containsString("cannot be changed from type [sparse_vector] to [dense_vector]"));
598591
}
599592
}
600593
}

x-pack/plugin/inference/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ setup:
2727
"service": "text_embedding_test_service",
2828
"service_settings": {
2929
"model": "my_model",
30-
"dimensions": 10,
30+
"dimensions": 4,
3131
"similarity": "cosine",
3232
"api_key": "abc64"
3333
},
@@ -820,7 +820,7 @@ setup:
820820
- match: { "test-index-options.mappings.properties.semantic_field.index_options.dense_vector.confidence_interval": 1.0 }
821821

822822
- do:
823-
catch: /Incompatible index options/
823+
catch: /Cannot update parameter \[index_options\]/
824824
indices.put_mapping:
825825
index: test-index-options
826826
body:

x-pack/plugin/inference/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping_bwc.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ setup:
2727
"service": "text_embedding_test_service",
2828
"service_settings": {
2929
"model": "my_model",
30-
"dimensions": 10,
30+
"dimensions": 4,
3131
"similarity": "cosine",
3232
"api_key": "abc64"
3333
},
@@ -723,7 +723,7 @@ setup:
723723
- match: { "test-index-options.mappings.properties.semantic_field.index_options.dense_vector.confidence_interval": 1.0 }
724724

725725
- do:
726-
catch: /Incompatible index options/
726+
catch: /Cannot update parameter \[index_options\]/
727727
indices.put_mapping:
728728
index: test-index-options
729729
body:

0 commit comments

Comments
 (0)