yangyichao-mango commented on a change in pull request #3184:
URL: 
https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453200205



##########
File path: 
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
         order by id asc
     </select>
 
+    <select id="queryTopNProcessInstance" 
resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+        select *
+        from t_ds_process_instance
+        where state = 7
+        and start_time between
+        #{startTime} and #{endTime}
+        order by end_time-start_time desc
+        limit #{size}
+    </select>

Review comment:
       > I still have a question about the first previous question: since Java 
doesn't have default params,when I want to set SUCCESS as the default status,I 
might overload the method like this:
   > ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, 
@Param("startTime") Date startTime, @Param("endTime") Date endTime);`
   > 
   > ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, 
@Param("startTime") Date startTime, @Param("endTime") Date endTime, 
@Param("status")ExecutionStatus status);`
   > but since there is no method body here,how do I set SUCCESS as the default 
status?
   > I am not so familiar with Java,I hope you can give me some advice
   
   Hi,
   Thx a lot for your answer and your contribution.
   
   Can you tell me where you used `queryTopNProcessInstance` method except test 
part? This method was not used elsewhere according to your Pull Request.
   
   According to your issue #3191 , if I understand it correctly, the purpose of 
this issue is to show the top-N tasks in the web interface. But at present, 
this PR does not seem to implement the issue completely. Are you still in the 
process of completing it?
   
   If you have any questions and suggestions, please put forward.
   
   ----------------------
   你可以告诉我在除了测试部分的其他哪些地方用到了 `queryTopNProcessInstance` 吗?根据你的 pr 
来看,这个方法似乎没有被其他地方引用到。
   
   根据你的 issue 来看,如果我理解正确的话,目的是想在web界面上展示运行时长topn的任务。但是目前这个 pr 似乎没有完全完成那个 
issue,你是还在完成的过程中吗?
   
   如果有其他问题或者疑问,敬请提出。




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to