amansinha100 commented on code in PR #4545:
URL: https://github.com/apache/hive/pull/4545#discussion_r1289623534
##########
common/src/java/org/apache/hadoop/hive/common/type/Date.java:
##########
@@ -175,22 +178,16 @@ public void setTimeInMillis(long epochMilli) {
*/
public static Date valueOf(final String text) {
String s = Objects.requireNonNull(text).trim();
- int idx = s.indexOf(" ");
- if (idx != -1) {
- s = s.substring(0, idx);
- } else {
- idx = s.indexOf('T');
- if (idx != -1) {
- s = s.substring(0, idx);
- }
- }
- LocalDate localDate;
+ ParsePosition pos = new ParsePosition(0);
try {
- localDate = LocalDate.parse(s, PARSE_FORMATTER);
+ TemporalAccessor t = PARSE_FORMATTER.parseUnresolved(s, pos);
Review Comment:
The DateTimeFormatter::parseUnresolved() documentation mentions that it is
meant for advanced use cases and that typical applications should use the
parse() method. I suppose that since you are accessing the internal state of
the TemporalAccessor, it does match such a 'advanced' use case.
It would be useful to add a comment here about why parseUnresolved() is
used. Further, it would be useful to include in the comment how the validation
is done since this method does not do validation of the actual value (e.g if
the month is 15). The validation is deferred to the later call LocalDate.of()
method.
##########
common/src/java/org/apache/hadoop/hive/common/type/Date.java:
##########
@@ -175,22 +178,16 @@ public void setTimeInMillis(long epochMilli) {
*/
public static Date valueOf(final String text) {
String s = Objects.requireNonNull(text).trim();
- int idx = s.indexOf(" ");
- if (idx != -1) {
- s = s.substring(0, idx);
- } else {
- idx = s.indexOf('T');
- if (idx != -1) {
- s = s.substring(0, idx);
- }
- }
- LocalDate localDate;
+ ParsePosition pos = new ParsePosition(0);
try {
- localDate = LocalDate.parse(s, PARSE_FORMATTER);
+ TemporalAccessor t = PARSE_FORMATTER.parseUnresolved(s, pos);
+ if (pos.getErrorIndex() >= 0) {
+ throw new DateTimeParseException("Text could not be parsed to date",
s, pos.getErrorIndex());
Review Comment:
nit: This exception is thrown but the catch block on line 190 prints a
generic error message rather than this specific one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
