[BugFix] Support cast constant time type to DateTime #55804
Conversation
|
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
| double second = startTime.getHour() * 3600D + startTime.getMinute() * 60D + startTime.getSecond(); | ||
| return ConstantOperator.createTime(second); | ||
| } | ||
|
|
||
| @ConstantFunction.List(list = { | ||
| @ConstantFunction(name = "now", argTypes = {}, returnType = DATETIME), | ||
| @ConstantFunction(name = "current_timestamp", argTypes = {}, returnType = DATETIME), |
There was a problem hiding this comment.
The most risky bug in this code is:
Integer division when calculating startTime could result in truncation.
You can modify the code like this:
LocalDateTime startTime = Instant.ofEpochMilli((connectContext.getStartTime() / 1000L) * 1000)
.atZone(TimeUtils.getTimeZone().toZoneId()).toLocalDateTime();This change ensures that integer division does not inadvertently truncate the milliseconds from epoch time, preserving the correctness of the calculation.
| } | ||
| res = ConstantOperator.createDatetime(dateTime, desc); | ||
| } else if (desc.isDecimalV2()) { | ||
| res = ConstantOperator.createDecimal(BigDecimal.valueOf(Double.parseDouble(childString)), Type.DECIMALV2); | ||
| } else if (desc.isDecimalV3()) { |
There was a problem hiding this comment.
The most risky bug in this code is:
Casting a Double to an int using ((Double) value).intValue() may cause a loss of precision if the double value represents fractional seconds, leading to incorrect time conversion. This can potentially lead to unexpected behavior when a fractional time value is provided.
You can modify the code like this:
} else if (type.isTime()) {
if (value instanceof Double) {
// Safely cast the double value to int by rounding to avoid precision loss
return String.valueOf((int) Math.round((Double) value));
}
}
mxdzs0612
force-pushed
the
cur_time
branch
4 times, most recently
from
0c79e08 to
502625d
Compare
Signed-off-by: Jiao Mingye <[email protected]>
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 25 / 25 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
|
@Mergifyio backport branch-3.3 |
|
@Mergifyio backport branch-3.4 |
✅ Backports have been created
|
✅ Backports have been created
|
Signed-off-by: Jiao Mingye <[email protected]> (cherry picked from commit f91fcfa)
Signed-off-by: Jiao Mingye <[email protected]> (cherry picked from commit f91fcfa)
#56337) Co-authored-by: Jiao Mingye <[email protected]>
Why I'm doing:
Support cast current_time in fe.
What I'm doing:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: