github-actions[bot] commented on code in PR #65319:
URL: https://github.com/apache/doris/pull/65319#discussion_r3576888149
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateLiteral.java:
##########
@@ -370,7 +370,7 @@ protected void init(String s) throws AnalysisException {
month = DateUtils.getOrDefault(dateTime, ChronoField.MONTH_OF_YEAR);
day = DateUtils.getOrDefault(dateTime, ChronoField.DAY_OF_MONTH);
- if (checkDatetime(dateTime) || checkRange(year, month, day) ||
checkDate(year, month, day)) {
+ if (checkRange(year, month, day) || checkDate(year, month, day)) {
Review Comment:
This relaxes the shared DATE/DATEV2 literal constructor, not just the
string-to-DATE cast path. `DateV2Literal(String)` inherits this initializer,
and `LogicalPlanBuilder.visitTypeConstructor` uses these constructors for
`DATE`, `DATEV2`, and `DATEV1` before falling back to a `Cast`. After this
change, a type constructor such as `DATE '2022-01-01 01:00:00'` or `DATEV2
'2022-01-01 00:00:00.000001'` is accepted as a date literal with the time
silently discarded. That is inconsistent with the still-strict
`parseDateLiteral(...)` path used by literal coercion, and it also widens other
direct constructor users such as stats min/max conversion in
`ExpressionEstimation.castMinMax`. Please keep `DateLiteral(String)` strict and
make the string cast path validate a datetime literal first, then explicitly
construct the DATE/DATEV2 from year/month/day.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/StringLikeLiteral.java:
##########
@@ -385,10 +361,30 @@ protected DateTimeV2Literal getDateTimeLiteral(String
year, String month, String
}
}
String format = String.format("%s-%s-%sT%s:%s:%s%s%s", year4, month,
date, hour, minute, second, fraction, tz);
- try {
- return new DateTimeV2Literal((DateTimeV2Type) targetType, format);
- } catch (AnalysisException e) {
- throw new CastException(e.getMessage(), e);
+ if (targetType.isDateType()) {
+ try {
+ return new DateLiteral(format);
+ } catch (AnalysisException e) {
+ throw new CastException(e.getMessage(), e);
+ }
+ } else if (targetType.isDateV2Type()) {
+ try {
+ return new DateV2Literal(format);
+ } catch (AnalysisException e) {
+ throw new CastException(e.getMessage(), e);
+ }
+ } else if (targetType.isDateTimeType()) {
+ try {
+ return new DateTimeLiteral(format);
Review Comment:
This DATETIME(v1) branch now bypasses the DateTimeV2 parsing path that used
to apply the 7th fractional digit carry before dropping microseconds.
`DateTimeLiteral.init` reads the 7th digit, but it only calls
`plusMicroSeconds(1)` for `DateTimeV2Literal` and `TimestampTzLiteral`, so
`cast('2024-01-01 00:00:00.9999999' as datetime)` folds to `2024-01-01
00:00:00` in FE. BE string-to-DATETIME parsing still treats DATETIME(v1) as a
datetime target and carries `.9999999` to the next second before storing the v1
value. Please keep the old DateTimeV2-parse-then-v1-convert behavior here, or
otherwise apply the same carry before constructing the `DateTimeLiteral`, and
add a fold-consistency case for plain `DATETIME`.
--
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]
