jt2594838 commented on a change in pull request #1466:
URL: https://github.com/apache/incubator-iotdb/pull/1466#discussion_r450593733
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/ClearCachePlan.java
##########
@@ -34,4 +34,9 @@ public ClearCachePlan() {
public List<Path> getPaths() {
return new ArrayList<>();
}
+
+ @Override
+ public List<String> getPathsStrings() {
+ return new ArrayList<>();
Review comment:
Use Collections.emptyList().
##########
File path:
cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -2614,6 +2641,43 @@ private AsyncClientPool getDataAsyncClientPool() {
return ret;
}
+ /**
+ * Get all paths after removing wildcards in the path
+ *
+ * @param originalPaths, a list of paths, potentially with wildcard
+ * @return a pair of path lists, the first are the existing full paths, the
second are invalid
+ * original paths
+ */
+ public Pair<List<String>, List<String>> getMatchedPaths(List<String>
originalPaths) {
+ ConcurrentSkipListSet<String> fullPaths = new ConcurrentSkipListSet<>();
+ ConcurrentSkipListSet<String> nonExistPaths = new
ConcurrentSkipListSet<>();
+ ExecutorService getAllPathsService = Executors
+ .newFixedThreadPool(partitionTable.getGlobalGroups().size());
+ for (String pathStr : originalPaths) {
+ getAllPathsService.submit(() -> {
+ try {
+ List<String> fullPathStrs = getMatchedPaths(pathStr);
+ if (fullPathStrs.isEmpty()) {
+ nonExistPaths.add(pathStr);
+ logger.error("Path {} is not found.", pathStr);
+ }
+ fullPaths.addAll(fullPathStrs);
Review comment:
`fullPaths.addAll(fullPathStrs);` may be put into the else branch to
reduce unnecessary method calls.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/AlterTimeSeriesPlan.java
##########
@@ -102,6 +103,15 @@ public String getAlias() {
return Collections.singletonList(path);
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ for (Path curPath : Collections.singletonList(path)) {
+ ret.add(curPath.getFullPath());
+ }
+ return ret;
Review comment:
Just `Return Collections.singletonList(path.getFullPath());` and check
other places, please.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowPlan.java
##########
@@ -196,13 +196,21 @@ public void markFailedMeasurementInsertion(int index,
Exception e) {
@Override
public List<Path> getPaths() {
List<Path> ret = new ArrayList<>();
-
for (String m : measurements) {
ret.add(new Path(deviceId, m));
}
return ret;
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ for (String m : measurements) {
+ ret.add(new Path(deviceId, m).getFullPath());
Review comment:
Just use `ret.add(deviceId + IoTDBConstant.PATH_SEPARATOR + m)`.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/DataAuthPlan.java
##########
@@ -49,6 +49,11 @@ public DataAuthPlan(OperatorType operatorType, List<String>
users) {
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
`Collections.emptyList()`
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/CreateTimeSeriesPlan.java
##########
@@ -133,12 +134,21 @@ public String toString() {
return String.format("seriesPath: %s, resultDataType: %s, encoding: %s,
compression: %s", path,
dataType, encoding, compressor);
}
-
+
@Override
public List<Path> getPaths() {
return Collections.singletonList(path);
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ for (Path curPath : Collections.singletonList(path)) {
+ ret.add(curPath.getFullPath());
+ }
+ return ret;
Review comment:
`Collections.singletonList(path.getFullPath())`
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/MergePlan.java
##########
@@ -38,4 +38,9 @@ public MergePlan() {
public List<Path> getPaths() {
return new ArrayList<>();
}
+
+ @Override
+ public List<String> getPathsStrings() {
+ return new ArrayList<>();
Review comment:
And here.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/SetStorageGroupPlan.java
##########
@@ -57,6 +58,15 @@ public void setPath(Path path) {
return ret;
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ if (path != null) {
+ ret.add(path.getFullPath());
+ }
+ return ret;
Review comment:
And here.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/SetTTLPlan.java
##########
@@ -54,6 +54,11 @@ public SetTTLPlan(String storageGroup) {
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
And here.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/TracingPlan.java
##########
@@ -38,6 +38,11 @@ public TracingPlan(boolean isTracingOn) {
return new ArrayList<>();
}
+ @Override
+ public List<String> getPathsStrings() {
+ return new ArrayList<>();
Review comment:
And here.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/crud/DeletePartitionPlan.java
##########
@@ -41,6 +41,11 @@ public DeletePartitionPlan(String storageGroupName,
Set<Long> partitionId) {
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
It is not a good idea to return a null collection, use
`Collections.emptyList()` instead.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/AuthorPlan.java
##########
@@ -254,6 +254,15 @@ public String toString() {
return ret;
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ if (nodeName != null) {
+ ret.add(nodeName.getFullPath());
+ }
+ return ret;
Review comment:
`return nodeName != null ?
Collections.singletonList(nodeName.getFullPath()) : Collections.emptyList()`.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/CreateSnapshotPlan.java
##########
@@ -35,4 +35,9 @@ public CreateSnapshotPlan() {
public List<Path> getPaths() {
return new ArrayList<>();
}
+
+ @Override
+ public List<String> getPathsStrings() {
+ return new ArrayList<>();
Review comment:
Use Collections.emptyList().
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertTabletPlan.java
##########
@@ -125,6 +125,23 @@ public void setRange(List<Integer> range) {
return ret;
}
+ @Override
+ public List<String> getPathsStrings() {
+ if (paths != null) {
+ List<String> ret = new ArrayList<>();
+ for (Path path : paths) {
+ ret.add(path.getFullPath());
+ }
+ return ret;
+ }
+ List<Path> ret = new ArrayList<>();
+ for (String m : measurements) {
+ ret.add(new Path(deviceId, m));
Review comment:
Just use `ret.add(deviceId + IoTDBConstant.PATH_SEPARATOR + m)` and
return ret.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/LoadConfigurationPlan.java
##########
@@ -115,6 +115,11 @@ public void deserialize(ByteBuffer buffer) {
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
`Collections.emptyList()`
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/ShowPlan.java
##########
@@ -39,6 +39,11 @@ public ShowPlan(ShowContentType showContentType){
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
And here.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/OperateFilePlan.java
##########
@@ -54,6 +54,11 @@ public OperateFilePlan(File file, File targetDir,
OperatorType operatorType) {
return null;
}
+ @Override
+ public List<String> getPathsStrings() {
+ return null;
Review comment:
And there.
##########
File path:
server/src/main/java/org/apache/iotdb/db/qp/physical/sys/LoadDataPlan.java
##########
@@ -48,6 +48,15 @@ public LoadDataPlan(String inputFilePath, String
measureType) {
return ret;
}
+ @Override
+ public List<String> getPathsStrings() {
+ List<String> ret = new ArrayList<>();
+ if (measureType != null) {
+ ret.add(measureType);
+ }
+ return ret;
Review comment:
Here too.
----------------------------------------------------------------
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
