morrySnow commented on code in PR #26827:
URL: https://github.com/apache/doris/pull/26827#discussion_r1393732400
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UnixTimestamp.java:
##########
@@ -81,7 +84,28 @@ public boolean nullable() {
if (arity() == 0) {
return false;
}
- return PropagateNullableOnDateLikeV2Args.super.nullable();
Review Comment:
remove PropagateNullableOnDateLikeV2Args trait from this class
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UnixTimestamp.java:
##########
@@ -42,9 +44,10 @@
public class UnixTimestamp extends ScalarFunction
implements ExplicitlyCastableSignature,
PropagateNullableOnDateLikeV2Args, Nondeterministic {
+ // we got changes in @{computeSignature}
Review Comment:
use java doc format comment
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UnixTimestamp.java:
##########
@@ -42,9 +44,10 @@
public class UnixTimestamp extends ScalarFunction
implements ExplicitlyCastableSignature,
PropagateNullableOnDateLikeV2Args, Nondeterministic {
+ // we got changes in @{computeSignature}
private static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(IntegerType.INSTANCE).args(),
-
FunctionSignature.ret(IntegerType.INSTANCE).args(DateTimeV2Type.SYSTEM_DEFAULT),
+ FunctionSignature.ret(DecimalV3Type.createDecimalV3Type(16,
6)).args(DateTimeV2Type.SYSTEM_DEFAULT),
Review Comment:
should update return type of
```java
FunctionSignature.ret(IntegerType.INSTANCE).args(VarcharType.SYSTEM_DEFAULT,
VarcharType.SYSTEM_DEFAULT),
FunctionSignature.ret(IntegerType.INSTANCE).args(StringType.INSTANCE,
StringType.INSTANCE)
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -412,6 +412,11 @@ private static Expression unSafeCast(Expression input,
DataType dataType) {
return promoted;
}
}
+ // adapt scale when from string to datetimev2 with float
+ if (type.isStringLikeType() && dataType.isDateTimeV2Type()) {
+ return recordTypeCoercionForSubQuery(input,
+ DateTimeV2Type.forTypeFromString(((Literal)
input).getStringValue()));
Review Comment:
should not do this, this will lead to wrong type coercion. think about this
scene. `IF(a = b, datetimev2(6)_col, string_literal_with_scale_3)`. then the
expression will be erwrite to `IF(a = b, datetimev2(6)_col,
cast(string_literal_with_scale_3 as datetimev2(3)))`. this is not right.
string_literal should always return scale with 6 to compatible with string
column.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UnixTimestamp.java:
##########
@@ -81,7 +84,28 @@ public boolean nullable() {
if (arity() == 0) {
return false;
}
- return PropagateNullableOnDateLikeV2Args.super.nullable();
+ if (arity() == 1) {
+ return child(0).nullable();
+ }
+ if (arity() == 2 && child(0).getDataType().isStringLikeType() &&
child(1).getDataType().isStringLikeType()) {
+ return true;
+ }
+ return child(0).nullable() || child(1).nullable();
Review Comment:
add comment to explain the nullable logic
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/DateTimeExtractAndTransform.java:
##########
@@ -494,9 +496,13 @@ public static Expression unixTimestamp(DateV2Literal date)
{
return new IntegerLiteral(getTimestamp(date.toJavaDateType()));
}
- @ExecFunction(name = "unix_timestamp", argTypes = {"DATETIMEV2"},
returnType = "INT")
+ @ExecFunction(name = "unix_timestamp", argTypes = { "DATETIMEV2" },
returnType = "DECIMALV3")
public static Expression unixTimestamp(DateTimeV2Literal date) {
- return new IntegerLiteral(getTimestamp(date.toJavaDateType()));
+ if (date.getMicroSecond() == 0) {
+ return new DecimalV3Literal(new
BigDecimal(getTimestamp(date.toJavaDateType()).toString()));
+ }
+ String val = getTimestamp(date.toJavaDateType()).toString() + "." +
date.getMicrosecondString();
+ return new DecimalV3Literal(new BigDecimal(val));
Review Comment:
u should create DecimalV3Literal with DecimalV3Type explicitly, because this
ctor do not ensure return same precision and scale decimal with original
expression. this may lead to wrong result or be crash
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/UnixTimestamp.java:
##########
@@ -81,7 +84,28 @@ public boolean nullable() {
if (arity() == 0) {
return false;
}
- return PropagateNullableOnDateLikeV2Args.super.nullable();
+ if (arity() == 1) {
+ return child(0).nullable();
+ }
+ if (arity() == 2 && child(0).getDataType().isStringLikeType() &&
child(1).getDataType().isStringLikeType()) {
+ return true;
+ }
+ return child(0).nullable() || child(1).nullable();
+ }
+
+ @Override
+ public FunctionSignature computeSignature(FunctionSignature signature) {
+ if (arity() != 1) {
+ return signature.withReturnType(IntegerType.INSTANCE);
Review Comment:
just return original signature is ok
--
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
