xiaokang commented on code in PR #28233:
URL: https://github.com/apache/doris/pull/28233#discussion_r1430892819
##########
fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java:
##########
@@ -88,13 +88,11 @@ public boolean matchesType(Type t) {
return false;
}
- // Array(Null) is a virtual Array type, can match any Array(...) type
- if (itemType.isNull() || ((ArrayType) t).getItemType().isNull()) {
- return true;
+ if (((ArrayType) t).getContainsNull() != getContainsNull()) {
Review Comment:
what's the sematics of getContainsNull() in ArrayType?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -386,16 +386,41 @@ public Function specializeTemplateFunction(Function
templateFunction, Function r
throw new TypeException(templateFunction
+ " is not support for template since it's not
a ScalarFunction");
}
- Type[] args = specializedFunction.getArgs();
+ ArrayList<Type> args = new ArrayList<>();
+ Collections.addAll(args, specializedFunction.getArgs());
Map<String, Type> specializedTypeMap = Maps.newHashMap();
boolean enableDecimal256 = SessionVariable.getEnableDecimal256();
- for (int i = 0; i < args.length; i++) {
- if (args[i].hasTemplateType()) {
+ int i = 0;
+ for (; i < args.size(); i++) {
+ if (args.get(i).hasTemplateType()) {
hasTemplateType = true;
- args[i] =
args[i].specializeTemplateType(requestFunction.getArgs()[i],
specializedTypeMap, false,
- enableDecimal256);
+ // if args[i] is template type, and
requestFunction.getArgs()[i] NULL_TYPE, we need call function
+ // deduce to get the specific type
+ Type deduceType = requestFunction.getArgs()[i];
+ if (requestFunction.getArgs()[i].isNull()
+ || (requestFunction.getArgs()[i] instanceof
ArrayType
+ && ((ArrayType)
requestFunction.getArgs()[i]).getItemType().isNull())
+ &&
FunctionTypeDeducers.DEDUCERS.containsKey(specializedFunction.functionName())) {
+ deduceType =
FunctionTypeDeducers.deduce(specializedFunction.functionName(), i,
requestFunction.getArgs());
+ args.set(i,
args.get(i).specializeTemplateType(deduceType == null ?
requestFunction.getArgs()[i]
+ : deduceType, specializedTypeMap, false,
enableDecimal256));
+ } else {
+ args.set(i,
args.get(i).specializeTemplateType(requestFunction.getArgs()[i],
+ specializedTypeMap, false, enableDecimal256));
+ }
}
}
+ // here need to support varArgs template according to request data
+ if (specializedFunction.hasVarArgs() && i <
requestFunction.getNumArgs()) {
+ for (; i < requestFunction.getNumArgs(); i++) {
+ if (requestFunction.getArgs()[i].isNull()) {
Review Comment:
add comment to explain
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/IsNullPredicate.java:
##########
@@ -47,17 +47,14 @@ public static void initBuiltins(FunctionSet functionSet) {
functionSet.addBuiltinBothScalaAndVectorized(ScalarFunction.createBuiltinOperator(IS_NOT_NULL,
null, Lists.newArrayList(t), Type.BOOLEAN,
NullableMode.ALWAYS_NOT_NULLABLE));
+ }
+ // for array type
+ for (Type complexType : Lists.newArrayList(Type.ARRAY, Type.MAP,
Type.GENERIC_STRUCT)) {
+
functionSet.addBuiltinBothScalaAndVectorized(ScalarFunction.createBuiltinOperator(IS_NULL,
null,
+ Lists.newArrayList(complexType), Type.BOOLEAN,
NullableMode.ALWAYS_NOT_NULLABLE));
- // for array type
- for (Type complexType : Lists.newArrayList(Type.ARRAY, Type.MAP,
Type.GENERIC_STRUCT)) {
Review Comment:
Is it added mutiple times?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -448,7 +473,20 @@ public static boolean isCastMatchAllowed(Function desc,
Function candicate) {
final Type[] candicateArgTypes = candicate.getArgs();
if (!(descArgTypes[0] instanceof ScalarType)
|| !(candicateArgTypes[0] instanceof ScalarType)) {
- if (candicateArgTypes[0] instanceof ArrayType ||
candicateArgTypes[0] instanceof MapType) {
+ if (candicateArgTypes[0] instanceof ArrayType) {
+ // match is exactly type. but for null type , with in
array|map elem can not return true, because for
+ // be will make null_type to uint8
Review Comment:
BE
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -386,16 +386,41 @@ public Function specializeTemplateFunction(Function
templateFunction, Function r
throw new TypeException(templateFunction
+ " is not support for template since it's not
a ScalarFunction");
}
- Type[] args = specializedFunction.getArgs();
+ ArrayList<Type> args = new ArrayList<>();
+ Collections.addAll(args, specializedFunction.getArgs());
Map<String, Type> specializedTypeMap = Maps.newHashMap();
boolean enableDecimal256 = SessionVariable.getEnableDecimal256();
- for (int i = 0; i < args.length; i++) {
- if (args[i].hasTemplateType()) {
+ int i = 0;
+ for (; i < args.size(); i++) {
+ if (args.get(i).hasTemplateType()) {
hasTemplateType = true;
- args[i] =
args[i].specializeTemplateType(requestFunction.getArgs()[i],
specializedTypeMap, false,
- enableDecimal256);
+ // if args[i] is template type, and
requestFunction.getArgs()[i] NULL_TYPE, we need call function
+ // deduce to get the specific type
+ Type deduceType = requestFunction.getArgs()[i];
+ if (requestFunction.getArgs()[i].isNull()
+ || (requestFunction.getArgs()[i] instanceof
ArrayType
Review Comment:
add () and indent to be more readable
##########
fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java:
##########
@@ -88,13 +88,11 @@ public boolean matchesType(Type t) {
return false;
}
- // Array(Null) is a virtual Array type, can match any Array(...) type
- if (itemType.isNull() || ((ArrayType) t).getItemType().isNull()) {
- return true;
+ if (((ArrayType) t).getContainsNull() != getContainsNull()) {
Review Comment:
What's the difference to itemType.isNull() ?
##########
fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java:
##########
@@ -2252,22 +2253,6 @@ public static boolean matchExactType(Type type1, Type
type2, boolean ignorePreci
return
isSameDecimalTypeWithDifferentPrecision(((ScalarType) type2).decimalPrecision(),
((ScalarType) type1).decimalPrecision());
}
- } else if (type2.isArrayType()) {
Review Comment:
It's moved to ArrayType.matchesType() but matchesType() is not invoked here.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/StructLiteral.java:
##########
@@ -95,6 +97,11 @@ public String getStringValueInFe() {
@Override
protected void toThrift(TExprNode msg) {
msg.node_type = TExprNodeType.STRUCT_LITERAL;
+ ((StructType) type).getFields().forEach(v ->
msg.setChildType(v.getType().getPrimitiveType().toThrift()));
Review Comment:
How it works when the code is missing before?
--
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]
