[GitHub] [iotdb] mychaow commented on a diff in pull request #5995: [IOTDB-3268] Refactor measurement check logic in session api for better performance

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

mychaow commented on code in PR #5995:
URL: https://github.com/apache/iotdb/pull/5995#discussion_r879973532


##########
server/src/main/java/org/apache/iotdb/db/service/thrift/impl/TSServiceImpl.java:
##########
@@ -2131,28 +2138,55 @@ private void addOperationLatency(Operation operation, long startTime) {
   }
 
   // check whether measurement is legal according to syntax convention
-  protected void isLegalMeasurementLists(List<List<String>> measurementLists) throws TException {
+  protected void isLegalMeasurementLists(List<List<String>> measurementLists)
+      throws MetadataException {
     if (measurementLists == null) {
       return;
     }
+    StringBuilder path = new StringBuilder(IoTDBConstant.PATH_ROOT);
     for (List<String> measurementList : measurementLists) {
-      isLegalMeasurements(measurementList);
+      for (String measurement : measurementList) {

Review Comment:
   same as above, the logic seems duplicated



##########
server/src/main/java/org/apache/iotdb/db/service/thrift/impl/TSServiceImpl.java:
##########
@@ -2131,28 +2138,55 @@ private void addOperationLatency(Operation operation, long startTime) {
   }
 
   // check whether measurement is legal according to syntax convention
-  protected void isLegalMeasurementLists(List<List<String>> measurementLists) throws TException {
+  protected void isLegalMeasurementLists(List<List<String>> measurementLists)
+      throws MetadataException {
     if (measurementLists == null) {
       return;
     }
+    StringBuilder path = new StringBuilder(IoTDBConstant.PATH_ROOT);
     for (List<String> measurementList : measurementLists) {
-      isLegalMeasurements(measurementList);
+      for (String measurement : measurementList) {
+        if (measurement != null) {
+          if (measurement.contains(TsFileConstant.PATH_SEPARATOR)
+              && !(measurement.startsWith(TsFileConstant.BACK_QUOTE_STRING)
+                  && measurement.endsWith(TsFileConstant.BACK_QUOTE_STRING))) {
+            throw new IllegalPathException(measurement);
+          } else {
+            path.append(".");
+            path.append(measurement);
+          }
+        }
+      }
+    }
+    try {
+      PathUtils.isLegalPath(path.toString());
+    } catch (IllegalPathException e) {
+      throw new MetadataException("find wrong node name according to syntax convention");
     }
   }
 
   // check whether measurement is legal according to syntax convention
-  protected void isLegalMeasurements(List<String> measurements) throws TException {
+  protected void isLegalMeasurements(List<String> measurements) throws MetadataException {
     if (measurements == null) {
       return;
     }

Review Comment:
   seems same



##########
server/src/main/java/org/apache/iotdb/db/service/thrift/impl/DataNodeTSIServiceImpl.java:
##########
@@ -1196,28 +1222,55 @@ private QueryId genQueryId(long id) {
   }
 
   // check whether measurement is legal according to syntax convention
-  protected void isLegalMeasurementLists(List<List<String>> measurementLists) throws TException {
+  protected void isLegalMeasurementLists(List<List<String>> measurementLists)
+      throws MetadataException {
     if (measurementLists == null) {
       return;
     }
+    StringBuilder path = new StringBuilder(IoTDBConstant.PATH_ROOT);
     for (List<String> measurementList : measurementLists) {
-      isLegalMeasurements(measurementList);
+      for (String measurement : measurementList) {

Review Comment:
   why not just call isLegalMeasurement?



-- 
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: reviews-unsubscribe@iotdb.apache.org

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