tanishq-chugh commented on code in PR #6366:
URL: https://github.com/apache/hive/pull/6366#discussion_r3077242003
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java:
##########
@@ -1717,6 +1717,10 @@ private boolean
checkExprNodeDescForDecimal64(ExprNodeDesc exprNodeDesc) throws
GenericUDF udf = ((ExprNodeGenericFuncDesc)
exprNodeDesc).getGenericUDF();
Class<?> udfClass = udf.getClass();
if (udf instanceof GenericUDFToDecimal) {
+ ExprNodeDesc child = exprNodeDesc.getChildren().get(0);
+ if (isDecimalFamily(child.getTypeString())) {
Review Comment:
Hi @abstractdog , Thanks for checking this.
I understand that we have the same check in the code following right after
in the for loop, but currently in such casts, the code flow does not reach the
for loop and directly returns true from [Code
Link](https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java#L1719-L1721),
which is not correct for such decimal to decimal casts and causing issue.
Also, if we remove the whole check of `udf instanceof GenericUDFToDecimal`
to make the code flow fall through the code logic ahead, i believe it will
cause performance regression for other casts. For exm: if we have an integer
column here in same case: `CAST(integer_column as DECIMAL(15,1))` , this would
tend to return false in such a case and IMO would be a performance regression.
Please do let me know what are your thoughts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
