You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Mohammad Islam <mi...@yahoo.com> on 2013/11/04 22:25:00 UTC
Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF for
UDFDate* classes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/
-----------------------------------------------------------
Review request for hive.
Bugs: HIVE-5731
https://issues.apache.org/jira/browse/HIVE-5731
Repository: hive-git
Description
-------
GenericUDF class is the latest and recommended base class for any UDFs.
This JIRA is to change the current UDFDate* classes extended from GenericUDF.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
Diff: https://reviews.apache.org/r/15213/diff/
Testing
-------
Thanks,
Mohammad Islam
Re: Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF
for UDFDate* classes
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/#review29405
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java
<https://reviews.apache.org/r/15213/#comment56632>
Instead of converttoDate() and creating new Date() everytime, your earlier approach of getting DateWritable via converter and than obtaining Date from it via dw.get() was better, since new one requires creating new Date() everytime.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java
<https://reviews.apache.org/r/15213/#comment56631>
Have this object result as a class member and then reuse that object across function call by doing result.set() on each invocation, that will help in object reuse and save new() on each function invocation.
- Ashutosh Chauhan
On Nov. 11, 2013, 9:03 p.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15213/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 9:03 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5731
> https://issues.apache.org/jira/browse/HIVE-5731
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> GenericUDF class is the latest and recommended base class for any UDFs.
> This JIRA is to change the current UDFDate* classes extended from GenericUDF.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 8d3a84f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
> ql/src/test/results/clientpositive/udf_to_date.q.out 6ff5ee8
>
> Diff: https://reviews.apache.org/r/15213/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF
for UDFDate* classes
Posted by Mohammad Islam <mi...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/
-----------------------------------------------------------
(Updated Nov. 11, 2013, 9:03 p.m.)
Review request for hive.
Changes
-------
Update after incorporating with Ashutosh's comments.
Bugs: HIVE-5731
https://issues.apache.org/jira/browse/HIVE-5731
Repository: hive-git
Description
-------
GenericUDF class is the latest and recommended base class for any UDFs.
This JIRA is to change the current UDFDate* classes extended from GenericUDF.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 8d3a84f
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
ql/src/test/results/clientpositive/udf_to_date.q.out 6ff5ee8
Diff: https://reviews.apache.org/r/15213/diff/
Testing
-------
Thanks,
Mohammad Islam
Re: Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF
for UDFDate* classes
Posted by Mohammad Islam <mi...@yahoo.com>.
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java, line 67
> > <https://reviews.apache.org/r/15213/diff/3/?file=378196#file378196line67>
> >
> > Shouldn't the outputOI be writableDateOI ?
I tried to use the same that was used in original UDFDate. It used Text as return.
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java, line 72
> > <https://reviews.apache.org/r/15213/diff/3/?file=378196#file378196line72>
> >
> > First argument should be argumentOI.
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java, line 80
> > <https://reviews.apache.org/r/15213/diff/3/?file=378196#file378196line80>
> >
> > First argument should be argumentOI.
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java, line 99
> > <https://reviews.apache.org/r/15213/diff/3/?file=378196#file378196line99>
> >
> > Instead of throwing up in parse exception, we should return null in such cases.
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java, line 93
> > <https://reviews.apache.org/r/15213/diff/3/?file=378197#file378197line93>
> >
> > outputOI should be writableDateOI
Same as above. followed the same used in UDFDate that returns Text.
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java, line 97
> > <https://reviews.apache.org/r/15213/diff/3/?file=378197#file378197line97>
> >
> > First arg should be ((PrimitiveObjectInspector) arguments[0])
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java, line 137
> > <https://reviews.apache.org/r/15213/diff/3/?file=378197#file378197line137>
> >
> > Instead of throwing exception, this should return null.
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java, line 91
> > <https://reviews.apache.org/r/15213/diff/3/?file=378198#file378198line91>
> >
> > In evaluate() you are creating new IntWritable everytime, instead that function should return int and you should do output.set() and return output. This way we will save unnecessary object creation of intWritable for each invocation.
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java, line 97
> > <https://reviews.apache.org/r/15213/diff/3/?file=378199#file378199line97>
> >
> > first argument should be ((PrimitiveObjectInspector) arguments[0])
Done
> On Nov. 9, 2013, 6:44 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java, line 137
> > <https://reviews.apache.org/r/15213/diff/3/?file=378199#file378199line137>
> >
> > this should return null, instead of throwing exception.
Done
- Mohammad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/#review28624
-----------------------------------------------------------
On Nov. 11, 2013, 9:03 p.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15213/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2013, 9:03 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5731
> https://issues.apache.org/jira/browse/HIVE-5731
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> GenericUDF class is the latest and recommended base class for any UDFs.
> This JIRA is to change the current UDFDate* classes extended from GenericUDF.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 8d3a84f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
> ql/src/test/results/clientpositive/udf_to_date.q.out 6ff5ee8
>
> Diff: https://reviews.apache.org/r/15213/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF
for UDFDate* classes
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/#review28624
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java
<https://reviews.apache.org/r/15213/#comment55521>
Shouldn't the outputOI be writableDateOI ?
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java
<https://reviews.apache.org/r/15213/#comment55518>
First argument should be argumentOI.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java
<https://reviews.apache.org/r/15213/#comment55519>
First argument should be argumentOI.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java
<https://reviews.apache.org/r/15213/#comment55520>
Instead of throwing up in parse exception, we should return null in such cases.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java
<https://reviews.apache.org/r/15213/#comment55522>
outputOI should be writableDateOI
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java
<https://reviews.apache.org/r/15213/#comment55523>
First arg should be ((PrimitiveObjectInspector) arguments[0])
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java
<https://reviews.apache.org/r/15213/#comment55524>
Instead of throwing exception, this should return null.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java
<https://reviews.apache.org/r/15213/#comment55528>
In evaluate() you are creating new IntWritable everytime, instead that function should return int and you should do output.set() and return output. This way we will save unnecessary object creation of intWritable for each invocation.
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java
<https://reviews.apache.org/r/15213/#comment55525>
outputOI should be writableOI
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java
<https://reviews.apache.org/r/15213/#comment55526>
first argument should be ((PrimitiveObjectInspector) arguments[0])
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java
<https://reviews.apache.org/r/15213/#comment55527>
this should return null, instead of throwing exception.
- Ashutosh Chauhan
On Nov. 5, 2013, 7:33 p.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15213/
> -----------------------------------------------------------
>
> (Updated Nov. 5, 2013, 7:33 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5731
> https://issues.apache.org/jira/browse/HIVE-5731
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> GenericUDF class is the latest and recommended base class for any UDFs.
> This JIRA is to change the current UDFDate* classes extended from GenericUDF.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 8d3a84f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
> ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
> ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
>
> Diff: https://reviews.apache.org/r/15213/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 15213: HIVE-5731: Use new GenericUDF instead of basic UDF
for UDFDate* classes
Posted by Mohammad Islam <mi...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15213/
-----------------------------------------------------------
(Updated Nov. 5, 2013, 7:33 p.m.)
Review request for hive.
Changes
-------
Addressed build error.
Bugs: HIVE-5731
https://issues.apache.org/jira/browse/HIVE-5731
Repository: hive-git
Description
-------
GenericUDF class is the latest and recommended base class for any UDFs.
This JIRA is to change the current UDFDate* classes extended from GenericUDF.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 8d3a84f
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDate.java 3df453c
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateAdd.java b1b0bf2
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateDiff.java da14c4f
ql/src/java/org/apache/hadoop/hive/ql/udf/UDFDateSub.java c8a1d1f
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDate.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateAdd.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDate.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateAdd.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateDiff.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestGenericUDFDateSub.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateAdd.java f0af069
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateDiff.java 8a6dbc3
ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFDateSub.java fa722a9
Diff: https://reviews.apache.org/r/15213/diff/
Testing
-------
Thanks,
Mohammad Islam