Remove dead code in parseArrayDynamic#108912
Conversation
|
|
||
| private static void parseArrayDynamic(DocumentParserContext context, String currentFieldName) throws IOException { | ||
| ensureNotStrict(context, currentFieldName); | ||
| if (context.dynamic() == ObjectMapper.Dynamic.FALSE) { |
There was a problem hiding this comment.
This is the removed part, the rest is just shifted.
There was a problem hiding this comment.
If I run the DynamicMappingTests#testArraysDynamicFalse() test then the if statement does seem to be executed.
There was a problem hiding this comment.
The test still passes though, I wonder if it has any effect in practice.
There was a problem hiding this comment.
So what happens is that the function calls #parseDynamicValue that also checks for this, returning immediately.
We can remove it to simplify the logic, but it's fine to keep it too.
There was a problem hiding this comment.
I think the test doesn't validate that skipChildren() is invoked, which I think is the point of the whole if statement (avoid parsing stuff that we're not going to use). Not sure how, but I think the test should somehow assert that skipChildren() is invoked.
There was a problem hiding this comment.
This is actually needed, I added more tests.. lemme revert for now.
|
@elasticsearchmachine update branch |
1 similar comment
|
@elasticsearchmachine update branch |
|
run elasticsearch-ci/bwc-snapshots |
|
run elasticsearch-ci/8.13.5 / bwc-snapshots |
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Noticed that the code never executed while running tests for #108911