Simulate ingest API uses existing index mapping when mapping_addition is given #132101
Conversation
masseyke
added
>bug
:Data Management/Ingest Node
|
Hi @masseyke, I've created a changelog YAML for you. |
masseyke
added
the
auto-backport
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the simulate ingest API where when an index exists and a mapping_addition is provided, the API incorrectly uses the mapping from the index's matching template instead of the actual index mapping. The fix ensures that the existing index mapping is always used as the base when merging with mapping_addition.
Key changes:
- Simplified the condition check to always use existing index mapping when no template substitutions are present
- Removed duplicate code path that incorrectly handled mapping addition for existing indices
- Added comprehensive test coverage for both regular indices and data streams
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| TransportSimulateBulkAction.java | Main fix - simplified logic to always use existing index mapping when available, removed duplicate handling code |
| 80_ingest_simulate.yml | Added extensive test cases covering the bug scenarios for both regular indices and data streams |
| 132101.yaml | Added changelog entry documenting the bug fix |
server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java
Show resolved
Hide resolved
|
Pinging @elastic/es-data-management (Team:Data Management) |
elasticsearchmachine
added
the
Team:Data Management
| CompressedXContent mappings = imd.mapping() == null ? null : imd.mapping().source(); | ||
| CompressedXContent mergedMappings = mappingAddition == null ? null : mergeMappings(mappings, mappingAddition); |
There was a problem hiding this comment.
These … == null ? null : … are calling out for Optional.map() :D
There was a problem hiding this comment.
Ha yeah I thought about doing that but then didn't (for reasons I don't remember). I'll do that now.
There was a problem hiding this comment.
Oh right, because mergeMappings throws a checked exception, and dealing with that checked exception makes the code far more complicated than just the null checks. I'll change the first line though.
…seyke/elasticsearch into fix/simulate-ingest-mapping-addition
💔 Backport failed
You can use sqren/backport to manually backport by running |
💔 Some backports could not be created
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
… is given (elastic#132101) (cherry picked from commit 593f48f) # Conflicts: # server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
… is given (elastic#132101) (cherry picked from commit 593f48f) # Conflicts: # server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java
3 participants
This fixes a bug described at #131608 (comment). The problem is that if an index exists and a
mapping_additionis given, we ignore the mapping from the index and instead use the mapping from the index's matching template if it exists (otherwise the index's mapping is used).