Copilot commented on code in PR #16435:
URL: https://github.com/apache/iotdb/pull/16435#discussion_r2378187231
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/PipeRealtimeDataRegionSource.java:
##########
@@ -237,7 +237,8 @@ public void customize(
if (PathUtils.isTableModelDatabase(databaseName)) {
isDbNameCoveredByPattern = tablePattern.coversDb(databaseName);
} else {
- isDbNameCoveredByPattern = treePattern.coversDb(databaseName);
+ isDbNameCoveredByPattern =
+ treePatterns.stream().allMatch(treePattern ->
treePattern.coversDb(databaseName));
Review Comment:
Using `allMatch()` here means the database is only considered covered if ALL
patterns cover it. This seems incorrect - the database should be covered if ANY
pattern covers it. Consider using `anyMatch()` instead.
```suggestion
treePatterns.stream().anyMatch(treePattern ->
treePattern.coversDb(databaseName));
```
##########
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()) {
Review Comment:
This condition checks if the pattern string is empty, but an empty string
should likely be handled differently than creating a pattern with an empty
string. Consider checking for null or whitespace-only strings instead.
```suggestion
if (pattern == null || pattern.trim().isEmpty()) {
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/historical/PipeHistoricalDataRegionTsFileAndDeletionSource.java:
##########
@@ -338,7 +338,8 @@ public void customize(
if (isTableModel) {
isDbNameCoveredByPattern = tablePattern.coversDb(databaseName);
} else {
- isDbNameCoveredByPattern = treePattern.coversDb(databaseName);
+ isDbNameCoveredByPattern =
+ treePatterns.stream().allMatch(treePattern ->
treePattern.coversDb(databaseName));
Review Comment:
Same issue as in PipeRealtimeDataRegionSource - using `allMatch()` here
means the database is only considered covered if ALL patterns cover it. This
should likely be `anyMatch()` to match if any pattern covers the database.
```suggestion
treePatterns.stream().anyMatch(treePattern ->
treePattern.coversDb(databaseName));
```
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/event/EnrichedEvent.java:
##########
@@ -559,8 +567,8 @@ public String coreReportMessage() {
+ "', replicateIndexForIoTV2="
+ replicateIndexForIoTV2
+ ", treePattern='"
- + treePattern
- + "', tablePattern='"
+ + treePatterns
+ + "', tablePatterns='"
Review Comment:
The field name in the string should be 'tablePattern' to match the actual
field name, not 'tablePatterns'.
```suggestion
+ "', tablePattern='"
```
--
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]
