Copilot commented on code in PR #16575:
URL: https://github.com/apache/iotdb/pull/16575#discussion_r2428350702
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -74,42 +76,142 @@ public static TreePattern
parsePipePatternFromSourceParameters(
isTreeModelDataAllowToBeCaptured(sourceParameters);
final String path = sourceParameters.getStringByKeys(EXTRACTOR_PATH_KEY,
SOURCE_PATH_KEY);
+ final String pattern =
+ sourceParameters.getStringByKeys(EXTRACTOR_PATTERN_KEY,
SOURCE_PATTERN_KEY);
- // 1. If "source.path" is specified, it will be interpreted as an
IoTDB-style path,
- // ignoring the other 2 parameters.
- if (path != null) {
- return new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, path);
+ // 1. If both "source.path" and "source.pattern" are specified, their
union will be used.
+ if (path != null && pattern != null) {
+ final List<TreePattern> result = new ArrayList<>();
+ // Parse "source.path" as IoTDB-style path.
+ result.addAll(
+ parseMultiplePatterns(
+ path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
+ // Parse "source.pattern" using the helper method.
+ result.addAll(
+ parsePatternsFromPatternParameter(
+ pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ return buildUnionPattern(isTreeModelDataAllowedToBeCaptured, result);
}
- final String pattern =
- sourceParameters.getStringByKeys(EXTRACTOR_PATTERN_KEY,
SOURCE_PATTERN_KEY);
+ // 2. If only "source.path" is specified, it will be interpreted as an
IoTDB-style path.
+ if (path != null) {
+ return buildUnionPattern(
+ isTreeModelDataAllowedToBeCaptured,
+ parseMultiplePatterns(
+ path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
+ }
- // 2. Otherwise, If "source.pattern" is specified, it will be interpreted
- // according to "source.pattern.format".
+ // 3. If only "source.pattern" is specified, parse it using the helper
method.
if (pattern != null) {
- final String patternFormat =
- sourceParameters.getStringByKeys(EXTRACTOR_PATTERN_FORMAT_KEY,
SOURCE_PATTERN_FORMAT_KEY);
+ return buildUnionPattern(
+ isTreeModelDataAllowedToBeCaptured,
+ parsePatternsFromPatternParameter(
+ pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ }
- // If "source.pattern.format" is not specified, use prefix format by
default.
- if (patternFormat == null) {
- return new PrefixTreePattern(isTreeModelDataAllowedToBeCaptured,
pattern);
+ // 4. If neither "source.path" nor "source.pattern" is specified,
+ // this pipe source will match all data.
+ return buildUnionPattern(
+ isTreeModelDataAllowedToBeCaptured,
+ Collections.singletonList(new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, null)));
+ }
+
+ /**
+ * A private helper method to parse a list of {@link TreePattern}s from the
"pattern" parameter,
+ * considering its "format".
+ *
+ * @param pattern The pattern string to parse.
+ * @param sourceParameters The source parameters to read the format from.
+ * @param isTreeModelDataAllowedToBeCaptured A boolean flag passed to the
TreePattern constructor.
+ * @return A list of parsed {@link TreePattern}s.
+ */
+ private static List<TreePattern> parsePatternsFromPatternParameter(
+ final String pattern,
+ final PipeParameters sourceParameters,
+ final boolean isTreeModelDataAllowedToBeCaptured) {
+ final String patternFormat =
+ sourceParameters.getStringByKeys(EXTRACTOR_PATTERN_FORMAT_KEY,
SOURCE_PATTERN_FORMAT_KEY);
+
+ // If "source.pattern.format" is not specified, use prefix format by
default.
+ if (patternFormat == null) {
+ return parseMultiplePatterns(
+ pattern, p -> new
PrefixTreePattern(isTreeModelDataAllowedToBeCaptured, p));
+ }
+
+ switch (patternFormat.toLowerCase()) {
+ case EXTRACTOR_PATTERN_FORMAT_IOTDB_VALUE:
+ return parseMultiplePatterns(
+ pattern, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p));
+ case EXTRACTOR_PATTERN_FORMAT_PREFIX_VALUE:
+ return parseMultiplePatterns(
+ pattern, p -> new
PrefixTreePattern(isTreeModelDataAllowedToBeCaptured, p));
+ default:
+ LOGGER.info(
+ "Unknown pattern format: {}, use prefix matching format by
default.", patternFormat);
+ return parseMultiplePatterns(
+ pattern, p -> new
PrefixTreePattern(isTreeModelDataAllowedToBeCaptured, p));
+ }
+ }
+
+ public static List<TreePattern> parseMultiplePatterns(
+ final String pattern, final Function<String, TreePattern>
patternSupplier) {
+ if (pattern.isEmpty()) {
+ return Collections.singletonList(patternSupplier.apply(pattern));
+ }
+
+ final List<TreePattern> patterns = new ArrayList<>();
+ final StringBuilder currentPattern = new StringBuilder();
+ boolean inBackticks = false;
+
+ for (final char c : pattern.toCharArray()) {
+ if (c == '`') {
+ inBackticks = !inBackticks;
+ currentPattern.append(c);
+ } else if (c == ',' && !inBackticks) {
+ final String singlePattern = currentPattern.toString().trim();
+ if (!singlePattern.isEmpty()) {
+ patterns.add(patternSupplier.apply(singlePattern));
+ }
+ currentPattern.setLength(0);
+ } else {
+ currentPattern.append(c);
}
+ }
+
+ final String lastPattern = currentPattern.toString().trim();
+ if (!lastPattern.isEmpty()) {
+ patterns.add(patternSupplier.apply(lastPattern));
+ }
+
+ return patterns;
+ }
- switch (patternFormat.toLowerCase()) {
- case EXTRACTOR_PATTERN_FORMAT_IOTDB_VALUE:
- return new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured,
pattern);
- case EXTRACTOR_PATTERN_FORMAT_PREFIX_VALUE:
- return new PrefixTreePattern(isTreeModelDataAllowedToBeCaptured,
pattern);
- default:
- LOGGER.info(
- "Unknown pattern format: {}, use prefix matching format by
default.", patternFormat);
- return new PrefixTreePattern(isTreeModelDataAllowedToBeCaptured,
pattern);
+ /**
+ * A private helper method to build the most specific UnionTreePattern
possible. If all patterns
+ * are IoTDBTreePattern, it returns an IoTDBUnionTreePattern. Otherwise, it
returns a general
Review Comment:
The Javadoc refers to 'IoTDBUnionTreePattern', but the actual class name is
'UnionIoTDBTreePattern'. Please update the Javadoc to avoid confusion.
```suggestion
* are IoTDBTreePattern, it returns a UnionIoTDBTreePattern. Otherwise, it
returns a general
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/schemaregion/PipePlanTreePatternParseVisitor.java:
##########
@@ -98,19 +98,20 @@ public Optional<PlanNode> visitCreateAlignedTimeSeries(
new CreateAlignedTimeSeriesNode(
node.getPlanNodeId(),
node.getDevicePath(),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getMeasurements()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getDataTypes()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getEncodings()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getCompressors()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getAliasList()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getTagsList()),
- IoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getAttributesList())))
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getMeasurements()),
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getDataTypes()),
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getEncodings()),
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getCompressors()),
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getAliasList()),
+ UnionIoTDBTreePattern.applyIndexesOnList(filteredIndexes,
node.getTagsList()),
+ UnionIoTDBTreePattern.applyIndexesOnList(
+ filteredIndexes, node.getAttributesList())))
Review Comment:
UnionIoTDBTreePattern does not define a static method applyIndexesOnList;
this will not compile. Use the shared utility TreePattern.applyIndexesOnList
instead for all list projections.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/receiver/protocol/thrift/IoTDBDataNodeReceiver.java:
##########
@@ -629,7 +635,7 @@ private TSStatus loadSchemaSnapShot(
// Here we apply the statements as many as possible
// Even if there are failed statements
Review Comment:
This unchecked cast assumes the built TreePattern is always a
UnionIoTDBTreePattern. To make this robust, either construct a
UnionIoTDBTreePattern directly (e.g., map to List<IoTDBTreePattern> and new
UnionIoTDBTreePattern(...)) or guard with an instanceof check and fail fast
with a clear error if a non-IoTDB union is received.
```suggestion
// Even if there are failed statements
if (!(treePattern instanceof UnionIoTDBTreePattern)) {
throw new IllegalArgumentException(
"Expected treePattern to be UnionIoTDBTreePattern, but got: "
+ treePattern.getClass().getName());
}
```
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java:
##########
@@ -1051,22 +1053,30 @@ protected TSStatus loadFileV2(
final Set<ConfigPhysicalPlanType> executionTypes =
PipeConfigRegionSnapshotEvent.getConfigPhysicalPlanTypeSet(
parameters.get(ColumnHeaderConstant.TYPE));
- final IoTDBTreePattern treePattern =
- new IoTDBTreePattern(
- parameters.containsKey(PipeTransferFileSealReqV2.TREE),
- parameters.get(ColumnHeaderConstant.PATH_PATTERN));
+ final boolean isTreeModelDataAllowedToBeCaptured =
+ parameters.containsKey(PipeTransferFileSealReqV2.TREE);
+ final List<TreePattern> treePatterns =
+ TreePattern.parseMultiplePatterns(
+ parameters.get(ColumnHeaderConstant.PATH_PATTERN),
+ p -> new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p));
+ final TreePattern treePattern =
+ TreePattern.buildUnionPattern(isTreeModelDataAllowedToBeCaptured,
treePatterns);
final TablePattern tablePattern =
new TablePattern(
parameters.containsKey(PipeTransferFileSealReqV2.TABLE),
parameters.get(PipeTransferFileSealReqV2.DATABASE_PATTERN),
parameters.get(ColumnHeaderConstant.TABLE_NAME));
final List<TSStatus> results = new ArrayList<>();
while (generator.hasNext()) {
- IoTDBConfigRegionSource.parseConfigPlan(generator.next(), treePattern,
tablePattern)
+ IoTDBConfigRegionSource.parseConfigPlan(
+ generator.next(), (UnionIoTDBTreePattern) treePattern,
tablePattern)
.filter(
configPhysicalPlan ->
IoTDBConfigRegionSource.isTypeListened(
- configPhysicalPlan, executionTypes, treePattern,
tablePattern))
+ configPhysicalPlan,
+ executionTypes,
+ (UnionIoTDBTreePattern) treePattern,
+ tablePattern))
Review Comment:
Multiple unchecked casts to UnionIoTDBTreePattern assume the union is always
IoTDB-only. Consider constructing UnionIoTDBTreePattern directly when building
the pattern list (or validating with instanceof and throwing a descriptive
error) to avoid potential ClassCastException if mixed pattern types are ever
introduced here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
