You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/10/12 08:57:44 UTC
[GitHub] [flink] lsyldliu commented on a diff in pull request #21026: [FLINK-29590][hive] fix literal issue in Hive dialect
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org