Skip to content

Conversation

@Darkheir
Copy link
Contributor

Description

Add support for datetimes without milliseconds or nanoseconds with a timezone set as it is supported by Elasticsearch.
The PR makes also date_optional_time and strict_date_optional_time_nanos more lenient with minutes and seconds as it was already the case for strict_date_optional_time.

How was this PR tested?

Unit tests

Signed-off-by: Darkheir <raphael.cohen@sekoia.io>
Copy link
Collaborator

@rdettai-sk rdettai-sk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not ISO 8601 compliant but definitively better. I also noticed that

  • there doesn't seem to be a difference between yyyy-MM-dd['T'HH[:mm[:ss[.SSS][Z]]]] and yyyy-MM-dd['T'HH[:mm[:ss[.SSSSS][Z]]]]
  • I think it should be strict_date_optional_time that forces yyyy-MM-dd and date_optional_time that has it optional

@Darkheir
Copy link
Contributor Author

  • I think it should be strict_date_optional_time that forces yyyy-MM-dd and date_optional_time that has it optional

It is complicated to find a good reference about the behavior inside Elasticsearch, but in their documentation it is written

Most of the below formats have a strict companion format, which means that year, month and day parts of the month must use respectively 4, 2 and 2 digits exactly, potentially prepending zeros. For instance a date like 5/11/1 would be considered invalid and would need to be rewritten to 2005/11/01 to be accepted by the date parser.

So there is nothing about the month and the days being mandatory or not...

I think that in both cases we should allow to have only the year or the year and the month.

Signed-off-by: Darkheir <raphael.cohen@sekoia.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants