xiedeyantu commented on code in PR #4651:
URL: https://github.com/apache/calcite/pull/4651#discussion_r2573015191
##########
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:
Although this is very old code, I still don't understand why the inner layer
processing requires an additional check on the outer layer's type—including the
case where `inner == TimeUnit.QUARTER`.
##########
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:
```
public static int unixTimestampExtract(TimeUnitRange range,
long timestamp) {
return unixTimeExtract(range,
(int) Math.floorMod(timestamp, MILLIS_PER_DAY));
}
```
I noticed a piece of code that requires the input to be in days. It seems
that modifying it in this way may not be appropriate. I understand that this is
not intended for testing purposes, but rather serves constant folding.
##########
core/src/test/java/org/apache/calcite/test/RexImplicationCheckerTest.java:
##########
@@ -483,13 +483,18 @@ public class RexImplicationCheckerTest {
hasToString("TRIM('BOTH', 'a', '1111':VARCHAR)"));
}
- /** Test case for simplifier of ceil/floor. */
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-2178";>[CALCITE-2178]
+ * Extend expression simplifier to work on datetime CEIL/FLOOR functions</a>,
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7304";>[CALCITE-7304]
+ * Floor/Ceil can not simplify with WEEK TimeUnit</a>. */
Review Comment:
As I mentioned earlier regarding a potential issue, I think we may need an
actual execution result to verify its correctness.
--
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]
