lsyldliu commented on code in PR #21026:
URL: https://github.com/apache/flink/pull/21026#discussion_r993131703
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/connectors/hive/HiveDialectITCase.java:
##########
@@ -39,6 +39,8 @@
import org.apache.flink.table.delegation.Parser;
import org.apache.flink.table.functions.hive.HiveGenericUDTFTest;
import
org.apache.flink.table.functions.hive.util.TestSplitUDTFInitializeWithStructObjectInspector;
+import org.apache.flink.table.module.CoreModule;
Review Comment:
This is not used.
##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/HiveParserTypeCheckProcFactory.java:
##########
@@ -1234,6 +1236,46 @@ protected ExprNodeDesc getXpathOrFuncExprNodeDesc(
desc =
ExprNodeGenericFuncDesc.newInstance(
new HiveGenericUDFInternalInterval(),
funcText, children);
+ } else if (genericUDF instanceof GenericUDFOPDivide &&
children.size() == 2) {
+ // special case for GenericUDFOPDivide
+ // if the divisor or dividend is decimal type and the
other one
+ // parameter is int/long literal, the TypeInfo of the
ExprNodeGenericFuncDesc
+ // may be different with inferred result type, which will
cause "Mismatch of
+ // expected output data type 'DECIMAL(..)' and function's
output type
+ // 'DECIMAL(..)'" in
BridgingFunctionGenUtil#verifyOutputType
+
+ // the reason is: in here we got expected result type,
which will consider
+ // int/long literal parameter as actual precision,
+ // but in the phase to infer result type phase, the
+ // int/long literal parameter will always be considered as
max precision. 10 for
+ // int, and 19 for long.
+ // To fix it, in here, we also should consider the
int/long literal parameter as
+ // max precision.
+ ExprNodeDesc exprNodeDesc1 = children.get(0);
+ ExprNodeDesc exprNodeDesc2 = children.get(1);
+ // if one parameter is decimal type, and the other is int
or long
+ if ((isDecimalTypeInfo(exprNodeDesc1)
+ && isIntOrLongTypeInfo(exprNodeDesc2)
Review Comment:
Does short or byte type also need to be considered?
##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/HiveParserTypeCheckProcFactory.java:
##########
@@ -1234,6 +1236,46 @@ protected ExprNodeDesc getXpathOrFuncExprNodeDesc(
desc =
ExprNodeGenericFuncDesc.newInstance(
new HiveGenericUDFInternalInterval(),
funcText, children);
+ } else if (genericUDF instanceof GenericUDFOPDivide &&
children.size() == 2) {
+ // special case for GenericUDFOPDivide
+ // if the divisor or dividend is decimal type and the
other one
+ // parameter is int/long literal, the TypeInfo of the
ExprNodeGenericFuncDesc
+ // may be different with inferred result type, which will
cause "Mismatch of
+ // expected output data type 'DECIMAL(..)' and function's
output type
+ // 'DECIMAL(..)'" in
BridgingFunctionGenUtil#verifyOutputType
+
+ // the reason is: in here we got expected result type,
which will consider
+ // int/long literal parameter as actual precision,
+ // but in the phase to infer result type phase, the
+ // int/long literal parameter will always be considered as
max precision. 10 for
+ // int, and 19 for long.
+ // To fix it, in here, we also should consider the
int/long literal parameter as
+ // max precision.
+ ExprNodeDesc exprNodeDesc1 = children.get(0);
+ ExprNodeDesc exprNodeDesc2 = children.get(1);
+ // if one parameter is decimal type, and the other is int
or long
+ if ((isDecimalTypeInfo(exprNodeDesc1)
+ && isIntOrLongTypeInfo(exprNodeDesc2)
Review Comment:
It would be better to adjust the order
```
&& exprNodeDesc2 instanceof ExprNodeConstantDesc &&
isIntOrLongTypeInfo(exprNodeDesc2)
```
##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/HiveParserTypeCheckProcFactory.java:
##########
@@ -1355,6 +1403,46 @@ private boolean isDescendant(Node ans, Node des) {
return false;
}
+ private boolean isDecimalTypeInfo(ExprNodeDesc exprNodeDesc) {
+ TypeInfo typeInfo = exprNodeDesc.getTypeInfo();
+ return typeInfo instanceof PrimitiveTypeInfo
+ && ((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory()
+ ==
PrimitiveObjectInspector.PrimitiveCategory.DECIMAL;
+ }
+
+ private boolean isIntOrLongTypeInfo(ExprNodeDesc exprNodeDesc) {
+ TypeInfo typeInfo = exprNodeDesc.getTypeInfo();
+ if (!(typeInfo instanceof PrimitiveTypeInfo)) {
+ return false;
+ }
+ return ((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory()
+ == PrimitiveObjectInspector.PrimitiveCategory.INT
+ || ((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory()
+ == PrimitiveObjectInspector.PrimitiveCategory.LONG;
+ }
+
+ private boolean canSafeFoldToConstant(ExprNodeConstantDesc
constantExpr) {
+ // if it's not primitive type, can't be folded to constant safely
+ boolean isPrimitiveTyp = constantExpr.getTypeInfo() instanceof
PrimitiveTypeInfo;
+ if (!isPrimitiveTyp) {
+ return false;
+ }
+ // if it's binary type, can't be folded to constant safely
+ boolean isBinaryConstant =
+ constantExpr.getTypeInfo() instanceof PrimitiveTypeInfo
+ && (((PrimitiveTypeInfo)
constantExpr.getTypeInfo())
+ .getPrimitiveCategory()
+ ==
PrimitiveObjectInspector.PrimitiveCategory.BINARY);
+ if (isBinaryConstant) {
+ return false;
+ }
+ // if it's Double.NAN, can't be folded to constant safely
+ boolean isNAN =
+ constantExpr.getValue() instanceof Double
Review Comment:
Float also maybe NaN, does the `constantExpr.getValue()` is Float type?
--
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]
