[
https://issues.apache.org/jira/browse/SOLR-8904?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15218207#comment-15218207
]
Steve Rowe commented on SOLR-8904:
----------------------------------
+1, lots of nice simplification. I like how in TestUseDocValuesAsStored you
expanded the random date range to include negative dates and dates much farther
into the future.
All Solr tests passed for me.
Two nits:
* In DateMathParser.parseMath() you carried over from DateMathUtil the obsolete
commented out code {{ p.setNow(toObject(val.substring(0,zz)));}} - I think it
should be removed.
* DateFieldTest has unused imports
In DateMathParser.parseNoMath() you have a TODO to make a Date-returning
temporal query - here's an (untested) one (probably should not hold up this
issue though):
{code:java}
/** Temporal query to convert from TemporalAccessor to Date. */
private static Date temporalToDate(TemporalAccessor temporal) {
long seconds = temporal.getLong(INSTANT_SECONDS);
int nanos = temporal.get(NANO_OF_SECOND);
if (seconds < 0 && nanos > 0) { // from Instant.toEpochMilli()
long millis = Math.multiplyExact(seconds + 1, 1_000); // millis is now
<= 0
long adjustment = nanos / 1_000_000 - 1_000; // adjustment is
now negative
return new Date(Math.addExact(millis, adjustment));
} else {
long millis = Math.multiplyExact(seconds, 1_000);
return new Date(Math.addExact(millis, nanos / 1_000_000));
}
}
{code}
> Switch from SimpleDateFormat to Java 8 DateTimeFormatter.ISO_INSTANT
> --------------------------------------------------------------------
>
> Key: SOLR-8904
> URL: https://issues.apache.org/jira/browse/SOLR-8904
> Project: Solr
> Issue Type: Task
> Reporter: David Smiley
> Assignee: David Smiley
> Fix For: 6.0
>
> Attachments: SOLR_8904.patch, SOLR_8904.patch,
> SOLR_8904_switch_from_SimpleDateFormat_to_Instant_parse_and_format.patch
>
>
> 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())}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
