Add mappings for enrich fields (#96056)#10
Add mappings for enrich fields (#96056)#10MitchLewis930 wants to merge 1 commit intopr_020_beforefrom
Conversation
We are developing a new component that performs lookup during query time. The idea is to utilize the existing enrich policies and indices used during indexing. However, to ensure proper functionality of the new component, we need the mapping types and doc_values of the enrich fields. With this change, populating the mapping for enrich fields is a best-effort process to maintain the current behavior. This means that any mapping conflicts arising between the enrich fields of the source indices will be ignored. I will follow up to address this validation issue.
There was a problem hiding this comment.
Pull request overview
This PR enhances the enrich policy functionality by adding field mappings for all enrich fields, not just the match field. Previously, only the match field had its mapping preserved in the enrich index, while other enrich fields were unmapped. This change ensures that all enrich fields retain their mapping types and settings from the source indices.
Changes:
- Refactored mapping validation and creation logic to handle all enrich fields
- Added validation for mapping conflicts across multiple source indices
- Updated test assertions to verify mappings for all fields, not just the match field
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPolicyRunner.java | Refactored mapping resolution to create mappings for all enrich fields with conflict detection across source indices |
| x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichPolicyRunnerTests.java | Updated all test cases to verify complete field mappings and added new tests for mapping conflict scenarios |
| docs/changelog/96056.yaml | Added changelog entry documenting the enhancement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| policy.getMatchField(), | ||
| Strings.collectionToCommaDelimitedString(policy.getIndices()), | ||
| Strings.collectionToCommaDelimitedString(formats) | ||
| ); | ||
| } | ||
| return null; | ||
| } | ||
| return new MappingTypeAndFormat(Iterables.get(types, 0), formats.isEmpty() ? null : Iterables.get(formats, 0)); |
There was a problem hiding this comment.
This error message references policy.getMatchField() but should reference the fieldName parameter since this validation is used for both match fields and enrich fields. The error message will be incorrect when validating enrich fields with format conflicts.
| policy.getMatchField(), | |
| Strings.collectionToCommaDelimitedString(policy.getIndices()), | |
| Strings.collectionToCommaDelimitedString(formats) | |
| ); | |
| } | |
| return null; | |
| } | |
| return new MappingTypeAndFormat(Iterables.get(types, 0), formats.isEmpty() ? null : Iterables.get(formats, 0)); | |
| fieldName, | |
| Strings.collectionToCommaDelimitedString(policy.getIndices()), | |
| Strings.collectionToCommaDelimitedString(formats) | |
| ); | |
| } | |
| return null; | |
| } | |
| return new MappingTypeAndFormat(Iterables.get(types, 0), formats.isEmpty() ? null : Iteriles.get(formats, 0)); |
| private static <T> T extractValues(Map<String, Object> properties, String path) { | ||
| return (T) properties.get(path); | ||
| } | ||
|
|
There was a problem hiding this comment.
The method name extractValues is misleading as it extracts a single value, not multiple values. Consider renaming to extractValue or getProperty to better reflect that it retrieves a single property value from the map.
| private static <T> T extractValues(Map<String, Object> properties, String path) { | |
| return (T) properties.get(path); | |
| } | |
| private static <T> T extractValue(Map<String, Object> properties, String path) { | |
| return (T) properties.get(path); | |
| } | |
| /** | |
| * @deprecated Use {@link #extractValue(Map, String)} instead. | |
| */ | |
| @Deprecated | |
| @SuppressWarnings("unchecked") | |
| private static <T> T extractValues(Map<String, Object> properties, String path) { | |
| return extractValue(properties, path); | |
| } |
| @@ -194,11 +195,64 @@ static void validateMappings( | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The MappingTypeAndFormat record lacks documentation explaining its purpose and the meaning of its parameters. Add a JavaDoc comment describing that it encapsulates field mapping type and optional format information extracted from source index mappings.
| /** | |
| * Encapsulates the mapping type and optional format for a field as derived from | |
| * the source index mappings used by an enrich policy. | |
| * | |
| * @param type the field mapping type (for example, {@code keyword}, {@code date}, etc.) | |
| * @param format the optional mapping format string for the field (for example, a date format), | |
| * or {@code null} if the field does not define a format | |
| */ |
2 participants
PR_020