morrySnow commented on code in PR #31632:
URL: https://github.com/apache/doris/pull/31632#discussion_r1512296667


##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVUtil.java:
##########
@@ -66,4 +83,115 @@ public static boolean mtmvContainsExternalTable(MTMV mtmv) {
         }
         return false;
     }
+
+    /**
+     * Obtain the minimum second from `syncLimit` `timeUnit` ago
+     *
+     * @param timeUnit
+     * @param syncLimit
+     * @return
+     * @throws AnalysisException
+     */
+    public static long getNowTruncSubSec(MTMVPartitionSyncTimeUnit timeUnit, 
int syncLimit)
+            throws AnalysisException {
+        if (syncLimit < 1) {
+            throw new AnalysisException("Unexpected syncLimit, syncLimit: " + 
syncLimit);
+        }
+        // get current time
+        Expression now = DateTimeAcquire.now();
+        if (!(now instanceof DateTimeLiteral)) {
+            throw new AnalysisException("Obtaining current time does not meet 
expectations, now: " + now);

Review Comment:
   error msg should tell user what do u expect



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -240,10 +242,14 @@ public Optional<String> getWorkloadGroup() {
         }
     }
 
+    public MTMVPartitionSyncConfig getPartitionSyncConfig() {
+        return 
MTMVUtil.generateMTMVPartitionSyncConfigByProperties(mvProperties);
+    }

Review Comment:
   if we use this function multi times, do we need merialize the result of this 
function for speed up?
   if we only use this function one time, why wrap it?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/ListPartitionItem.java:
##########
@@ -99,6 +101,25 @@ public PartitionKeyDesc toPartitionKeyDesc(int pos) throws 
AnalysisException {
         return PartitionKeyDesc.createIn(Lists.newArrayList(res));
     }
 
+    @Override
+    public boolean isSatisfyConfig(int pos, Optional<String> 
dateFormatOptional, long nowTruncSubSec)

Review Comment:
   function name is not very good. satisfy config is very common, cannot know 
satisfy what config



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionSyncTimeUnit.java:
##########
@@ -0,0 +1,37 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.mtmv;
+
+import java.util.Optional;
+
+public enum MTMVPartitionSyncTimeUnit {
+    YEAR,
+    MONTH,
+    DAY;
+

Review Comment:
   mulit blank line



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVUtil.java:
##########
@@ -17,6 +17,10 @@
 
 package org.apache.doris.mtmv;
 
+import org.apache.doris.analysis.DateLiteral;

Review Comment:
   if u mixed-use same name class in analysis and Nereids, u should use simple 
name from one package to avoid typo. for this file, u should always use simple 
name for Nereids' expression, and use full-qualifier name for legacy planner's 
expression.



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -217,7 +219,7 @@ public Map<String, String> alterMvProperties(Map<String, 
String> mvProperties) {
     public long getGracePeriod() {
         readMvLock();
         try {
-            if 
(mvProperties.containsKey(PropertyAnalyzer.PROPERTIES_GRACE_PERIOD)) {
+            if 
(!StringUtils.isEmpty(mvProperties.get(PropertyAnalyzer.PROPERTIES_GRACE_PERIOD)))
 {

Review Comment:
   because we allow empty string? should we add a check for properties?



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVUtil.java:
##########
@@ -66,4 +83,115 @@ public static boolean mtmvContainsExternalTable(MTMV mtmv) {
         }
         return false;
     }
+
+    /**
+     * Obtain the minimum second from `syncLimit` `timeUnit` ago
+     *
+     * @param timeUnit
+     * @param syncLimit
+     * @return
+     * @throws AnalysisException
+     */
+    public static long getNowTruncSubSec(MTMVPartitionSyncTimeUnit timeUnit, 
int syncLimit)
+            throws AnalysisException {
+        if (syncLimit < 1) {
+            throw new AnalysisException("Unexpected syncLimit, syncLimit: " + 
syncLimit);
+        }
+        // get current time
+        Expression now = DateTimeAcquire.now();
+        if (!(now instanceof DateTimeLiteral)) {
+            throw new AnalysisException("Obtaining current time does not meet 
expectations, now: " + now);
+        }
+        DateTimeLiteral nowLiteral = (DateTimeLiteral) now;
+        // date trunc
+        now = DateTimeExtractAndTransform
+                .dateTrunc(nowLiteral, new VarcharLiteral(timeUnit.name()));
+        if (!(now instanceof DateTimeLiteral)) {
+            throw new AnalysisException("date trunc not meet expectations, 
nowTrunc: " + now);
+        }
+        nowLiteral = (DateTimeLiteral) now;
+        // date sub
+        if (syncLimit > 1) {
+            nowLiteral = dateSub(nowLiteral, timeUnit, syncLimit - 1);
+        }
+        return ((IntegerLiteral) 
DateTimeExtractAndTransform.unixTimestamp(nowLiteral)).getValue();
+    }
+
+    private static DateTimeLiteral dateSub(
+            org.apache.doris.nereids.trees.expressions.literal.DateLiteral 
date, MTMVPartitionSyncTimeUnit timeUnit,
+            int num)
+            throws AnalysisException {
+        IntegerLiteral integerLiteral = new IntegerLiteral(num);
+        Expression result;
+        switch (timeUnit) {
+            case DAY:
+                result = DateTimeArithmetic.dateSub(date, integerLiteral);
+                break;
+            case YEAR:
+                result = DateTimeArithmetic.yearsSub(date, integerLiteral);
+                break;
+            case MONTH:
+                result = DateTimeArithmetic.monthsSub(date, integerLiteral);
+                break;
+            default:
+                throw new AnalysisException("not support timeUnit: " + 
timeUnit.name());
+        }
+        if (!(result instanceof DateTimeLiteral)) {
+            throw new AnalysisException("date sub not meet expectations, 
result: " + result);
+        }
+        return (DateTimeLiteral) result;
+    }
+
+    /**
+     * Convert LiteralExpr to second
+     *
+     * @param expr
+     * @param dateFormatOptional
+     * @return
+     * @throws AnalysisException
+     */
+    public static long getExprTimeSec(LiteralExpr expr, Optional<String> 
dateFormatOptional) throws AnalysisException {
+        if (expr instanceof MaxLiteral) {
+            return Long.MAX_VALUE;
+        }
+        if (expr instanceof NullLiteral) {
+            return Long.MIN_VALUE;
+        }
+        if (expr instanceof DateLiteral) {
+            return ((DateLiteral) expr).unixTimestamp(TimeUtils.getTimeZone()) 
/ 1000;
+        }
+        if (!dateFormatOptional.isPresent()) {
+            throw new AnalysisException("expr is not DateLiteral and 
DateFormat is not present.");
+        }
+        String dateFormat = dateFormatOptional.get();
+        Expression strToDate = DateTimeExtractAndTransform
+                .strToDate(new VarcharLiteral(expr.getStringValue()), new 
VarcharLiteral(dateFormat));

Review Comment:
   use `Literal.fromLegacyLiteral` to convert legacy planner's literal to 
Nereids literal 



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVUtil.java:
##########
@@ -66,4 +83,115 @@ public static boolean mtmvContainsExternalTable(MTMV mtmv) {
         }
         return false;
     }
+
+    /**
+     * Obtain the minimum second from `syncLimit` `timeUnit` ago
+     *
+     * @param timeUnit
+     * @param syncLimit
+     * @return
+     * @throws AnalysisException
+     */
+    public static long getNowTruncSubSec(MTMVPartitionSyncTimeUnit timeUnit, 
int syncLimit)
+            throws AnalysisException {
+        if (syncLimit < 1) {
+            throw new AnalysisException("Unexpected syncLimit, syncLimit: " + 
syncLimit);
+        }
+        // get current time
+        Expression now = DateTimeAcquire.now();
+        if (!(now instanceof DateTimeLiteral)) {
+            throw new AnalysisException("Obtaining current time does not meet 
expectations, now: " + now);
+        }
+        DateTimeLiteral nowLiteral = (DateTimeLiteral) now;
+        // date trunc
+        now = DateTimeExtractAndTransform
+                .dateTrunc(nowLiteral, new VarcharLiteral(timeUnit.name()));
+        if (!(now instanceof DateTimeLiteral)) {
+            throw new AnalysisException("date trunc not meet expectations, 
nowTrunc: " + now);
+        }
+        nowLiteral = (DateTimeLiteral) now;
+        // date sub
+        if (syncLimit > 1) {
+            nowLiteral = dateSub(nowLiteral, timeUnit, syncLimit - 1);
+        }
+        return ((IntegerLiteral) 
DateTimeExtractAndTransform.unixTimestamp(nowLiteral)).getValue();
+    }
+
+    private static DateTimeLiteral dateSub(
+            org.apache.doris.nereids.trees.expressions.literal.DateLiteral 
date, MTMVPartitionSyncTimeUnit timeUnit,
+            int num)
+            throws AnalysisException {
+        IntegerLiteral integerLiteral = new IntegerLiteral(num);
+        Expression result;
+        switch (timeUnit) {
+            case DAY:
+                result = DateTimeArithmetic.dateSub(date, integerLiteral);
+                break;
+            case YEAR:
+                result = DateTimeArithmetic.yearsSub(date, integerLiteral);
+                break;
+            case MONTH:
+                result = DateTimeArithmetic.monthsSub(date, integerLiteral);
+                break;
+            default:
+                throw new AnalysisException("not support timeUnit: " + 
timeUnit.name());

Review Comment:
   error msg is not clearify. u should say mtmv partition rollup not support 
.....



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to