Copilot commented on code in PR #16435:
URL: https://github.com/apache/iotdb/pull/16435#discussion_r2378215183
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -68,48 +72,97 @@ public boolean isRoot() {
*
* @return The interpreted {@link TreePattern} which is not {@code null}.
*/
- public static TreePattern parsePipePatternFromSourceParameters(
+ public static List<TreePattern> parsePipePatternFromSourceParameters(
final PipeParameters sourceParameters) {
final boolean isTreeModelDataAllowedToBeCaptured =
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 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 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 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 Collections.singletonList(
+ new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, null));
+ }
- 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 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));
}
- // 3. If neither "source.path" nor "source.pattern" is specified,
- // this pipe source will match all data.
- return new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, null);
+ 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));
+ }
+ }
+
+ private static List<TreePattern> parseMultiplePatterns(
+ final String pattern, final Function<String, TreePattern>
patternSupplier) {
+ if (pattern.isEmpty()) {
+ return Collections.singletonList(patternSupplier.apply(pattern));
Review Comment:
This condition will cause empty patterns to be processed as valid patterns.
An empty pattern should likely be filtered out or handled differently to avoid
creating meaningless pattern matchers.
```suggestion
// Return an empty list if the pattern is empty, to avoid creating
meaningless pattern matchers.
return Collections.emptyList();
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tablet/PipeRawTabletInsertionEvent.java:
##########
@@ -425,21 +438,31 @@ public Tablet convertToTablet() {
if (!shouldParseTimeOrPattern()) {
return tablet;
}
- return initEventParser().convertToTablet();
+ // TODO: handle multiple patterns
+ return initEventParsers().get(0).convertToTablet();
Review Comment:
Using `get(0)` without checking if the list is empty will cause an
IndexOutOfBoundsException. Add a null/empty check or handle the case where no
patterns exist.
```suggestion
List<TabletInsertionEventParser> parsers = initEventParsers();
if (parsers == null || parsers.isEmpty()) {
// Handle the case where no parser exists; return null or throw an
exception as appropriate
return null;
}
return parsers.get(0).convertToTablet();
```
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/event/EnrichedEvent.java:
##########
@@ -111,9 +111,11 @@ protected EnrichedEvent(
this.endTime = endTime;
isPatternParsed =
- (treePattern == null || treePattern.isRoot())
- && (tablePattern == null
- ||
!tablePattern.hasUserSpecifiedDatabasePatternOrTablePattern());
+ Objects.isNull(treePatterns)
+ || ((treePatterns.stream()
+ .allMatch(treePattern -> treePattern == null ||
treePattern.isRoot()))
Review Comment:
The nested null checks and stream operations make this logic complex.
Consider extracting this into a private helper method like
`areAllPatternsRootOrNull()` for better readability.
--
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]