[ 
https://issues.apache.org/jira/browse/HIVE-15291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15705632#comment-15705632
 ] 

Peter Vary commented on HIVE-15291:
-----------------------------------

Hi Dhiraj,

Good job with the tests.
It is just nitpicking, but I think it is enough to add the new method 
(testGetTimestampFromString), and it is not needed to add the same conversion 
to the test which checks the conversion to timestamps with seconds 
(testgetTimestampWithSecondsInt).

Sadly our QA framework is not perfect yet. There are flaky tests which we 
collect under HIVE-15058 like HIVE-15084 "Flaky test: 
TestMiniTezCliDriver:explainanalyze_2, 3, 4, 5". There are some tests which are 
failed not because of the change.
Please check the test results with these in mind as described here: 
https://cwiki.apache.org/confluence/display/Hive/HowToCommit#HowToCommit-PreCommitruns,andcommittingpatches

After this - if there is no related test errors - your patch is ready to be 
commited (at least, in my opinion).

As I wrote to you in email I am also new to hive. It might be a good idea to 
ask [~jdere] to review this change, since he was working on the Timestamps 
lately so he knows this part of code very well.

Thanks for the patch, and the prompt changes.

Peter




> Comparison of timestamp fails if only date part is provided. 
> -------------------------------------------------------------
>
>                 Key: HIVE-15291
>                 URL: https://issues.apache.org/jira/browse/HIVE-15291
>             Project: Hive
>          Issue Type: Bug
>          Components: Hive, UDF
>    Affects Versions: 2.1.0
>            Reporter: Dhiraj Kumar
>            Assignee: Dhiraj Kumar
>         Attachments: HIVE-15291.1.patch, HIVE-15291.2.patch
>
>
> Summary : If a query needs to compare two timestamp with one timestamp 
> provided in "YYYY-MM-DD" format, skipping the time part, it returns incorrect 
> result. 
> Steps to reproduce : 
> 1. Start a hive-cli. 
> 2. Fire up the query -> select cast("2016-12-31 12:00:00" as timestamp) > 
> "2016-12-30";
> 3. Expected result : true
> 4. Actual result : NULL
> Detailed description : 
> If two primitives of different type needs to compared, a common comparator 
> type is chosen. Prior to 2.1, Common type Text was chosen to compare 
> Timestamp type and Text type. 
> In version 2.1, Common type Timestamp is chosen to compare Timestamp type and 
> Text type. This leads to converting Text type (YYYY-MM-DD) into 
> java.sql.Timestamp which throws exception saying the input is not in proper 
> format. The exception is suppressed and a null is returned. 
> Code below from org.apache.hadoop.hive.ql.exec.FunctionRegistry
> {code:java}
> if (pgA == PrimitiveGrouping.STRING_GROUP && pgB == 
> PrimitiveGrouping.DATE_GROUP) {
>       return b;
>     }
>     // date/timestamp is higher precedence than String_GROUP
>     if (pgB == PrimitiveGrouping.STRING_GROUP && pgA == 
> PrimitiveGrouping.DATE_GROUP) {
>       return a;
>     }
> {code}
> The bug was introduced in  
> [HIVE-13381|https://issues.apache.org/jira/browse/HIVE-13381]



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to