morningman commented on code in PR #17342:
URL: https://github.com/apache/doris/pull/17342#discussion_r1124068043


##########
fe/fe-core/src/main/java/org/apache/doris/alter/AlterHandler.java:
##########
@@ -140,7 +142,20 @@ public Long 
getAlterJobV2Num(org.apache.doris.alter.AlterJobV2.JobState state, l
     }
 
     public Long getAlterJobV2Num(org.apache.doris.alter.AlterJobV2.JobState 
state) {
-        return alterJobsV2.values().stream().filter(e -> e.getJobState() == 
state).count();
+        Long counter = 0L;
+
+        for (AlterJobV2 job : alterJobsV2.values()) {

Review Comment:
   This method is only called in `show proc` stmt, which already check the 
ADMIN priv.
   So no need to check priv here again.
   You can just leave a comment to here for other developer to notice that.



##########
fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java:
##########
@@ -1146,6 +1147,24 @@ public List<List<Comparable>> 
getAlterJobInfosByDb(Database db) {
         return rollupJobInfos;
     }
 
+    public List<List<Comparable>> getAllAlterJobInfos() {

Review Comment:
   Same as AlterHandler, no need to check priv



##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -1599,6 +1599,26 @@ private void runAlterJobV2() {
         });
     }
 
+    public List<List<Comparable>> getAllAlterJobInfos() {
+        List<List<Comparable>> schemaChangeJobInfos = new LinkedList<>();

Review Comment:
   Same as AlterHandler, no need to check priv



##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/LoadProcDir.java:
##########
@@ -57,18 +57,26 @@ public LoadProcDir(Load load, Database db) {
 
     @Override
     public ProcResult fetchResult() throws AnalysisException {
-        Preconditions.checkNotNull(db);
         Preconditions.checkNotNull(load);
 
         BaseProcResult result = new BaseProcResult();
         result.setNames(TITLE_NAMES);
 
         // merge load job from load and loadManager
-        LinkedList<List<Comparable>> loadJobInfos = 
load.getLoadJobInfosByDb(db.getId(), db.getFullName(),
+        LinkedList<List<Comparable>> loadJobInfos;
+
+        // db is null means need total result of all databases
+        if (db == null) {
+            loadJobInfos = load.getAllLoadJobInfos();

Review Comment:
   I think we can refactor this class this time, to remove `load` instance, 
only use `LoadManager`.
   Because there is no more load job saved in `load` class. It should be 
deprecated in future.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to