github-actions[bot] commented on code in PR #65319:
URL: https://github.com/apache/doris/pull/65319#discussion_r3576849383
##########
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:
Directly constructing `DateTimeLiteral` here drops the BE-compatible carry
from the 7th fractional digit for DATETIME(v1) casts. The old path parsed
through `DateTimeV2Literal(MAX)` before building the v1 literal, so
`cast('2024-12-31 23:59:59.9999999' as datetime)` folded to `2025-01-01
00:00:00`, matching BE's `CastToDateOrDatetime::from_string_strict_mode` carry
when the fractional length is greater than 6 and digit 7 is at least 5.
`DateTimeLiteral.init` records that digit but only rounds for
`DateTimeV2Literal`/`TimestampTzLiteral`, so this new path keeps `2024-12-31
23:59:59`. Please keep the DateTimeV2 validation/rounding path for DATETIME(v1)
before constructing the v1 literal, and add a fold-consistency case for the
fractional carry.
##########
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 removes the only check that kept `DateLiteral(String)` strict to
date-only or midnight inputs. After `parseDateTime(s)`, `init` now keeps only
year/month/day, so `new DateLiteral("2022-01-01 01:00:00.000000")` and `new
DateLiteral("2022-01-01 00:00:00.000001")` stop throwing even though
`DateLiteralTest.reject` still expects those constructor calls to fail.
`DateV2Literal(String)` inherits the same path, and
`LogicalPlanBuilder.visitTypeConstructor` uses these constructors directly for
`DATE`/`DATEV1`/`DATEV2` typed literals before falling back to a cast. Please
keep the direct date-literal constructors strict and put the string-to-DATE
cast truncation behind a separate helper that validates the full datetime
before dropping the time fields, or update the constructor/type-constructor
contract and tests if this broader literal acceptance is intentional.
##########
regression-test/suites/cast_p0/cast_to_datetime.groovy:
##########
@@ -237,6 +237,11 @@ qt_sql204 """ select cast('9999-12-31 23:59:59.999999
+00:00' as datetime(6)) ""
testFoldConst("select cast('9999-12-31 23:59:59.999999 +00:00' as
datetime(6))")
qt_sql205 """ select cast('0000-01-01 00:00:00+12:00' as datetime(6)); """
testFoldConst("select cast('0000-01-01 00:00:00+12:00' as datetime(6));")
+testFoldConst("select cast('9999-12-31 23:59:59.999999' as date), "
+ + "cast('9999-12-31 23:59:59.999999' as datev2)")
+waitUntilSafeExecutionTime("NOT_CROSS_DAY_BOUNDARY", 2)
Review Comment:
This guard does not actually make the following negative TIME
`testFoldConst` deterministic near midnight. `testFoldConst` runs the folded
query and then a separate `debug_skip_fold_constant=true` query, and this
expression's date comes from `LocalDateTime.now(DateUtils.getTimeZone())` on
the FE-folded side versus the runtime timestamp on the non-folded side. The
helper computes `ChronoUnit.SECONDS.between(now, startOfNextDay)` and sleeps
only that truncated value, so in the last fractional second before midnight it
sleeps 0, and between 1 and 2 seconds before midnight it can wake still before
midnight. Please make the guard sleep past the boundary with a buffer, or make
this regression use a deterministic base timestamp before comparing folded and
non-folded results.
--
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]
