You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/07/05 13:08:03 UTC
[GitHub] [hive] ashish-kumar-sharma opened a new pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
ashish-kumar-sharma opened a new pull request #2445:
URL: https://github.com/apache/hive/pull/2445
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Currently Date.java and Timestamp.java use DateTimeFormatter for parsing to convert the date/timpstamp from int,string,char etc to Date or Timestamp.
Default DateTimeFormatter which use ResolverStyle.LENIENT which mean date like "1992-13-12" is converted to "2000-01-12",
Moving DateTimeFormatter which use ResolverStyle.STRICT which mean date like "1992-13-12" is not be converted instead NULL is return.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
ResolverStyle.LENIENT to ResolverStyle.STRICT
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
UTs and QTs added as part of PR
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] adesh-rao commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
adesh-rao commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-876270108
This is going to change the behavior for customers. Previously, the queries which used to work fine, will start throwing exception now.
Maybe we should put it behind a flag ?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-884641257
@zabetak @sankarh Addressed all the comments and got green build also. Could you guys please review the PR?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r673814327
##########
File path: serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
##########
@@ -1254,10 +1257,13 @@ public static Timestamp getTimestampFromString(String s) {
s = s.trim();
s = trimNanoTimestamp(s);
+ if(StringUtils.isEmpty(s))
+ return null;
+
try {
return TimestampUtils.stringToTimestamp(s);
- } catch (IllegalArgumentException e) {
- return null;
+ } catch (IllegalArgumentException | DateTimeException e) {
+ throw new IllegalArgumentException("Cannot parse " + s);
Review comment:
reverted to null as discussed
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sankarh commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
sankarh commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-877124916
> This is going to change the behavior for customers. Previously, the queries which used to work fine, will start throwing exception now.
> Maybe we should put it behind a flag ?
I'm also in favour of keeping this change within a config.
@zabetak What is your opinion?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r673814134
##########
File path: ql/src/test/queries/clientpositive/type_conversions_1.q
##########
@@ -18,9 +18,3 @@ select
cast(null as binary)
from src limit 1;
--- Invalid conversions, should all be null
Review comment:
reverted
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sankarh commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
sankarh commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r666676465
##########
File path: ql/src/test/queries/clientpositive/ambiguitycheck.q
##########
@@ -32,11 +32,9 @@ select int(1.2) from src limit 1;
select bigint(1.34) from src limit 1;
select binary('1') from src limit 1;
select boolean(1) from src limit 1;
-select date('1') from src limit 2;
Review comment:
Why these queries are removed?
##########
File path: ql/src/test/queries/clientpositive/materialized_view_rewrite_in_between.q
##########
@@ -12,7 +12,7 @@ create database expr2;
use expr2;
create table sales(prod_id int, cust_id int, store_id int, sale_date timestamp, qty int, amt double, descr string);
insert into sales values
-(11,1,101,'12/24/2013',1000,1234.00,'onedummytwo');
+(11,1,101,'2013-12-24',1000,1234.00,'onedummytwo');
Review comment:
Is this a change in supported formats?
##########
File path: serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
##########
@@ -1254,10 +1257,13 @@ public static Timestamp getTimestampFromString(String s) {
s = s.trim();
s = trimNanoTimestamp(s);
+ if(StringUtils.isEmpty(s))
+ return null;
+
try {
return TimestampUtils.stringToTimestamp(s);
- } catch (IllegalArgumentException e) {
- return null;
+ } catch (IllegalArgumentException | DateTimeException e) {
+ throw new IllegalArgumentException("Cannot parse " + s);
Review comment:
What is the impact of throwing the exception instead of returning null?
##########
File path: ql/src/test/queries/clientpositive/type_conversions_1.q
##########
@@ -18,9 +18,3 @@ select
cast(null as binary)
from src limit 1;
--- Invalid conversions, should all be null
Review comment:
Why removed?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r673814445
##########
File path: ql/src/test/queries/clientpositive/materialized_view_rewrite_in_between.q
##########
@@ -12,7 +12,7 @@ create database expr2;
use expr2;
create table sales(prod_id int, cust_id int, store_id int, sale_date timestamp, qty int, amt double, descr string);
insert into sales values
-(11,1,101,'12/24/2013',1000,1234.00,'onedummytwo');
+(11,1,101,'2013-12-24',1000,1234.00,'onedummytwo');
Review comment:
reverted changes
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-874472306
@zabetak Could you please review the PR?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r673845756
##########
File path: common/src/test/org/apache/hive/common/util/TestTimestampParser.java
##########
@@ -47,12 +47,18 @@ public void testDefault() {
Assert.assertEquals(Timestamp.valueOf("1945-12-31T23:59:59"),
tsp.parseTimestamp("1945-12-31 23:59:59"));
+
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testDefaultInvalid() {
final TimestampParser tsp = new TimestampParser();
- tsp.parseTimestamp("12345");
+ Assert.assertEquals(null, tsp.parseTimestamp("12345"));
+ Assert.assertEquals(null, tsp.parseTimestamp("1945-12-45 23:59:59"));
+ Assert.assertEquals(null, tsp.parseTimestamp("1945-15-20 23:59:59"));
+ Assert.assertEquals(null, tsp.parseTimestamp("0000-00-00 00:00:00"));
+ Assert.assertEquals(null, tsp.parseTimestamp(""));
+ Assert.assertEquals(null, tsp.parseTimestamp("null"));
Review comment:
`assertEquals` -> `assertNull`
##########
File path: common/src/java/org/apache/hive/common/util/TimestampParser.java
##########
@@ -191,7 +190,13 @@ public Timestamp parseTimestamp(final String text)
LOG.debug("Could not parse timestamp text: {}", text);
}
}
- return Timestamp.valueOf(text);
+ Timestamp timestamp = null;
+ try {
+ timestamp = Timestamp.valueOf(text);
+ } catch (IllegalArgumentException e) {
+ LOG.info(e.getMessage());
Review comment:
Do we need to LOG this exception? I have the impression that when this method returns null we will generate an log message anyways. Moreover, I think that logging at INFO level may be a bit too much for something that seems to be in the regular control flow.
##########
File path: common/src/test/org/apache/hive/common/util/TestTimestampParser.java
##########
@@ -25,7 +25,7 @@
/**
* Test suite for parsing timestamps.
*/
-public class TestTimestampParser {
+public class TestTimestampParser {
Review comment:
Nit: Extra space
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
##########
@@ -173,8 +173,8 @@ public void testWrongDateStr() throws HiveException {
ObjectInspector[] arguments = { valueOI0, valueOI1 };
udf.initialize(arguments);
- runAndVerify("2014-02-30", 1, "2014-04-02", udf);
- runAndVerify("2014-02-32", 1, "2014-04-04", udf);
+ runAndVerify("2014-02-30", 1, null, udf);
+ runAndVerify("2014-02-32", 1, null, udf);
Review comment:
Worth adding a comment that this behavior is also compatible with MySQL:
```
SELECT DATE_ADD('2014-02-30',INTERVAL 1 MONTH);
+-----------------------------------------+
| DATE_ADD('2014-02-30',INTERVAL 1 MONTH) |
+-----------------------------------------+
| NULL |
+-----------------------------------------+
1 row in set, 1 warning (0.00 sec)
```
Moreover it is good to mention in the JIRA which UDFs are impacted by the change in this PR and update the [wiki|https://cwiki.apache.org/confluence/display/hive/languagemanual+udf] accordingly.
##########
File path: serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
##########
@@ -1254,9 +1260,13 @@ public static Timestamp getTimestampFromString(String s) {
s = s.trim();
s = trimNanoTimestamp(s);
+ if(StringUtils.isEmpty(s))
+ return null;
+
try {
return TimestampUtils.stringToTimestamp(s);
- } catch (IllegalArgumentException e) {
+ } catch (IllegalArgumentException | DateTimeException e) {
+ LOG.info("cannot parse datetime : {}", s);
Review comment:
Logging at INFO level may be too much. I would consider WARN, DEBUG, or remove the line altogether.
##########
File path: ql/src/test/results/clientpositive/llap/probedecode_mapjoin_stats.q.out
##########
@@ -416,4 +416,4 @@ POSTHOOK: Input: default@orders_fact
POSTHOOK: Input: default@seller_dim
#### A masked pattern was here ####
101 101 12345 12345 Seller 1 Item 101 2001-01-30 00:00:00
-104 104 23456 23456 Seller 2 Item 104 2002-03-02 00:00:00
+104 104 23456 23456 Seller 2 Item 104 NULL
Review comment:
Why do we get this NULL value?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r674145304
##########
File path: ql/src/test/results/clientpositive/llap/probedecode_mapjoin_stats.q.out
##########
@@ -416,4 +416,4 @@ POSTHOOK: Input: default@orders_fact
POSTHOOK: Input: default@seller_dim
#### A masked pattern was here ####
101 101 12345 12345 Seller 1 Item 101 2001-01-30 00:00:00
-104 104 23456 23456 Seller 2 Item 104 2002-03-02 00:00:00
+104 104 23456 23456 Seller 2 Item 104 NULL
Review comment:
Because of the following query in probedecode_mapjoin_stats.q
INSERT INTO orders_fact values(23456, 104, '2002-02-30 00:00:00');
Also timestamp format is "YYYY-MM-DD HH:MM:SS" due to that '2002-02-30 00:00:00' value get converted to null. Since probedecode_mapjoin_stats.q is to test happy flow of mapjoin_stats so lets make it some valid date to avoid confusion.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak closed pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2445:
URL: https://github.com/apache/hive/pull/2445
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r673814570
##########
File path: ql/src/test/queries/clientpositive/ambiguitycheck.q
##########
@@ -32,11 +32,9 @@ select int(1.2) from src limit 1;
select bigint(1.34) from src limit 1;
select binary('1') from src limit 1;
select boolean(1) from src limit 1;
-select date('1') from src limit 2;
Review comment:
reverted
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-874472306
@zabetak Could you please review the PR?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2445:
URL: https://github.com/apache/hive/pull/2445#issuecomment-877163732
@mattmccline-microsoft @ashish-kumar-sharma @sankarh I added a few comments in the JIRA since I think we should agree there before doing a line by line review.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r674145711
##########
File path: serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorUtils.java
##########
@@ -1254,9 +1260,13 @@ public static Timestamp getTimestampFromString(String s) {
s = s.trim();
s = trimNanoTimestamp(s);
+ if(StringUtils.isEmpty(s))
+ return null;
+
try {
return TimestampUtils.stringToTimestamp(s);
- } catch (IllegalArgumentException e) {
+ } catch (IllegalArgumentException | DateTimeException e) {
+ LOG.info("cannot parse datetime : {}", s);
Review comment:
Removed
##########
File path: common/src/java/org/apache/hive/common/util/TimestampParser.java
##########
@@ -191,7 +190,13 @@ public Timestamp parseTimestamp(final String text)
LOG.debug("Could not parse timestamp text: {}", text);
}
}
- return Timestamp.valueOf(text);
+ Timestamp timestamp = null;
+ try {
+ timestamp = Timestamp.valueOf(text);
+ } catch (IllegalArgumentException e) {
+ LOG.info(e.getMessage());
Review comment:
Removed
##########
File path: common/src/test/org/apache/hive/common/util/TestTimestampParser.java
##########
@@ -47,12 +47,18 @@ public void testDefault() {
Assert.assertEquals(Timestamp.valueOf("1945-12-31T23:59:59"),
tsp.parseTimestamp("1945-12-31 23:59:59"));
+
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testDefaultInvalid() {
final TimestampParser tsp = new TimestampParser();
- tsp.parseTimestamp("12345");
+ Assert.assertEquals(null, tsp.parseTimestamp("12345"));
+ Assert.assertEquals(null, tsp.parseTimestamp("1945-12-45 23:59:59"));
+ Assert.assertEquals(null, tsp.parseTimestamp("1945-15-20 23:59:59"));
+ Assert.assertEquals(null, tsp.parseTimestamp("0000-00-00 00:00:00"));
+ Assert.assertEquals(null, tsp.parseTimestamp(""));
+ Assert.assertEquals(null, tsp.parseTimestamp("null"));
Review comment:
Done
##########
File path: common/src/test/org/apache/hive/common/util/TestTimestampParser.java
##########
@@ -25,7 +25,7 @@
/**
* Test suite for parsing timestamps.
*/
-public class TestTimestampParser {
+public class TestTimestampParser {
Review comment:
Done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ashish-kumar-sharma commented on a change in pull request #2445: HIVE-25306: Move Date and Timestamp parsing from ResolverStyle.LENIENT to ResolverStyle.STRICT
Posted by GitBox <gi...@apache.org>.
ashish-kumar-sharma commented on a change in pull request #2445:
URL: https://github.com/apache/hive/pull/2445#discussion_r674145571
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
##########
@@ -173,8 +173,8 @@ public void testWrongDateStr() throws HiveException {
ObjectInspector[] arguments = { valueOI0, valueOI1 };
udf.initialize(arguments);
- runAndVerify("2014-02-30", 1, "2014-04-02", udf);
- runAndVerify("2014-02-32", 1, "2014-04-04", udf);
+ runAndVerify("2014-02-30", 1, null, udf);
+ runAndVerify("2014-02-32", 1, null, udf);
Review comment:
Sure. I will get access and then update the UDF.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org