Properly handle multi fields in block loaders with synthetic source enabled #127483
Conversation
lkts
changed the title
lkts
changed the title
| } | ||
|
|
||
| @Override | ||
| public String text() throws IOException { |
There was a problem hiding this comment.
This is actually a fix for an edge case that the new test finds.
| import static org.elasticsearch.index.mapper.BlockLoaderTestCase.buildSpecification; | ||
| import static org.elasticsearch.index.mapper.BlockLoaderTestCase.hasDocValues; | ||
|
|
||
| public class TextFieldWithParentBlockLoaderTests extends MapperServiceTestCase { |
There was a problem hiding this comment.
This is now a dedicated test for this case and it does not include "normal" tests that are covered in TextFieldBlockLoaderTests.
| return null; | ||
| } | ||
|
|
||
| var map = commonMappingParameters(); |
There was a problem hiding this comment.
This map was previously shared between invocations of a lambda returned by this method. That led to generated mappings being very similar most of the time which is not what we want.
| import static org.apache.lucene.tests.util.LuceneTestCase.newDirectory; | ||
| import static org.apache.lucene.tests.util.LuceneTestCase.random; | ||
|
|
||
| public class BlockLoaderTestRunner { |
There was a problem hiding this comment.
This is needed for TextFieldWithParentBlockLoaderTests to work.
lkts
added
auto-backport
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
|
@lkts is this one still backport_pending? |
|
This is merged to 8.19. |
4 participants
This is a follow up to #127273 which discovered an issue with multi-fields together with fallback synthetic source and fixed it for
text. This PR fixes that same problem for all block loaders.