Allow [null] values in [null_value] #61798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romseygeek
added
>enhancement
:Search Foundations/Mapping
|
Pinging @elastic/es-search (:Search/Mapping) |
nik9000
approved these changes
Sep 1, 2020
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| public abstract class MapperServiceTestCase extends ESTestCase { |
There was a problem hiding this comment.
👍 These are quite useful and I could see us wanting them, say, in the AggregatorTestCase to make it very easy to index with a mapping.
romseygeek
added a commit
to romseygeek/elasticsearch
that referenced
this pull request
Sep 1, 2020
Several field mappers have a null_value parameter, that allows you to specify a placeholder value to insert into a document if the incoming value for that field is null. The default value for this is always null, meaning "add no placeholder". However, we explicitly bar users from setting this parameter directly to null (done in elastic#7978, in order to fix an NPE). This exclusion means that if a mapper is serialized with include_defaults, then we either need to special-case null_value to ensure that it is not output when it holds the default value, or we find that the resulting serialized form cannot be used to create a mapping. This stops us doing some useful generic testing of mappers. This commit permits null as a parameter value for null_value, and changes the tests to check that it is a) permissible and b) applied without throwing errors. As part of the testing changes, a new base class MapperServiceTestCase is refactored from MapperTestCase, holding the various helper methods related to building mappings but not the single-mapper specific abstract methods. Closes elastic#58823
romseygeek
added a commit
that referenced
this pull request
Sep 2, 2020
Several field mappers have a null_value parameter, that allows you to specify a placeholder value to insert into a document if the incoming value for that field is null. The default value for this is always null, meaning "add no placeholder". However, we explicitly bar users from setting this parameter directly to null (done in #7978, in order to fix an NPE). This exclusion means that if a mapper is serialized with include_defaults, then we either need to special-case null_value to ensure that it is not output when it holds the default value, or we find that the resulting serialized form cannot be used to create a mapping. This stops us doing some useful generic testing of mappers. This commit permits null as a parameter value for null_value, and changes the tests to check that it is a) permissible and b) applied without throwing errors. As part of the testing changes, a new base class MapperServiceTestCase is refactored from MapperTestCase, holding the various helper methods related to building mappings but not the single-mapper specific abstract methods. Closes #58823
romseygeek
added a commit
that referenced
this pull request
Sep 2, 2020
This commit adds a test to MapperTestCase that explicitly checks that a mapper can serialize all its default values, and that this serialization can then be re-parsed. Note that the test is disabled for non-parametrized mappers as their serialization may in some cases output parameters that are not accepted. Gradually moving all mappers to parametrized form will address this. The commit also contains a fix to keyword mappers, which were not correctly serializing the similarity parameter; this partially addresses #61563. It also enables `null` as a value for `null_value` on `scaled_float`, as a follow-up to #61798
romseygeek
added a commit
to romseygeek/elasticsearch
that referenced
this pull request
Sep 2, 2020
…ic#61844) This commit adds a test to MapperTestCase that explicitly checks that a mapper can serialize all its default values, and that this serialization can then be re-parsed. Note that the test is disabled for non-parametrized mappers as their serialization may in some cases output parameters that are not accepted. Gradually moving all mappers to parametrized form will address this. The commit also contains a fix to keyword mappers, which were not correctly serializing the similarity parameter; this partially addresses elastic#61563. It also enables `null` as a value for `null_value` on `scaled_float`, as a follow-up to elastic#61798
romseygeek
added a commit
that referenced
this pull request
Sep 3, 2020
… (#61877) This commit adds a test to MapperTestCase that explicitly checks that a mapper can serialize all its default values, and that this serialization can then be re-parsed. Note that the test is disabled for non-parametrized mappers as their serialization may in some cases output parameters that are not accepted. Gradually moving all mappers to parametrized form will address this. The commit also contains a fix to keyword mappers, which were not correctly serializing the similarity parameter; this partially addresses #61563. It also enables `null` as a value for `null_value` on `scaled_float`, as a follow-up to #61798
61 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several field mappers have a
null_valueparameter, that allows you to specify a placeholdervalue to insert into a document if the incoming value for that field is
null. The default valuefor this is always
null, meaning "add no placeholder". However, we explicitly bar users fromsetting this parameter directly to
null(done in #7978, in order to fix an NPE).This exclusion means that if a mapper is serialized with
include_defaults, then we either needto special-case
null_valueto ensure that it is not output when it holds the default value, orwe find that the resulting serialized form cannot be used to create a mapping. This stops us
doing some useful generic testing of mappers.
This commit permits
nullas a parameter value fornull_value, and changes the tests to checkthat it is a) permissible and b) applied without throwing errors. As part of the testing changes,
a new base class
MapperServiceTestCaseis refactored fromMapperTestCase, holdingthe various helper methods related to building mappings but not the single-mapper specific
abstract methods.
Closes #58823