You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Syed Albiz <s....@gmail.com> on 2011/06/13 22:40:58 UTC
Review Request: HIVE-872: Allow BIGINT constants
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
Review request for hive and John Sichi.
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by John Sichi <js...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/#review829
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java
<https://reviews.apache.org/r/889/#comment1796>
assert endsWith "L"
- John
On 2011-06-13 22:41:25, Syed Albiz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/889/
> -----------------------------------------------------------
>
> (Updated 2011-06-13 22:41:25)
>
>
> Review request for hive and John Sichi.
>
>
> Summary
> -------
>
> Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
>
>
> This addresses bug HIVE-872.
> https://issues.apache.org/jira/browse/HIVE-872
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
> ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
> ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/889/diff
>
>
> Testing
> -------
>
> TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
>
>
> Thanks,
>
> Syed
>
>
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by John Sichi <js...@fb.com>.
> On 2011-06-14 19:14:04, John Sichi wrote:
> > Wouldn't it be better to change COALESCE to just check that the two objects have the same type family (both numeric)? That's what happens in standard SQL, and then the widest type is used for the result. This is what UNION ALL should be doing also, although currently it uses the same annoying check as COALESCE.
> >
Also, CAST(0 AS BIGINT) already works the same as 0L.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/#review834
-----------------------------------------------------------
On 2011-06-14 19:04:53, Syed Albiz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/889/
> -----------------------------------------------------------
>
> (Updated 2011-06-14 19:04:53)
>
>
> Review request for hive and John Sichi.
>
>
> Summary
> -------
>
> Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
>
>
> This addresses bug HIVE-872.
> https://issues.apache.org/jira/browse/HIVE-872
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
> ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
> ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/889/diff
>
>
> Testing
> -------
>
> TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
>
>
> Thanks,
>
> Syed
>
>
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
> On 2011-06-14 19:14:04, John Sichi wrote:
> > Wouldn't it be better to change COALESCE to just check that the two objects have the same type family (both numeric)? That's what happens in standard SQL, and then the widest type is used for the result. This is what UNION ALL should be doing also, although currently it uses the same annoying check as COALESCE.
> >
>
> John Sichi wrote:
> Also, CAST(0 AS BIGINT) already works the same as 0L.
>
Yep, I think the request was to provide a shorthand for the explicit CAST() call. After talking more with jhsu it doesn't look like there are any major uses for that shorthand if UDFs apply type widening automatically.
- Syed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/#review834
-----------------------------------------------------------
On 2011-06-14 19:04:53, Syed Albiz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/889/
> -----------------------------------------------------------
>
> (Updated 2011-06-14 19:04:53)
>
>
> Review request for hive and John Sichi.
>
>
> Summary
> -------
>
> Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
>
>
> This addresses bug HIVE-872.
> https://issues.apache.org/jira/browse/HIVE-872
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
> ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
> ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/889/diff
>
>
> Testing
> -------
>
> TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
>
>
> Thanks,
>
> Syed
>
>
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by John Sichi <js...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/#review834
-----------------------------------------------------------
Wouldn't it be better to change COALESCE to just check that the two objects have the same type family (both numeric)? That's what happens in standard SQL, and then the widest type is used for the result. This is what UNION ALL should be doing also, although currently it uses the same annoying check as COALESCE.
- John
On 2011-06-14 19:04:53, Syed Albiz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/889/
> -----------------------------------------------------------
>
> (Updated 2011-06-14 19:04:53)
>
>
> Review request for hive and John Sichi.
>
>
> Summary
> -------
>
> Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
>
>
> This addresses bug HIVE-872.
> https://issues.apache.org/jira/browse/HIVE-872
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
> ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
> ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/889/diff
>
>
> Testing
> -------
>
> TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
>
>
> Thanks,
>
> Syed
>
>
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-21 02:50:57.581129)
Review request for hive and John Sichi.
Changes
-------
Update test cases udf_coalesce.q and union2.q to check that type-widening is not applied incorrectly, use getCommonClassForComparison instead of getCommonClass to compute type widening on union operator (fixing explode_null.q)
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/UnionOperator.java 2462517
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ec816e9
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCoalesce.java f46c16c
ql/src/test/queries/clientnegative/udf_coalesce.q 6d8da79
ql/src/test/queries/clientnegative/union2.q 403d19d
ql/src/test/queries/clientpositive/type_widening.q PRE-CREATION
ql/src/test/results/clientnegative/udf_coalesce.q.out a4c3cab
ql/src/test/results/clientnegative/union2.q.out 16cfe03
ql/src/test/results/clientpositive/type_widening.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-16 02:32:20.778571)
Review request for hive and John Sichi.
Changes
-------
Added ORDER BY to testcase & regenerated output
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/UnionOperator.java 2462517
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ec816e9
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCoalesce.java f46c16c
ql/src/test/queries/clientpositive/type_widening.q PRE-CREATION
ql/src/test/results/clientpositive/type_widening.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by John Sichi <js...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/#review847
-----------------------------------------------------------
ql/src/test/queries/clientpositive/type_widening.q
<https://reviews.apache.org/r/889/#comment1838>
Need ORDER BY
- John
On 2011-06-15 00:27:01, Syed Albiz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/889/
> -----------------------------------------------------------
>
> (Updated 2011-06-15 00:27:01)
>
>
> Review request for hive and John Sichi.
>
>
> Summary
> -------
>
> Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
>
>
> This addresses bug HIVE-872.
> https://issues.apache.org/jira/browse/HIVE-872
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/exec/UnionOperator.java 2462517
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ec816e9
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCoalesce.java f46c16c
> ql/src/test/queries/clientpositive/type_widening.q PRE-CREATION
> ql/src/test/results/clientpositive/type_widening.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/889/diff
>
>
> Testing
> -------
>
> TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
>
>
> Thanks,
>
> Syed
>
>
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-15 00:27:01.842091)
Review request for hive and John Sichi.
Changes
-------
Gut the changes to TypeProcFactory and the grammer, instead amend COALESCE and UNION ALL to allow type widening.
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/exec/UnionOperator.java 2462517
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ec816e9
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCoalesce.java f46c16c
ql/src/test/queries/clientpositive/type_widening.q PRE-CREATION
ql/src/test/results/clientpositive/type_widening.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-14 19:04:53.630834)
Review request for hive and John Sichi.
Changes
-------
After talking to jhsu about this, looks like the motivation for this is to provide a way to coerce int literals into bigints for UDFs that expect arguments of the same type. E.g. COALESCE(0, 1152921504606846976) will currently fail. With this updated patch, COALESCE(0L, 1152921504606846976) passes.
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-13 23:32:55.134796)
Review request for hive and John Sichi.
Changes
-------
Added assert
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed
Re: Review Request: HIVE-872: Allow BIGINT constants
Posted by Syed Albiz <s....@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/889/
-----------------------------------------------------------
(Updated 2011-06-13 22:41:25.002039)
Review request for hive and John Sichi.
Changes
-------
Added a testcase to cover using bigint constants in a couple of scenarios.
Summary
-------
Added a rule to the lexical grammar to allow BIGINT constants ending with 'L', and a clause to the TypeCheckProcFactory to ensure it gets interpreted properly.
This addresses bug HIVE-872.
https://issues.apache.org/jira/browse/HIVE-872
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 9161319
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java dfadb9f
ql/src/test/queries/clientpositive/bigint_const.q PRE-CREATION
ql/src/test/results/clientpositive/bigint_const.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/889/diff
Testing
-------
TestCliDriver passes, previous behaviour was to accept bigint constants specified without 'L', which is also preserved, so adding additional tests for this case seems unnecessary.
Thanks,
Syed