Short circuit date patterns after first match #83764
Conversation
danhermann
added
>bug
:Data Management/Ingest Node
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
Hi @danhermann, I've created a changelog YAML for you. |
| for (Function<Map<String, Object>, Function<String, ZonedDateTime>> dateParser : dateParsers) { | ||
| int dateParserIndex = 0; | ||
| while (dateTime == null && dateParserIndex < dateParsers.size()) { | ||
| var dateParser = dateParsers.get(dateParserIndex); |
There was a problem hiding this comment.
I'm not sure I understand why we do it this way, why doesn't it work to do:
for (Function<stuff> dateParser : dateParsers) {
try {
dateTime = dateParser.apply(ingestDocument.getSourceAndMetadata()).apply(value);
break;
catch (Exception e) {
...
}
}To just short circuit for the first non-error-throwing date parser?
There was a problem hiding this comment.
Anything that terminates the loop after the parser succeeds will work.
There was a problem hiding this comment.
Okay, that seems more readable to me than a counter than increments and the null check, what do you think?
There was a problem hiding this comment.
I like having the loop termination criteria explicitly specified upfront with a while loop but I don't feel that strongly about it.
|
@elasticmachine update branch |
|
expected head sha didn’t match current head ref. |
|
@elasticmachine update branch |
|
Should this go into 7.17.x also? It seems like a really unfortunate bug with a small and non-dangerous fix. What do you think? |
|
@dakrone FYI this was not at all a "non-dangerous fix"; I was banging my head in order to determine why I was not seeing events in my index after the update to 7.17.1, until I realised that the @timestamp field values of all new events were in microseconds (!) instead of milliseconds. Turns out the formats order I was using in the ingest pipeline processor (which was ISO8601, UNIX, UNIX_MS, since I have 2 sources that send dates in either seconds or milliseconds format) had to be changed to ISO8601, UNIX_MS, UNIX, otherwise the milliseconds (not the seconds, those still worked fine) were converted to microseconds, pointing some thousands year in the future! |
In other words, don't attempt to parse the date with the rest of the patterns after one matches.
Relates to #73918