Ring-k commented on a change in pull request #1230:
URL: https://github.com/apache/incubator-iotdb/pull/1230#discussion_r429964808



##########
File path: 
cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1397,6 +1401,91 @@ public TSStatus executeNonQuery(PhysicalPlan plan) {
     }
   }
 
+
+  public TSStatus validatePlan(PhysicalPlan plan) {
+    TSStatus result = StatusUtils.OK;
+    if (plan instanceof SetStorageGroupPlan) {
+      return validateSetStorageGroupPlan((SetStorageGroupPlan) plan);
+    } else if (plan instanceof InsertPlan) {
+      return validateInsertPlan((InsertPlan) plan);
+    }
+    // TODO add more pre-validations
+    return result;
+  }
+
+  private TSStatus validateSetStorageGroupPlan(SetStorageGroupPlan plan) {
+    String path = plan.getPath().getFullPath();
+    if (getAllStorageGroupNames().contains(path)) {
+      return StatusUtils.PATH_ALREADY_EXIST_ERROR;
+    }

Review comment:
       That's right. Thanks for your reminding.

##########
File path: 
cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1397,6 +1401,91 @@ public TSStatus executeNonQuery(PhysicalPlan plan) {
     }
   }
 
+
+  public TSStatus validatePlan(PhysicalPlan plan) {
+    TSStatus result = StatusUtils.OK;
+    if (plan instanceof SetStorageGroupPlan) {
+      return validateSetStorageGroupPlan((SetStorageGroupPlan) plan);
+    } else if (plan instanceof InsertPlan) {
+      return validateInsertPlan((InsertPlan) plan);
+    }
+    // TODO add more pre-validations
+    return result;
+  }
+
+  private TSStatus validateSetStorageGroupPlan(SetStorageGroupPlan plan) {
+    String path = plan.getPath().getFullPath();
+    if (getAllStorageGroupNames().contains(path)) {
+      return StatusUtils.PATH_ALREADY_EXIST_ERROR;
+    }
+    return StatusUtils.OK;
+  }
+
+  private TSStatus validateInsertPlan(InsertPlan plan) {
+    List<Path> paths = (plan).getPaths();
+    String[] values = (plan).getValues();
+    List<String> pathStrings = new ArrayList<>();
+    for (Path path : paths) {
+      pathStrings.add(path.getFullPath());
+    }
+    List<MeasurementSchema> measurementSchemas;
+    try {
+      measurementSchemas = pullTimeSeriesSchemas(pathStrings);

Review comment:
       The schema cache can help to accelerate the validations. Thanks for your 
suggestions.

##########
File path: 
cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1397,6 +1401,91 @@ public TSStatus executeNonQuery(PhysicalPlan plan) {
     }
   }
 
+
+  public TSStatus validatePlan(PhysicalPlan plan) {
+    TSStatus result = StatusUtils.OK;
+    if (plan instanceof SetStorageGroupPlan) {
+      return validateSetStorageGroupPlan((SetStorageGroupPlan) plan);
+    } else if (plan instanceof InsertPlan) {
+      return validateInsertPlan((InsertPlan) plan);
+    }
+    // TODO add more pre-validations
+    return result;
+  }
+
+  private TSStatus validateSetStorageGroupPlan(SetStorageGroupPlan plan) {
+    String path = plan.getPath().getFullPath();
+    if (getAllStorageGroupNames().contains(path)) {
+      return StatusUtils.PATH_ALREADY_EXIST_ERROR;
+    }
+    return StatusUtils.OK;
+  }
+
+  private TSStatus validateInsertPlan(InsertPlan plan) {
+    List<Path> paths = (plan).getPaths();
+    String[] values = (plan).getValues();
+    List<String> pathStrings = new ArrayList<>();
+    for (Path path : paths) {
+      pathStrings.add(path.getFullPath());
+    }
+    List<MeasurementSchema> measurementSchemas;
+    try {
+      measurementSchemas = pullTimeSeriesSchemas(pathStrings);
+    } catch (MetadataException e) {
+      return StatusUtils.NO_STORAGE_GROUP;
+    }
+    for (int i = 0; i < paths.size(); i++) {
+      String measurement = paths.get(i).getMeasurement();
+      String value = values[i];
+      TSStatus result = null;
+      for (MeasurementSchema schema : measurementSchemas) {
+        if (schema.getMeasurementId().equals(measurement)) {
+          try {
+            tryParse(value, schema.getType());
+            result = StatusUtils.OK;
+          } catch (IllegalArgumentException e) {
+            result = new TSStatus(StatusUtils.WRITE_PROCESS_ERROR.getCode());
+            result.setMessage(StatusUtils.WRITE_PROCESS_ERROR.getMessage() + 
e.getMessage());
+            return result;
+          }
+          break;
+        }
+      }
+      if (result == null) {
+        return StatusUtils.PATH_NOT_EXIST_ERROR;
+      }
+    }
+    return StatusUtils.OK;
+  }
+
+
+  private void tryParse(String value, TSDataType type) {
+    switch (type) {
+      case INT32:
+        Integer.parseInt(value);
+        break;
+      case INT64:
+        Long.parseLong(value);
+        break;
+      case BOOLEAN:
+        if (!(value.equals("true") || value.equals("TRUE")
+            || value.equals("false") || value.equals("FALSE")
+            || value.equals("0") || value.equals("1"))) {

Review comment:
       Oh, I didn't notice that. That's convenient. Thanks for your suggestion 
:)




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


Reply via email to