morrySnow commented on code in PR #49540:
URL: https://github.com/apache/doris/pull/49540#discussion_r2017826216
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -100,11 +162,41 @@ public boolean isPositive() {
@Override
public int getMonotonicFunctionChildIndex() {
+ if (getArgument(0).getDataType().isDateLikeType()) {
+ return 0;
+ } else if (getArgument(1).getDataType().isDateLikeType()) {
+ return 1;
+ }
+ boolean firstArgIsStringLiteral =
+ getArgument(0).isConstant() && getArgument(0) instanceof
VarcharLiteral;
+ boolean secondArgIsStringLiteral =
+ getArgument(1).isConstant() && getArgument(1) instanceof
VarcharLiteral;
+ if (firstArgIsStringLiteral && !secondArgIsStringLiteral) {
+ return 1;
+ } else if (!firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ return 0;
+ } else if (firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ boolean timeUnitIsFirst = legalTimeUint.contains(((VarcharLiteral)
getArgument(0))
+ .getStringValue().toLowerCase());
+ return timeUnitIsFirst ? 1 : 0;
+ }
return 0;
Review Comment:
these code is useless, when come to this function, arguments' type must same
with signature's.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -62,15 +70,26 @@ public DateTrunc(Expression arg0, Expression arg1) {
@Override
public void checkLegalityBeforeTypeCoercion() {
- if (getArgument(1).isConstant() == false || !(getArgument(1)
instanceof VarcharLiteral)) {
- throw new AnalysisException("the second parameter of "
+ boolean firstArgIsStringLiteral =
+ getArgument(0).isConstant() && getArgument(0) instanceof
VarcharLiteral;
+ boolean secondArgIsStringLiteral =
+ getArgument(1).isConstant() && getArgument(1) instanceof
VarcharLiteral;
+ if (!firstArgIsStringLiteral && !secondArgIsStringLiteral) {
+ throw new AnalysisException("the time unit parameter of "
+ getName() + " function must be a string constant: " +
toSql());
- }
- final String constParam = ((VarcharLiteral)
getArgument(1)).getStringValue().toLowerCase();
- if (!Lists.newArrayList("year", "quarter", "month", "week", "day",
"hour", "minute", "second")
- .contains(constParam)) {
- throw new AnalysisException("date_trunc function second param only
support argument is "
- + "year|quarter|month|week|day|hour|minute|second");
+ } else if (firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ if (!legalTimeUint.contains(((VarcharLiteral)
getArgument(0)).getStringValue().toLowerCase())
+ && !legalTimeUint.contains(((VarcharLiteral)
getArgument(1)).getStringValue().toLowerCase())) {
+ throw new AnalysisException("date_trunc function time unit
param only support argument is "
+ + "year|quarter|month|week|day|hour|minute|second");
Review Comment:
```suggestion
+ String.join("|", legalTimeUint));
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -84,8 +103,51 @@ public DateTrunc withChildren(List<Expression> children) {
}
@Override
- public List<FunctionSignature> getSignatures() {
- return SIGNATURES;
+ public FunctionSignature customSignature() {
Review Comment:
could simplify it by
```java
if (getArgment(0).getDataType().isDateLikeType()) {
return FunctionSignature.ret(getArgment(0).getDataType())
.args(getArgment(0).getDataType(), VarcharType.SYSTEM_DEFAULT);
} else if (getArgment(1).getDataType().isDateLikeType()) {
...
} else if (firstArgIsStringLiteral && secondArgIsStringLiteral) {
...
} else if (firstArgIsStringLiteral) {
return FunctionSignature.ret(DateTimeV2Type.SYSTEM_DEFAULT)
.args(VarcharType.SYSTEM_DEFAULT, DateTimeV2Type.SYSTEM_DEFAULT);
} else if (secondArgIsStringLiteral) {
...
} else {
...
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -42,17 +42,25 @@
* ScalarFunction 'date_trunc'. This class is generated by GenerateFunction.
*/
public class DateTrunc extends ScalarFunction
- implements BinaryExpression, ExplicitlyCastableSignature,
AlwaysNullable, Monotonic {
+ implements BinaryExpression, AlwaysNullable, Monotonic,
CustomSignature {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DateTimeV2Type.SYSTEM_DEFAULT)
.args(DateTimeV2Type.SYSTEM_DEFAULT,
VarcharType.SYSTEM_DEFAULT),
FunctionSignature.ret(DateTimeType.INSTANCE).args(DateTimeType.INSTANCE,
VarcharType.SYSTEM_DEFAULT),
FunctionSignature.ret(DateV2Type.INSTANCE)
.args(DateV2Type.INSTANCE, VarcharType.SYSTEM_DEFAULT),
- FunctionSignature.ret(DateType.INSTANCE).args(DateType.INSTANCE,
VarcharType.SYSTEM_DEFAULT)
+ FunctionSignature.ret(DateType.INSTANCE).args(DateType.INSTANCE,
VarcharType.SYSTEM_DEFAULT),
+ FunctionSignature.ret(DateTimeV2Type.SYSTEM_DEFAULT)
+ .args(VarcharType.SYSTEM_DEFAULT,
DateTimeV2Type.SYSTEM_DEFAULT),
+
FunctionSignature.ret(DateTimeType.INSTANCE).args(VarcharType.SYSTEM_DEFAULT,
DateTimeType.INSTANCE),
+
FunctionSignature.ret(DateV2Type.INSTANCE).args(VarcharType.SYSTEM_DEFAULT,
DateV2Type.INSTANCE),
+
FunctionSignature.ret(DateType.INSTANCE).args(VarcharType.SYSTEM_DEFAULT,
DateType.INSTANCE)
);
+ private static List<String> legalTimeUint =
+ Lists.newArrayList("year", "quarter", "month", "week", "day",
"hour", "minute", "second");
Review Comment:
```suggestion
private final static List<String> LEGAL_TIME_UNIT =
ImmutableList.of("year", "quarter", "month", "week", "day",
"hour", "minute", "second");
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -100,11 +162,41 @@ public boolean isPositive() {
@Override
public int getMonotonicFunctionChildIndex() {
+ if (getArgument(0).getDataType().isDateLikeType()) {
+ return 0;
+ } else if (getArgument(1).getDataType().isDateLikeType()) {
+ return 1;
+ }
+ boolean firstArgIsStringLiteral =
+ getArgument(0).isConstant() && getArgument(0) instanceof
VarcharLiteral;
+ boolean secondArgIsStringLiteral =
+ getArgument(1).isConstant() && getArgument(1) instanceof
VarcharLiteral;
+ if (firstArgIsStringLiteral && !secondArgIsStringLiteral) {
+ return 1;
+ } else if (!firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ return 0;
+ } else if (firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ boolean timeUnitIsFirst = legalTimeUint.contains(((VarcharLiteral)
getArgument(0))
+ .getStringValue().toLowerCase());
+ return timeUnitIsFirst ? 1 : 0;
+ }
return 0;
}
@Override
public Expression withConstantArgs(Expression literal) {
- return new DateTrunc(literal, child(1));
+ boolean firstArgIsStringLiteral =
+ getArgument(0).isConstant() && getArgument(0) instanceof
VarcharLiteral;
+ boolean secondArgIsStringLiteral =
+ getArgument(1).isConstant() && getArgument(1) instanceof
VarcharLiteral;
+ if (firstArgIsStringLiteral && secondArgIsStringLiteral) {
+ if (legalTimeUint.contains(((VarcharLiteral)
getArgument(0)).getStringValue().toLowerCase())) {
+ return new DateTrunc(child(0), literal);
+ } else {
+ return new DateTrunc(literal, child(1));
+ }
+ }
+ return firstArgIsStringLiteral
+ ? new DateTrunc(child(0), literal) : new DateTrunc(literal,
child(1));
Review Comment:
use argument's datatype, just like what u do in
`getMonotonicFunctionChildIndex`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateTrunc.java:
##########
@@ -62,15 +70,26 @@ public DateTrunc(Expression arg0, Expression arg1) {
@Override
public void checkLegalityBeforeTypeCoercion() {
- if (getArgument(1).isConstant() == false || !(getArgument(1)
instanceof VarcharLiteral)) {
- throw new AnalysisException("the second parameter of "
+ boolean firstArgIsStringLiteral =
+ getArgument(0).isConstant() && getArgument(0) instanceof
VarcharLiteral;
Review Comment:
```suggestion
getArgument(0) instanceof StringLikeLiteral;
```
--
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]
