You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Chaoyu Tang <ct...@gmail.com> on 2015/04/13 14:10:01 UTC
Review Request 33128: HIVE-10313:Literal Decimal ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/
-----------------------------------------------------------
Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
Bugs: HIVE-10313
https://issues.apache.org/jira/browse/HIVE-10313
Repository: hive-git
Description
-------
In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
==
else if (expr.getText().endsWith("BD")) {
// Literal decimal
String strVal = expr.getText().substring(0, expr.getText().length() - 2);
HiveDecimal hd = HiveDecimal.create(strVal);
int prec = 1;
int scale = 0;
if (hd != null) {
prec = hd.precision();
scale = hd.scale();
}
DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
return new ExprNodeConstantDesc(typeInfo, strVal);
}
==
It shoudl be created from HiveDecimal hd instread.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
Diff: https://reviews.apache.org/r/33128/diff/
Testing
-------
Manually tests
kick off precommit build tests
Thanks,
Chaoyu Tang
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Chaoyu Tang <ct...@gmail.com>.
> On April 15, 2015, 10:26 p.m., Jimmy Xiang wrote:
> > ql/src/test/results/clientpositive/literal_decimal.q.out, line 17
> > <https://reviews.apache.org/r/33128/diff/2/?file=925443#file925443line17>
> >
> > Why does the output changed to null?
The input is 1E99BD in literal_decimal.q, the output should be 1E99 instead of the original "null". It is actually the problem is (using string instead of HiveDecimal). Thanks
- Chaoyu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/#review80273
-----------------------------------------------------------
On April 13, 2015, 4:51 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33128/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:51 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
>
>
> Bugs: HIVE-10313
> https://issues.apache.org/jira/browse/HIVE-10313
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
> ==
> else if (expr.getText().endsWith("BD")) {
> // Literal decimal
> String strVal = expr.getText().substring(0, expr.getText().length() - 2);
> HiveDecimal hd = HiveDecimal.create(strVal);
> int prec = 1;
> int scale = 0;
> if (hd != null) {
> prec = hd.precision();
> scale = hd.scale();
> }
> DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
> return new ExprNodeConstantDesc(typeInfo, strVal);
> }
> ==
> It shoudl be created from HiveDecimal hd instread.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
> ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
> ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
> ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
>
> Diff: https://reviews.apache.org/r/33128/diff/
>
>
> Testing
> -------
>
> Manually tests
> kick off precommit build tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Jimmy Xiang <jx...@cloudera.com>.
> On April 15, 2015, 10:26 p.m., Jimmy Xiang wrote:
> > ql/src/test/results/clientpositive/literal_decimal.q.out, line 17
> > <https://reviews.apache.org/r/33128/diff/2/?file=925443#file925443line17>
> >
> > Why does the output changed to null?
>
> Chaoyu Tang wrote:
> The input is 1E99BD in literal_decimal.q, the output should be 1E99 instead of the original "null". It is actually the problem is (using string instead of HiveDecimal). Thanks
Cool. The right side is the new one, right? It seems the original is 1E99?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/#review80273
-----------------------------------------------------------
On April 13, 2015, 4:51 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33128/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:51 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
>
>
> Bugs: HIVE-10313
> https://issues.apache.org/jira/browse/HIVE-10313
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
> ==
> else if (expr.getText().endsWith("BD")) {
> // Literal decimal
> String strVal = expr.getText().substring(0, expr.getText().length() - 2);
> HiveDecimal hd = HiveDecimal.create(strVal);
> int prec = 1;
> int scale = 0;
> if (hd != null) {
> prec = hd.precision();
> scale = hd.scale();
> }
> DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
> return new ExprNodeConstantDesc(typeInfo, strVal);
> }
> ==
> It shoudl be created from HiveDecimal hd instread.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
> ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
> ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
> ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
>
> Diff: https://reviews.apache.org/r/33128/diff/
>
>
> Testing
> -------
>
> Manually tests
> kick off precommit build tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Chaoyu Tang <ct...@gmail.com>.
> On April 15, 2015, 10:26 p.m., Jimmy Xiang wrote:
> > ql/src/test/results/clientpositive/literal_decimal.q.out, line 17
> > <https://reviews.apache.org/r/33128/diff/2/?file=925443#file925443line17>
> >
> > Why does the output changed to null?
>
> Chaoyu Tang wrote:
> The input is 1E99BD in literal_decimal.q, the output should be 1E99 instead of the original "null". It is actually the problem is (using string instead of HiveDecimal). Thanks
>
> Jimmy Xiang wrote:
> Cool. The right side is the new one, right? It seems the original is 1E99?
Yes, Jimmy, you are right. The original "1E99" shown in Select Operator expression was from the strVal but it should have been null if it were from HiveDecimal, since 1E99 has become null during query parsing. When a HiveDecimal is created from 1E99, its precision will be 100 which exceeds the maximun HiveDecimal precision 38, so it is null after normalization. See related methods in HiveDecimal.java:
public static HiveDecimal create(String dec)
private static BigDecimal normalize(BigDecimal bd, boolean allowRounding) {
private static BigDecimal trim(BigDecimal d)
--
The patch is actually not intended to address above issue, but for an issue first found in the conversion between decimals. For example, if I convert the value of ExprNodeConstantDesc created from a decimal 1111.22BD (typeinfo decimal(6,2)) to a decimal (10,4). I saw the ClassCastException error:
---
ERROR ql.Driver: FAILED: ClassCastException java.lang.String cannot be cast to org.apache.hadoop.hive.common.type.HiveDecimal
java.lang.ClassCastException: java.lang.String cannot be cast to org.apache.hadoop.hive.common.type.HiveDecimal
at org.apache.hadoop.hive.serde2.objectinspector.primitive.JavaHiveDecimalObjectInspector.getPrimitiveJavaObject(JavaHiveDecimalObjectInspector.java:55)
at org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorUtils.getHiveDecimal(PrimitiveObjectInspectorUtils.java:1001)
at org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorConverter$HiveDecimalConverter.convert(PrimitiveObjectInspectorConverter.java:349
...
The cause is that ExprNodeConstantDesc contains value of type string instead of HiveDecimal. Actually I think it is quite obvious that ExprNodeConstantDesc should be created with a value of type typeInfo in constructor ExprNodeConstantDesc(TypeInfo typeInfo, Object value).
- Chaoyu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/#review80273
-----------------------------------------------------------
On April 13, 2015, 4:51 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33128/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:51 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
>
>
> Bugs: HIVE-10313
> https://issues.apache.org/jira/browse/HIVE-10313
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
> ==
> else if (expr.getText().endsWith("BD")) {
> // Literal decimal
> String strVal = expr.getText().substring(0, expr.getText().length() - 2);
> HiveDecimal hd = HiveDecimal.create(strVal);
> int prec = 1;
> int scale = 0;
> if (hd != null) {
> prec = hd.precision();
> scale = hd.scale();
> }
> DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
> return new ExprNodeConstantDesc(typeInfo, strVal);
> }
> ==
> It shoudl be created from HiveDecimal hd instread.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
> ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
> ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
> ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
>
> Diff: https://reviews.apache.org/r/33128/diff/
>
>
> Testing
> -------
>
> Manually tests
> kick off precommit build tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Jimmy Xiang <jx...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/#review80273
-----------------------------------------------------------
ql/src/test/results/clientpositive/literal_decimal.q.out
<https://reviews.apache.org/r/33128/#comment130082>
Why does the output changed to null?
- Jimmy Xiang
On April 13, 2015, 4:51 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33128/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:51 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
>
>
> Bugs: HIVE-10313
> https://issues.apache.org/jira/browse/HIVE-10313
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
> ==
> else if (expr.getText().endsWith("BD")) {
> // Literal decimal
> String strVal = expr.getText().substring(0, expr.getText().length() - 2);
> HiveDecimal hd = HiveDecimal.create(strVal);
> int prec = 1;
> int scale = 0;
> if (hd != null) {
> prec = hd.precision();
> scale = hd.scale();
> }
> DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
> return new ExprNodeConstantDesc(typeInfo, strVal);
> }
> ==
> It shoudl be created from HiveDecimal hd instread.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
> ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
> ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
> ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
>
> Diff: https://reviews.apache.org/r/33128/diff/
>
>
> Testing
> -------
>
> Manually tests
> kick off precommit build tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Jimmy Xiang <jx...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/#review80341
-----------------------------------------------------------
Ship it!
Ship It!
- Jimmy Xiang
On April 13, 2015, 4:51 p.m., Chaoyu Tang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33128/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:51 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
>
>
> Bugs: HIVE-10313
> https://issues.apache.org/jira/browse/HIVE-10313
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
> ==
> else if (expr.getText().endsWith("BD")) {
> // Literal decimal
> String strVal = expr.getText().substring(0, expr.getText().length() - 2);
> HiveDecimal hd = HiveDecimal.create(strVal);
> int prec = 1;
> int scale = 0;
> if (hd != null) {
> prec = hd.precision();
> scale = hd.scale();
> }
> DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
> return new ExprNodeConstantDesc(typeInfo, strVal);
> }
> ==
> It shoudl be created from HiveDecimal hd instread.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
> ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
> ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
> ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
>
> Diff: https://reviews.apache.org/r/33128/diff/
>
>
> Testing
> -------
>
> Manually tests
> kick off precommit build tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 33128: HIVE-10313:Literal Decimal
ExprNodeConstantDesc
should contain value of HiveDecimal instead of String
Posted by Chaoyu Tang <ct...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33128/
-----------------------------------------------------------
(Updated April 13, 2015, 4:51 p.m.)
Review request for hive, Ashutosh Chauhan, Szehon Ho, and Xuefu Zhang.
Changes
-------
Updated two failed test outputs
Bugs: HIVE-10313
https://issues.apache.org/jira/browse/HIVE-10313
Repository: hive-git
Description
-------
In TyepCheckProcFactory.NumExprProcessor, the ExprNodeConstantDesc is currently created from strVal:
==
else if (expr.getText().endsWith("BD")) {
// Literal decimal
String strVal = expr.getText().substring(0, expr.getText().length() - 2);
HiveDecimal hd = HiveDecimal.create(strVal);
int prec = 1;
int scale = 0;
if (hd != null) {
prec = hd.precision();
scale = hd.scale();
}
DecimalTypeInfo typeInfo = TypeInfoFactory.getDecimalTypeInfo(prec, scale);
return new ExprNodeConstantDesc(typeInfo, strVal);
}
==
It shoudl be created from HiveDecimal hd instread.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 79d38bc
ql/src/test/results/clientpositive/literal_decimal.q.out b2a23cf
ql/src/test/results/clientpositive/tez/vector_decimal_2.q.out e67ab7b
ql/src/test/results/clientpositive/vector_decimal_2.q.out b22d00c
Diff: https://reviews.apache.org/r/33128/diff/
Testing
-------
Manually tests
kick off precommit build tests
Thanks,
Chaoyu Tang