Improve error messages on bad [format] and [null_value] params for date mapper #61932
Conversation
romseygeek
added
>enhancement
:Search Foundations/Mapping
|
Pinging @elastic/es-search (:Search/Mapping) |
|
@elasticmachine run elasticsearch-ci/packaging-sample-windows (because that's what we're doing today, apparently) |
jtibshirani
left a comment
jtibshirani
left a comment
There was a problem hiding this comment.
I left some small comments, but looks good to me once those are addressed !
| return new DateFieldType(buildFullName(context), index.getValue(), docValues.getValue(), | ||
| dateTimeFormatter, resolution, meta.getValue()); | ||
| try { | ||
| DateFormatter dateTimeFormatter = DateFormatter.forPattern(format.getValue()).withLocale(locale.getValue()); |
There was a problem hiding this comment.
Small comment, maybe we should scope the try/ catch down to avoid rewrapping an IllegalArgumentException from the constructor call ? It doesn't seem like a concern for this specific constructor, but seems like good practice.
| try { | ||
| return formatter.parseMillis(nullValue.getValue()); | ||
| } | ||
| catch (Exception e) { |
There was a problem hiding this comment.
Small comment, catch is usually on same line as previous brace.
Not relevant for this PR, but reading this made me notice that we parse null_value dates different from ones that appear in documents. Specifically, I wonder why we don't use DateFieldType#parse here ?
There was a problem hiding this comment.
Specifically, I wonder why we don't use DateFieldType#parse here ?
This is a bug, I'll open a separate PR to fix.
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/packaging-sample-windows |
…te mapper (#61932) Currently, if an incorrectly formatted date is passed as a null_value for a date field mapper configuration, you get a vague error: Failed to parse mapping [_doc]: cannot parse empty date Similarly, if you pass an incorrect format, you get the error: Failed to parse mapping [_doc]: Invalid format [...] This commit improves both these errors by including the mapper name and parameter that are misconfigured. Fixes #61712
…te mapper (#61932) Currently, if an incorrectly formatted date is passed as a null_value for a date field mapper configuration, you get a vague error: Failed to parse mapping [_doc]: cannot parse empty date Similarly, if you pass an incorrect format, you get the error: Failed to parse mapping [_doc]: Invalid format [...] This commit improves both these errors by including the mapper name and parameter that are misconfigured. Fixes #61712
4 participants
Currently, if an incorrectly formatted date is passed as a
null_valuefor a date field mapperconfiguration, you get a vague error:
Similarly, if you pass an incorrect format, you get the error:
This commit improves both these errors by including the mapper name and parameter that
are misconfigured.
Fixes #61712