xuzifu666 commented on code in PR #4651:
URL: https://github.com/apache/calcite/pull/4651#discussion_r2559697288
##########
core/src/main/java/org/apache/calcite/rex/RexInterpreter.java:
##########
@@ -403,7 +404,7 @@ private static TimeUnitRange subUnit(TimeUnitRange unit) {
case QUARTER:
return TimeUnitRange.MONTH;
default:
- return TimeUnitRange.DAY;
+ return unit;
Review Comment:
The modification here is to enable the test class RexInterpreter to support
time units such as ```HOUR/MINUTE/SECOND```， otherwise here would not support
such as ```HOUR/MINUTE/SECOND``` time unit. Since this class is only for
testing, I think the modification is acceptable.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2671,6 +2673,12 @@ private static boolean canRollUp(TimeUnit outer,
TimeUnit inner) {
if (inner == TimeUnit.QUARTER) {
return outer == TimeUnit.YEAR;
}
+ if (outer == TimeUnit.WEEK && inner != TimeUnit.YEAR && inner !=
TimeUnit.MONTH) {
Review Comment:
This change is because the order of WEEK is after
YEAR/MONTH/DAY/HOUR/MINUTE/SECOND in TimeUnit, so the inner and outer cases are
handled separately.
--
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]
