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: [email protected]

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


Reply via email to