You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/06/07 06:49:07 UTC

[GitHub] [iotdb] LebronAl commented on a change in pull request #3275: cluster bug - fix privilege check

LebronAl commented on a change in pull request #3275:
URL: https://github.com/apache/iotdb/pull/3275#discussion_r646312746



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/coordinator/Coordinator.java
##########
@@ -334,7 +345,8 @@ private TSStatus forwardPlan(List<PartitionGroup> partitionGroups, PhysicalPlan
   private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, PhysicalPlan plan) {
     // the error codes from the groups that cannot execute the plan
     TSStatus status;
-    if (planGroupMap.size() == 1) {
+    // need to create substatus for multiPlan
+    if (planGroupMap.size() == 1 && !(plan instanceof CreateMultiTimeSeriesPlan)) {

Review comment:
       I think these specializations make the code difficult to maintain, so why not do permission checking in `TSServiceImpl`?

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -590,7 +590,17 @@ public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsi
               Collections.singletonList(
                   new PartialPath(((SetDeviceTemplatePlan) plan).getPrefixPath()))));
     } else {
-      storageGroups.addAll(getStorageGroups(plan.getPaths()));
+      List<PartialPath> paths = new ArrayList<>();

Review comment:
       why not judge in this way?
   ```
   } else if (plan instanceof CreateMultiTimeSeriesPlan) {
      xxx
   } else {
   storageGroups.addAll(getStorageGroups(paths));
   }
   ```




-- 
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:
users@infra.apache.org