You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Navis Ryu <na...@nexr.com> on 2011/12/08 10:33:37 UTC
Review Request: HIVE-2586 Implement literal float/double type and make
possible to compare
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3073/
-----------------------------------------------------------
Review request for hive, John Sichi and Carl Steinbach.
Summary
-------
Is there any reason should not to implement comparing float/double?
This addresses bug HIVE-2586.
https://issues.apache.org/jira/browse/HIVE-2586
Diffs
-----
data/files/floatdouble.txt PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g eecd9e7
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 59e55ae
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 3fb3879
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqual.java dc4670e
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrGreaterThan.java f44f353
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrLessThan.java 7d74e82
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPGreaterThan.java 47fceb1
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPLessThan.java 12369a8
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNotEqual.java 22b3bef
ql/src/test/queries/clientpositive/compare_float_double.q PRE-CREATION
ql/src/test/results/clientpositive/compare_float_double.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/3073/diff
Testing
-------
added test : compare_float_double.q
Thanks,
Navis
Re: Review Request: HIVE-2586 Implement literal float/double type and make
possible to compare
Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3073/
-----------------------------------------------------------
(Updated 2011-12-20 07:22:34.645716)
Review request for hive, John Sichi and Carl Steinbach.
Summary
-------
Is there any reason should not to implement comparing float/double?
This addresses bug HIVE-2586.
https://issues.apache.org/jira/browse/HIVE-2586
Diffs (updated)
-----
data/files/employee.txt PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 888bf47
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 59e55ae
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 3fb3879
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqual.java dc4670e
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrGreaterThan.java f44f353
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrLessThan.java 7d74e82
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPGreaterThan.java 47fceb1
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPLessThan.java 12369a8
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNotEqual.java 22b3bef
ql/src/test/org/apache/hadoop/hive/ql/QTestUtil.java 498a293
ql/src/test/queries/clientpositive/compare_float_double.q PRE-CREATION
ql/src/test/results/clientpositive/compare_float_double.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/3073/diff
Testing
-------
added test : compare_float_double.q
Thanks,
Navis
Re: Review Request: HIVE-2586 Implement literal float/double type and make
possible to compare
Posted by Carl Steinbach <ca...@cloudera.com>.
> On 2011-12-19 21:21:20, Carl Steinbach wrote:
> > ql/src/test/queries/clientpositive/compare_float_double.q, line 1
> > <https://reviews.apache.org/r/3073/diff/2/?file=63280#file63280line1>
> >
> > Please move these tests to ops_comparison.q. I don't think there's much benefit to adding a new data file with float data, and loading data into the table adds a lot of time to the test.
> >
> > Also, this only provides coverage for the changes in GenericUDFOPEqual. Please add coverage for the other operators that you modified.
>
> Navis Ryu wrote:
> I thought for meaningful test column should be compared to constant/column instead of comparing constant to constant and I couldn't find any existing table contains float/double column. All agree on you except that.
Ok, in that case can you please add an "all_types" table that contains one column per primitive type (e.g. one float column, one int column, etc), and add code to QTestUtil so that this table is automatically created for each CliDriver test?
- Carl
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3073/#review3982
-----------------------------------------------------------
On 2011-12-08 09:33:37, Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3073/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 09:33:37)
>
>
> Review request for hive, John Sichi and Carl Steinbach.
>
>
> Summary
> -------
>
> Is there any reason should not to implement comparing float/double?
>
>
> This addresses bug HIVE-2586.
> https://issues.apache.org/jira/browse/HIVE-2586
>
>
> Diffs
> -----
>
> data/files/floatdouble.txt PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g eecd9e7
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 59e55ae
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 3fb3879
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqual.java dc4670e
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrGreaterThan.java f44f353
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrLessThan.java 7d74e82
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPGreaterThan.java 47fceb1
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPLessThan.java 12369a8
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNotEqual.java 22b3bef
> ql/src/test/queries/clientpositive/compare_float_double.q PRE-CREATION
> ql/src/test/results/clientpositive/compare_float_double.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3073/diff
>
>
> Testing
> -------
>
> added test : compare_float_double.q
>
>
> Thanks,
>
> Navis
>
>
Re: Review Request: HIVE-2586 Implement literal float/double type and make
possible to compare
Posted by Navis Ryu <na...@nexr.com>.
> On 2011-12-19 21:21:20, Carl Steinbach wrote:
> > ql/src/test/queries/clientpositive/compare_float_double.q, line 1
> > <https://reviews.apache.org/r/3073/diff/2/?file=63280#file63280line1>
> >
> > Please move these tests to ops_comparison.q. I don't think there's much benefit to adding a new data file with float data, and loading data into the table adds a lot of time to the test.
> >
> > Also, this only provides coverage for the changes in GenericUDFOPEqual. Please add coverage for the other operators that you modified.
I thought for meaningful test column should be compared to constant/column instead of comparing constant to constant and I couldn't find any existing table contains float/double column. All agree on you except that.
- Navis
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3073/#review3982
-----------------------------------------------------------
On 2011-12-08 09:33:37, Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3073/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 09:33:37)
>
>
> Review request for hive, John Sichi and Carl Steinbach.
>
>
> Summary
> -------
>
> Is there any reason should not to implement comparing float/double?
>
>
> This addresses bug HIVE-2586.
> https://issues.apache.org/jira/browse/HIVE-2586
>
>
> Diffs
> -----
>
> data/files/floatdouble.txt PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g eecd9e7
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 59e55ae
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 3fb3879
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqual.java dc4670e
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrGreaterThan.java f44f353
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrLessThan.java 7d74e82
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPGreaterThan.java 47fceb1
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPLessThan.java 12369a8
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNotEqual.java 22b3bef
> ql/src/test/queries/clientpositive/compare_float_double.q PRE-CREATION
> ql/src/test/results/clientpositive/compare_float_double.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3073/diff
>
>
> Testing
> -------
>
> added test : compare_float_double.q
>
>
> Thanks,
>
> Navis
>
>
Re: Review Request: HIVE-2586 Implement literal float/double type and make
possible to compare
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3073/#review3982
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
<https://reviews.apache.org/r/3073/#comment8991>
Please change this to "FloatLiteral" and "DoubleLiteral".
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java
<https://reviews.apache.org/r/3073/#comment8992>
Please update this comment.
ql/src/test/queries/clientpositive/compare_float_double.q
<https://reviews.apache.org/r/3073/#comment8993>
Please move these tests to ops_comparison.q. I don't think there's much benefit to adding a new data file with float data, and loading data into the table adds a lot of time to the test.
Also, this only provides coverage for the changes in GenericUDFOPEqual. Please add coverage for the other operators that you modified.
- Carl
On 2011-12-08 09:33:37, Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3073/
> -----------------------------------------------------------
>
> (Updated 2011-12-08 09:33:37)
>
>
> Review request for hive, John Sichi and Carl Steinbach.
>
>
> Summary
> -------
>
> Is there any reason should not to implement comparing float/double?
>
>
> This addresses bug HIVE-2586.
> https://issues.apache.org/jira/browse/HIVE-2586
>
>
> Diffs
> -----
>
> data/files/floatdouble.txt PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g eecd9e7
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 59e55ae
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 3fb3879
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqual.java dc4670e
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrGreaterThan.java f44f353
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPEqualOrLessThan.java 7d74e82
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPGreaterThan.java 47fceb1
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPLessThan.java 12369a8
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNotEqual.java 22b3bef
> ql/src/test/queries/clientpositive/compare_float_double.q PRE-CREATION
> ql/src/test/results/clientpositive/compare_float_double.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3073/diff
>
>
> Testing
> -------
>
> added test : compare_float_double.q
>
>
> Thanks,
>
> Navis
>
>