Re: Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-18 Thread Mohammad Islam

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/#review37540
---

Ship it!


+1

- Mohammad Islam


On March 14, 2014, 9:23 p.m., Jason Dere wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19212/
 ---
 
 (Updated March 14, 2014, 9:23 p.m.)
 
 
 Review request for hive.
 
 
 Bugs: HIVE-6645
 https://issues.apache.org/jira/browse/HIVE-6645
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 - fix null inputs
 - allow char/varchar params
 - tests
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java 
 c31174a 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
  dc259c6 
   ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
  PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/19212/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jason Dere
 




Re: Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-14 Thread Mohammad Islam

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/#review37177
---



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
https://reviews.apache.org/r/19212/#comment68592

do we need to check if arguments[1].get() is null  as done for arguments[0]?
Or the converter will handle it and return 'null'.



ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
https://reviews.apache.org/r/19212/#comment68594

what about test cases where 1st arg is not null but the second arg is  null?


- Mohammad Islam


On March 14, 2014, 2:40 a.m., Jason Dere wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19212/
 ---
 
 (Updated March 14, 2014, 2:40 a.m.)
 
 
 Review request for hive.
 
 
 Bugs: HIVE-6645
 https://issues.apache.org/jira/browse/HIVE-6645
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 - fix null inputs
 - allow char/varchar params
 - tests
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java 
 c31174a 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
  dc259c6 
   ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
  PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/19212/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jason Dere
 




Re: Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-14 Thread Jason Dere


 On March 14, 2014, 7:35 a.m., Mohammad Islam wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java,
   line 132
  https://reviews.apache.org/r/19212/diff/1/?file=519528#file519528line132
 
  do we need to check if arguments[1].get() is null  as done for 
  arguments[0]?
  Or the converter will handle it and return 'null'.

I'll add the testcase for this, and we'll find out :)


- Jason


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/#review37177
---


On March 14, 2014, 2:40 a.m., Jason Dere wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19212/
 ---
 
 (Updated March 14, 2014, 2:40 a.m.)
 
 
 Review request for hive.
 
 
 Bugs: HIVE-6645
 https://issues.apache.org/jira/browse/HIVE-6645
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 - fix null inputs
 - allow char/varchar params
 - tests
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java 
 c31174a 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
  dc259c6 
   ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
  PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/19212/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jason Dere
 




Re: Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-14 Thread Jason Dere


 On March 14, 2014, 7:35 a.m., Mohammad Islam wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java,
   line 132
  https://reviews.apache.org/r/19212/diff/1/?file=519528#file519528line132
 
  do we need to check if arguments[1].get() is null  as done for 
  arguments[0]?
  Or the converter will handle it and return 'null'.
 
 Jason Dere wrote:
 I'll add the testcase for this, and we'll find out :)

It does work fine, but probably for the best to have that null check in there 
as well.


- Jason


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/#review37177
---


On March 14, 2014, 2:40 a.m., Jason Dere wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/19212/
 ---
 
 (Updated March 14, 2014, 2:40 a.m.)
 
 
 Review request for hive.
 
 
 Bugs: HIVE-6645
 https://issues.apache.org/jira/browse/HIVE-6645
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 - fix null inputs
 - allow char/varchar params
 - tests
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java 
 c31174a 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
  dc259c6 
   ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
  PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/19212/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jason Dere
 




Re: Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-14 Thread Jason Dere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/
---

(Updated March 14, 2014, 9:23 p.m.)


Review request for hive.


Changes
---

Updating patch based on feedback from Mohammad Islam


Bugs: HIVE-6645
https://issues.apache.org/jira/browse/HIVE-6645


Repository: hive-git


Description
---

- fix null inputs
- allow char/varchar params
- tests


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java c31174a 
  
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
 dc259c6 
  ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/19212/diff/


Testing
---


Thanks,

Jason Dere



Review Request 19212: HIVE-6645: to_date()/to_unix_timestamp() fail with NPE if input is null

2014-03-13 Thread Jason Dere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19212/
---

Review request for hive.


Bugs: HIVE-6645
https://issues.apache.org/jira/browse/HIVE-6645


Repository: hive-git


Description
---

- fix null inputs
- allow char/varchar params
- tests


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java c31174a 
  
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFToUnixTimeStamp.java
 dc259c6 
  ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java 384ce4e 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFToUnixTimestamp.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/19212/diff/


Testing
---


Thanks,

Jason Dere