Extend the date rounding logic to be conditional

[pgomulka]

Date rounding logic should take into account the fields that will be
parsed be a parser. If a parser has a DayOfYear field, the rounding logic
should not try to default DayOfMonth as it will conflict with DayOfYear

However the DateTimeFormatter does not have a public method to return
information of fields that will be parsed. The hacky workaround is
to rely on toString() implementation that will return a field info when
it was defined with textual pattern.

This commits introduced conditional logic for DayOfYear, ClockHourOfAMPM and HourOfAmPM

closes #89096
closes #58986

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[elasticsearchmachine]

Hi @pgomulka, I've created a changelog YAML for you.

[grcevski]

LGTM! I left one small remark about potential performance issue.

[grcevski]

Just a small comment here, since we might call this frequently enough and we don't know the cost of parser.toString(), do you mind extracting the parsed value into a local for the if conditionals?

[pgomulka]

good idea

[ChrisHegarty]

LGTM

[pgomulka]

I just realised that we possibly want to add a week based year defaulting too. So that we support YYYY'W'ww[e] ([ ] means parse optional day of week). It might be a bit artificial though, because I don't think [] syntax is commonly used (I might be mistaken..) and we don't have a named formatter with day of week within startOptional/endOptional

assuming [] is rare and thus the only optional fields that could conflict are month of year and day of month, this logic could be simplified to just

 if(parserAsString.contains(ChronoField.MONTH_OF_YEAR.toString()) ){
          builder.parseDefaulting(ChronoField.MONTH_OF_YEAR, 1L);
      }
      if(parserAsString.contains(ChronoField.DAY_OF_MONTH.toString()) ){
          builder.parseDefaulting(ChronoField.DAY_OF_MONTH, 1L);
      }

let me know what you think

[pgomulka]

@elasticmachine update branch

[grcevski]

I just realised that we possibly want to add a week based year defaulting too. So that we support YYYY'W'ww[e] ([ ] means parse optional day of week). It might be a bit artificial though, because I don't think [] syntax is commonly used (I might be mistaken..) and we don't have a named formatter with day of week within startOptional/endOptional

assuming [] is rare and thus the only optional fields that could conflict are month of year and day of month, this logic could be simplified to just

 if(parserAsString.contains(ChronoField.MONTH_OF_YEAR.toString()) ){
          builder.parseDefaulting(ChronoField.MONTH_OF_YEAR, 1L);
      }
      if(parserAsString.contains(ChronoField.DAY_OF_MONTH.toString()) ){
          builder.parseDefaulting(ChronoField.DAY_OF_MONTH, 1L);
      }

let me know what you think

I think that's a fair assumption. We don't have to make this more complicated at this point. I'd just put a comment that we don't handle the 'optional day of week' format, but adding day of month and month of year sound great!

[grcevski]

Since we are backporting to 7.17, can we please backport to 8.4.1 too, it's the current release.