github-actions[bot] commented on code in PR #67500:
URL: https://github.com/apache/doris/pull/67500#discussion_r3944302427
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/TimeV2Literal.java:
##########
@@ -263,7 +263,7 @@ protected void init(String s) throws AnalysisException {
protected static boolean checkRange(double hour, int minute, int second,
int microsecond) {
return hour > 838 || minute > 59 || second > 59 || microsecond >
999999 || minute < 0 || second < 0
- || microsecond < 0;
+ || microsecond < 0 || (hour == 838 && minute == 59 && second
== 59 && microsecond != 0);
Review Comment:
[P1] Keep endpoint parsing digit-exact
For `TIME(6)`, `838:59:59.000000499999999999999999` has a seventh fractional
digit of `4`, so the BE string parser correctly rounds down to the exact valid
endpoint. FE's `parseTimeLiteral`/`init` first convert the seconds to `double`;
this value becomes `59.0000005`, `Math.round(secPart * 1e6)` yields `59000001`,
and this new predicate rejects the resulting one microsecond. Because
`TimeChecker` accepts arbitrary-length fractions and character-literal coercion
invokes this FE parser, `MAKETIME(838, 59, 59) =
'838:59:59.000000499999999999999999'` leaves the right side as VARCHAR and then
selects `DATETIMEV2(6)` instead of comparing TIME values, even though the BE
TIME cast accepts the endpoint. The negative endpoint is symmetric. Please
parse and round the fractional digits exactly like BE (or with decimal
arithmetic) before enforcing this bound, and cover both signs through
string-to-TIME coercion.
--
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]
