EmmyMiao87 commented on code in PR #8861:
URL: https://github.com/apache/doris/pull/8861#discussion_r917716883


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -122,12 +126,40 @@ public List<Table> getTables() throws AnalysisException {
         return tables;
     }
 
+    public List<String> getPartitionNames() {

Review Comment:
   The get function in semantic parsing is generally divided into two types. 
One is to directly get the parameters specified by the user, such as what you 
are implementing now. Another is to get the real value after semantic analysis, 
such as the function getTables.
   
   Generally speaking, what the external class wants to get is the second. So I 
think in your case, you don't really need the first type of function.
   If you need a function that returns a real value, you should write
   
   ```
   public void analyze() {
        if (partition name is empty or null) {
            add all partition name in partitionNames;
        }
   }
   
   public List<String> getParititionNames() {
        check(analyzed == true);
        return partitionNames;
   }
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -202,17 +236,22 @@ public void analyze(Analyzer analyzer) throws 
UserException {
                     checkAnalyzePriv(dbName, table.getName());
                 }
 
-                this.dbId = db.getId();
+                dbId = db.getId();
                 for (Table table : tables) {
                     long tblId = table.getId();
-                    this.tblIds.add(tblId);
+                    tblIds.add(tblId);
                 }
             } finally {
                 db.readUnlock();
             }
         }
 
-        // step2: analyze properties
+        // step2: check partition
+        if (partitionNames != null) {
+            partitionNames.analyze(analyzer);

Review Comment:
   If partitionname is specified but an error is specified, it should also be 
checked here.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -202,17 +236,22 @@ public void analyze(Analyzer analyzer) throws 
UserException {
                     checkAnalyzePriv(dbName, table.getName());
                 }
 
-                this.dbId = db.getId();
+                dbId = db.getId();
                 for (Table table : tables) {
                     long tblId = table.getId();
-                    this.tblIds.add(tblId);
+                    tblIds.add(tblId);
                 }
             } finally {
                 db.readUnlock();
             }
         }
 
-        // step2: analyze properties
+        // step2: check partition
+        if (partitionNames != null) {
+            partitionNames.analyze(analyzer);

Review Comment:
   Can the temporary partition also collect statistics?
   If not, is it necessary to prohibit users from specifying temporary 
partitions during semantic parsing?



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -60,47 +60,50 @@
  *      properties: properties of statistics jobs
  */
 public class AnalyzeStmt extends DdlStmt {
-    private static final Logger LOG = LogManager.getLogger(AnalyzeStmt.class);
-
-    // time to wait for collect  statistics
+    /** time to wait for collect  statistics */
     public static final String CBO_STATISTICS_TASK_TIMEOUT_SEC = 
"cbo_statistics_task_timeout_sec";
 
     private static final ImmutableSet<String> PROPERTIES_SET = new 
ImmutableSet.Builder<String>()
             .add(CBO_STATISTICS_TASK_TIMEOUT_SEC)
             .build();
 
-    public static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 
0L;
+    private static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 
0L;
 
     private final TableName dbTableName;
+    private final PartitionNames partitionNames;
     private final List<String> columnNames;
     private final Map<String, String> properties;
 
     // after analyzed
     private long dbId;
     private final Set<Long> tblIds = Sets.newHashSet();
 
-    public AnalyzeStmt(TableName dbTableName, List<String> columns, 
Map<String, String> properties) {
+    public AnalyzeStmt(TableName dbTableName,
+            List<String> columns,
+            PartitionNames partitionNames,
+            Map<String, String> properties) {

Review Comment:
   At present, our data structure design and api design cannot be satisfied. 
Analyze specifies the syntax of multiple tables.
   For this function, the current pr can be recorded as a TODO, and the 
subsequent pr will add related functions.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java:
##########
@@ -60,47 +60,50 @@
  *      properties: properties of statistics jobs
  */
 public class AnalyzeStmt extends DdlStmt {
-    private static final Logger LOG = LogManager.getLogger(AnalyzeStmt.class);
-
-    // time to wait for collect  statistics
+    /** time to wait for collect  statistics */
     public static final String CBO_STATISTICS_TASK_TIMEOUT_SEC = 
"cbo_statistics_task_timeout_sec";
 
     private static final ImmutableSet<String> PROPERTIES_SET = new 
ImmutableSet.Builder<String>()
             .add(CBO_STATISTICS_TASK_TIMEOUT_SEC)
             .build();
 
-    public static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 
0L;
+    private static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 
0L;
 
     private final TableName dbTableName;
+    private final PartitionNames partitionNames;
     private final List<String> columnNames;
     private final Map<String, String> properties;
 
     // after analyzed
     private long dbId;
     private final Set<Long> tblIds = Sets.newHashSet();
 
-    public AnalyzeStmt(TableName dbTableName, List<String> columns, 
Map<String, String> properties) {
+    public AnalyzeStmt(TableName dbTableName,
+            List<String> columns,
+            PartitionNames partitionNames,

Review Comment:
   My question, I forgot you didn't implement the grammar



-- 
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