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 2020/05/27 03:53:47 UTC
[GitHub] [incubator-iotdb] Ring-k commented on a change in pull request #1230: [IOTDB-665] validate physical plan before fowarding
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:
users@infra.apache.org