-
Couldn't load subscription status.
- Fork 3.6k
fix converting date in yyyyMM and yyyy formats to Unix timestamp report Invalid value for DayOfMonth (valid values 1–28/31): 0. #57234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
run buildall |
ClickBench: Total hot run time: 29.22 s |
FE UT Coverage ReportIncrement line coverage |
| if (accessor.isSupported(ChronoField.DAY_OF_MONTH)) { | ||
| return accessor.get(ChronoField.DAY_OF_MONTH); | ||
| } else if (accessor.isSupported(ChronoField.DAY_OF_YEAR)) { | ||
| return accessor.get(ChronoField.DAY_OF_YEAR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
day of year is 1..366. not suitable here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you attach an error backstrace here? By this issue I'm not very sure whether this fix is proper
| @@ -370,8 +370,8 @@ public static LocalDateTime getTime(DateTimeFormatter formatter, String value) { | |||
| TemporalAccessor accessor = formatter.parse(value); | |||
| return LocalDateTime.of( | |||
| getOrDefault(accessor, ChronoField.YEAR), | |||
| getOrDefault(accessor, ChronoField.MONTH_OF_YEAR), | |||
| getOrDefault(accessor, ChronoField.DAY_OF_MONTH), | |||
| getMonthOrDefault(accessor), | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By changing this, you actually changed the function str_to_date's behaviour of FE. you should add some regression cases to make sure the behaviour is same of BE and FE. could use qt_sql and testFoldConst in reg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
run buildall |
ClickBench: Total hot run time: 28.02 s |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-DS: Total hot run time: 190769 ms |
ClickBench: Total hot run time: 27.74 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
ClickBench: Total hot run time: 28.21 s |
FE Regression Coverage ReportIncrement line coverage |
|
Hi @zclllyybb @yiguolei If you have time, could you please take a look at this PR? The failing tests now pass on my local machine. |
Hi @KeeProMise , thanks for your contribution! In this PR you actually modified the logic of function
|
|
@zclllyybb stacktrace Caused by: org.apache.doris.common.AnalysisException: errCode = 2, detailMessage = Invalid value for DayOfMonth (valid values 1 - 28/31): 0
... 18 more
Caused by: java.time.DateTimeException: Invalid value for DayOfMonth (valid values 1 - 28/31): 0
at java.time.temporal.ValueRange.checkValidValue(ValueRange.java:319) ~[?:?]
at java.time.temporal.ChronoField.checkValidValue(ChronoField.java:718) ~[?:?]
at java.time.LocalDate.of(LocalDate.java:272) ~[?:?]
at java.time.LocalDateTime.of(LocalDateTime.java:363) ~[?:?]
at org.apache.doris.nereids.util.DateUtils.getTime(DateUtils.java:166) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.nereids.trees.expressions.functions.executable.DateTimeExtractAndTransform.strToDate(DateTimeExtractAndTransform.java:695) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.mtmv.MTMVUtil.getExprTimeSec(MTMVUtil.java:129) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.catalog.ListPartitionItem.isGreaterThanSpecifiedTime(ListPartitionItem.java:130) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.mtmv.MTMVRelatedPartitionDescSyncLimitGenerator.apply(MTMVRelatedPartitionDescSyncLimitGenerator.java:62) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.mtmv.MTMVPartitionUtil.generateRelatedPartitionDescs(MTMVPartitionUtil.java:184) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.mtmv.MTMVPartitionUtil.getPartitionDescsByRelatedTable(MTMVPartitionUtil.java:166) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.nereids.trees.plans.commands.info.CreateMTMVInfo.generatePartitionDesc(CreateMTMVInfo.java:333) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.nereids.trees.plans.commands.info.CreateMTMVInfo.analyzeQuery(CreateMTMVInfo.java:273) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.nereids.trees.plans.commands.info.CreateMTMVInfo.analyze(CreateMTMVInfo.java:163) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.nereids.trees.plans.commands.CreateMTMVCommand.run(CreateMTMVCommand.java:51) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.StmtExecutor.executeByNereids(StmtExecutor.java:753) ~[doris-fe.jar:1.2-SNAPSHOT]
... 17 more |
What problem does this PR solve?
Issue Number: #57233
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)