Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-8904

Switch from SimpleDateFormat to Java 8 DateTimeFormatter.ISO_INSTANT

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 6.0, 6.1, 7.0
    • None
    • None

    Description

      I'd like to move Solr away from SimpleDateFormat to Java 8's java.time.formatter.DateTimeFormatter API, particularly using simply ISO_INSTANT without any custom rules. This especially involves our DateFormatUtil class in Solr core, but also involves DateUtil (I filed SOLR-8903 to deal with additional delete/move/deprecations for that one).

      In particular, there's new Date(Instant.parse(d).toEpochMilli()) for parsing and DateTimeFormatter.ISO_INSTANT.format(d.toInstant()) for formatting. Simple & thread-safe!

      I want to simply cut over completely without having special custom rules. There are differences in how ISO_INSTANT does things:

      • Formatting: Milliseconds are 0 padded to 3 digits if the milliseconds is non-zero. Thus 30 milliseconds will have ".030" added on. Our current formatting code emits ".03".
      • Dates with years after '9999' (i.e. 10000 and beyond, >= 5 digit years): ISO_INSTANT strictly demands a leading '+' – it is formatted with a "+" and if such a year is parsed it must have a "+" or there is an exception. SimpleDateFormatter requires the opposite – no '+' and and if you tried to give it one, it would throw an exception.
      • Currently we don't support negative years (resulting in invisible errors mostly!). ISO_INSTANT supports this!

      In addition, DateFormatUtil.parseDate currently allows the trailing 'Z' to be optional, but the only caller that could exploit this is the analytics module. I'd like to remove the optional-ness of 'Z' and inline this method away to new Date(Instant.parse(d).toEpochMilli()).

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            dsmiley David Smiley
            dsmiley David Smiley
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment