[GitHub] [dolphinscheduler] mgduoduo commented on a change in pull request #7491: [Feature-#6422] [api-server] task group queue

[GitBox <gi...@apache.org>]

mgduoduo commented on a change in pull request #7491:
URL: https://github.com/apache/dolphinscheduler/pull/7491#discussion_r773542908



##########
File path: dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_postgre.sql
##########
@@ -1015,6 +1017,7 @@ CREATE TABLE t_ds_task_group (
    name        varchar(100) DEFAULT NULL ,
    description varchar(200) DEFAULT NULL ,
    group_size  int NOT NULL ,
+   project_code bigint DEFAULT '0' ,

Review comment:
       We can prepare a sql to initialize the value of `project_code` based on `project_id`  from table `t_ds_project` , e.g.:
   `update t_ds_task_group a, t_ds_project b set a.project_code =b.code where a.project_id=b.id`

##########
File path: dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql
##########
@@ -1051,7 +1053,7 @@ CREATE TABLE `t_ds_task_group` (
    `group_size` int (11) NOT NULL COMMENT'group size',
    `use_size` int (11) DEFAULT '0' COMMENT 'used size',
    `user_id` int(11) DEFAULT NULL COMMENT 'creator id',
-   `project_id` int(11) DEFAULT NULL COMMENT 'project id',

Review comment:
       It's risky to remove an old field and replace it with a new one without compatibility considerations. It's better to keep them all, or to initialize value for `project_code` while deleting the `project_id`.

##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupQueueMapper.xml
##########
@@ -117,4 +123,32 @@
         where task_id = #{taskId}
     </select>
 
+    <select id="queryTaskGroupQueueByTaskGroupIdPaging" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroupQueue">
+        select
+            queue.id, queue.task_name, queue.group_id, queue.process_id, queue.priority, queue.status
+             , queue.force_start, queue.create_time, queue.update_time,
+               process.name as processInstanceName,p.name as projectName,p.code as projectCode
+        from t_ds_task_group_queue queue
+            left join t_ds_process_instance process on queue.process_id = process.id
+            left join t_ds_process_definition p_f on process.process_definition_code = p_f.code
+                                                         and process.process_definition_version = p_f.version
+            join t_ds_project as p on p_f.project_code = p.code

Review comment:
       It should use  `left join t_ds_project `  as the `p_f` is a `left join` table, otherwise, some data will be lost from the query results.

##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml
##########
@@ -52,6 +53,14 @@
         </where>
     </select>
 
+    <select id="queryTaskGroupPagingByProjectCode" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroup">
+        select
+        <include refid="baseSql">
+        </include>
+        from t_ds_task_group
+        where project_code = #{projectCode} or project_code = 0

Review comment:
       +1

##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskGroupController.java
##########
@@ -287,9 +316,13 @@ public Result wakeCompulsively(@ApiIgnore @RequestAttribute(value = Constants.SE
     @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
     public Result queryTasksByGroupId(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
                                       @RequestParam("groupId") Integer groupId,
+                                      @RequestParam(value = "taskInstanceName",required = false) String taskName,
+                                      @RequestParam(value = "processInstanceName",required = false) String processName,
+                                      @RequestParam(value = "status",required = false) Integer status,
                                       @RequestParam("pageNo") Integer pageNo,
                                       @RequestParam("pageSize") Integer pageSize) {
-        Map<String, Object> result = taskGroupQueueService.queryTasksByGroupId(loginUser, groupId, pageNo, pageSize);
+        Map<String, Object> result = taskGroupQueueService.queryTasksByGroupId(loginUser, taskName,processName,status,

Review comment:
       It seems better to assemble an object instead of these parameters

##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/TaskGroupMapper.xml
##########
@@ -52,6 +53,14 @@
         </where>
     </select>
 
+    <select id="queryTaskGroupPagingByProjectCode" resultType="org.apache.dolphinscheduler.dao.entity.TaskGroup">
+        select
+        <include refid="baseSql">
+        </include>
+        from t_ds_task_group
+        where project_code = #{projectCode} or project_code = 0

Review comment:
       A question is why including  `project_code = 0` here?  Is it because the `project_code` is a new column in table `t_ds_task_group` with default value `0` and without any other logic to initializate the value? I would suggest adding a little logic or preparing a DML SQL to initialize the data for the new column so that `or project_code = 0` can be removed.




-- 
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-unsubscribe@dolphinscheduler.apache.org

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