morrySnow commented on code in PR #51270:
URL: https://github.com/apache/doris/pull/51270#discussion_r2108313083
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -376,59 +380,83 @@ public static FunctionSignature
dynamicComputePropertiesOfArray(
ArrayType.of(arrayType.getItemType(), arrayType.containsNull()
|| containsNull));
}
- private static FunctionSignature defaultDateTimeV2PrecisionPromotion(
- FunctionSignature signature, List<Expression> arguments) {
- DateTimeV2Type finalType = null;
+ // for time type with precision(now are DateTimeV2Type and TimeV2Type),
+ // we will promote the precision of the type to the maximum precision of
all arguments
+ private static FunctionSignature
defaultTimePrecisionPromotion(FunctionSignature signature,
+ List<Expression> arguments) {
+ ImmutableList.Builder<DataType> newArgTypesBuilder =
ImmutableList.builderWithExpectedSize(signature.arity);
+
+ int finalTypeScale = -1;
for (int i = 0; i < arguments.size(); i++) {
- DataType targetType;
+ DataType signatureArgType;
if (i >= signature.argumentsTypes.size()) {
Preconditions.checkState(signature.getVarArgType().isPresent(),
"argument size larger than signature");
- targetType = signature.getVarArgType().get();
+ signatureArgType = signature.getVarArgType().get();
} else {
- targetType = signature.getArgType(i);
+ signatureArgType = signature.getArgType(i);
}
- List<DataType> argTypes = extractArgumentType(DateTimeV2Type.class,
- targetType, arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
+ List<DataType> nestedTypes =
extractArgumentTypeBySignature(DateTimeV2Type.class,
Review Comment:
why named it as nestedTypes?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -376,59 +380,83 @@ public static FunctionSignature
dynamicComputePropertiesOfArray(
ArrayType.of(arrayType.getItemType(), arrayType.containsNull()
|| containsNull));
}
- private static FunctionSignature defaultDateTimeV2PrecisionPromotion(
- FunctionSignature signature, List<Expression> arguments) {
- DateTimeV2Type finalType = null;
+ // for time type with precision(now are DateTimeV2Type and TimeV2Type),
+ // we will promote the precision of the type to the maximum precision of
all arguments
+ private static FunctionSignature
defaultTimePrecisionPromotion(FunctionSignature signature,
+ List<Expression> arguments) {
+ ImmutableList.Builder<DataType> newArgTypesBuilder =
ImmutableList.builderWithExpectedSize(signature.arity);
+
+ int finalTypeScale = -1;
for (int i = 0; i < arguments.size(); i++) {
- DataType targetType;
+ DataType signatureArgType;
if (i >= signature.argumentsTypes.size()) {
Preconditions.checkState(signature.getVarArgType().isPresent(),
"argument size larger than signature");
- targetType = signature.getVarArgType().get();
+ signatureArgType = signature.getVarArgType().get();
} else {
- targetType = signature.getArgType(i);
+ signatureArgType = signature.getArgType(i);
}
- List<DataType> argTypes = extractArgumentType(DateTimeV2Type.class,
- targetType, arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
+ List<DataType> nestedTypes =
extractArgumentTypeBySignature(DateTimeV2Type.class,
+ signatureArgType, arguments.get(i).getDataType());
+ List<DataType> timeV2Types =
extractArgumentTypeBySignature(TimeV2Type.class,
+ signatureArgType, arguments.get(i).getDataType());
+ nestedTypes.addAll(timeV2Types);
+ if (nestedTypes.isEmpty()) {
+ // if no DateTimeV2Type or TimeV2Type in the argument, no
precision promotion
continue;
}
- for (DataType argType : argTypes) {
+ // for Map or Struct, we have more than one nested type
+ for (DataType argType : nestedTypes) {
Expression arg = arguments.get(i);
- DateTimeV2Type dateTimeV2Type;
+ DataType targetNestedType; // real target type for this nested
type
if (arg instanceof StringLikeLiteral) {
- StringLikeLiteral str = (StringLikeLiteral)
arguments.get(i);
- dateTimeV2Type =
DateTimeV2Type.forTypeFromString(str.getStringValue());
+ StringLikeLiteral str = (StringLikeLiteral) arg;
+ if (argType instanceof TimeV2Type) {
+ targetNestedType =
TimeV2Type.forTypeFromString(str.getStringValue());
+ } else { // must be DateTimeV2Type
+ targetNestedType =
DateTimeV2Type.forTypeFromString(str.getStringValue());
+ }
} else {
- dateTimeV2Type = DateTimeV2Type.forType(argType);
+ if (argType instanceof TimeV2Type) {
+ targetNestedType = TimeV2Type.forType(argType);
+ } else { // must be DateTimeV2Type
+ targetNestedType = DateTimeV2Type.forType(argType);
+ }
}
- if (finalType == null) {
- finalType = dateTimeV2Type;
+ // promote the precision of TimeV2Type to DateTimeV2Type
+ if (finalTypeScale < 0) {
+ if (targetNestedType instanceof DateTimeV2Type) {
Review Comment:
if u think process datetimev2 and timev2's scale togather is make sense. i
think it is better to let them implement a common trait. and the trait has
inferface such as `fromType`, `getScale`, etc..
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/TimeV2Type.java:
##########
@@ -27,7 +29,9 @@
*/
public class TimeV2Type extends PrimitiveType implements RangeScalable {
Review Comment:
maybe we should remove TimeV2Type and retain TimeType
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -376,59 +380,83 @@ public static FunctionSignature
dynamicComputePropertiesOfArray(
ArrayType.of(arrayType.getItemType(), arrayType.containsNull()
|| containsNull));
}
- private static FunctionSignature defaultDateTimeV2PrecisionPromotion(
- FunctionSignature signature, List<Expression> arguments) {
- DateTimeV2Type finalType = null;
+ // for time type with precision(now are DateTimeV2Type and TimeV2Type),
+ // we will promote the precision of the type to the maximum precision of
all arguments
+ private static FunctionSignature
defaultTimePrecisionPromotion(FunctionSignature signature,
+ List<Expression> arguments) {
+ ImmutableList.Builder<DataType> newArgTypesBuilder =
ImmutableList.builderWithExpectedSize(signature.arity);
Review Comment:
why move this here? we should better move it back
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
