Copilot commented on code in PR #16789:
URL: https://github.com/apache/iotdb/pull/16789#discussion_r2554978833
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -274,65 +286,145 @@ public static TreePattern parsePatternFromString(
}
/**
- * A private helper method to parse a set of 'path' and 'pattern' keys into
a single union
- * TreePattern. This contains the original logic of
parsePipePatternFromSourceParameters.
- *
- * @param sourceParameters The source parameters.
- * @param isTreeModelDataAllowedToBeCaptured Flag for TreePattern
constructor.
- * @param extractorPathKey Key for extractor path (e.g., "extractor.path").
- * @param sourcePathKey Key for source path (e.g., "source.path").
- * @param extractorPatternKey Key for extractor pattern (e.g.,
"extractor.pattern").
- * @param sourcePatternKey Key for source pattern (e.g., "source.pattern").
- * @param defaultPattern The pattern to return if both path and pattern are
null. If this
- * parameter is null, this method returns null.
- * @return The parsed TreePattern, or defaultPattern, or null if
defaultPattern is null and no
- * patterns are specified.
+ * Helper method to parse pattern parameters into a list of patterns without
creating the Union
+ * object immediately.
*/
- private static TreePattern parsePatternUnion(
+ private static List<TreePattern> parsePatternList(
final PipeParameters sourceParameters,
final boolean isTreeModelDataAllowedToBeCaptured,
final String extractorPathKey,
final String sourcePathKey,
final String extractorPatternKey,
- final String sourcePatternKey,
- final TreePattern defaultPattern) {
+ final String sourcePatternKey) {
final String path = sourceParameters.getStringByKeys(extractorPathKey,
sourcePathKey);
final String pattern =
sourceParameters.getStringByKeys(extractorPatternKey, sourcePatternKey);
- // 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.
+ final List<TreePattern> result = new ArrayList<>();
+
+ if (path != null) {
result.addAll(
parseMultiplePatterns(
path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
- // Parse "source.pattern" using the helper method.
+ }
+
+ if (pattern != null) {
result.addAll(
parsePatternsFromPatternParameter(
pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
- return buildUnionPattern(isTreeModelDataAllowedToBeCaptured, result);
}
- // 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)));
+ return result;
+ }
+
+ /**
+ * Removes patterns from the list that are covered by other patterns in the
same list. For
+ * example, if "root.**" and "root.db.**" are present, "root.db.**" is
removed.
+ */
+ private static List<TreePattern> optimizePatterns(final List<TreePattern>
patterns) {
+ if (patterns == null || patterns.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (patterns.size() == 1) {
+ return patterns;
}
- // 3. If only "source.pattern" is specified, parse it using the helper
method.
- if (pattern != null) {
- return buildUnionPattern(
- isTreeModelDataAllowedToBeCaptured,
- parsePatternsFromPatternParameter(
- pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ final List<TreePattern> optimized = new ArrayList<>();
+ // Determine coverage using base paths
+ for (int i = 0; i < patterns.size(); i++) {
+ final TreePattern current = patterns.get(i);
+ boolean isCoveredByOther = false;
+
+ for (int j = 0; j < patterns.size(); j++) {
+ if (i == j) {
+ continue;
+ }
+ final TreePattern other = patterns.get(j);
+
+ // If 'other' covers 'current', 'current' is redundant.
+ // Note: if they mutually cover each other (duplicates), we must
ensure we keep one.
+ // We use index comparison to break ties for exact duplicates.
+ if (covers(other, current)) {
+ if (covers(current, other)) {
+ // Both cover each other (likely identical). Keep the one with
smaller index.
+ if (j < i) {
+ isCoveredByOther = true;
+ break;
+ }
+ } else {
+ // Strict coverage
+ isCoveredByOther = true;
+ break;
+ }
+ }
+ }
+
+ if (!isCoveredByOther) {
+ optimized.add(current);
+ }
+ }
+ return optimized;
+ }
+
+ /**
+ * Prunes patterns from the inclusion list that are fully covered by ANY
pattern in the exclusion
+ * list.
+ */
+ private static List<TreePattern> pruneInclusionPatterns(
+ final List<TreePattern> inclusion, final List<TreePattern> exclusion) {
+ if (inclusion == null || inclusion.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (exclusion == null || exclusion.isEmpty()) {
+ return inclusion;
}
- // 4. If neither "source.path" nor "source.pattern" is specified,
- // return the provided default pattern (which may be null).
- return defaultPattern;
+ final List<TreePattern> prunedInclusion = new ArrayList<>();
+ for (final TreePattern inc : inclusion) {
+ boolean isFullyExcluded = false;
+ for (final TreePattern exc : exclusion) {
+ if (covers(exc, inc)) {
+ isFullyExcluded = true;
+ break;
+ }
+ }
+ if (!isFullyExcluded) {
+ prunedInclusion.add(inc);
+ }
+ }
+ return prunedInclusion;
+ }
+
+ /** Checks if 'coverer' pattern fully covers 'coveree' pattern. */
+ private static boolean covers(final TreePattern coverer, final TreePattern
coveree) {
+ try {
+ final List<PartialPath> covererPaths = coverer.getBaseInclusionPaths();
+ final List<PartialPath> covereePaths = coveree.getBaseInclusionPaths();
+
+ if (covererPaths.isEmpty() || covereePaths.isEmpty()) {
+ return false;
+ }
+
+ // Logic: For 'coverer' to cover 'coveree', ALL paths in 'coveree' must
be included
+ // by at least one path in 'coverer'.
+ for (final PartialPath sub : covereePaths) {
+ boolean isSubCovered = false;
+ for (final PartialPath sup : covererPaths) {
+ if (sup.include(sub)) {
+ isSubCovered = true;
+ break;
+ }
+ }
+ if (!isSubCovered) {
+ return false;
+ }
+ }
+ return true;
+ } catch (final Exception e) {
+ // In case of path parsing errors or unsupported operations, assume no
coverage
+ // to be safe and avoid aggressive pruning.
+ return false;
+ }
}
Review Comment:
The new methods `optimizePatterns`, `pruneInclusionPatterns`, and `covers`
lack documentation. While `optimizePatterns` has a brief comment explaining its
purpose, the other two methods are completely undocumented. Consider adding
JavaDoc comments that explain:
- The purpose and algorithm of each method
- Parameter descriptions and constraints
- Return value specifications
- Edge cases and examples
For instance, `covers` should document what "covers" means in terms of path
matching semantics.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -274,65 +286,145 @@ public static TreePattern parsePatternFromString(
}
/**
- * A private helper method to parse a set of 'path' and 'pattern' keys into
a single union
- * TreePattern. This contains the original logic of
parsePipePatternFromSourceParameters.
- *
- * @param sourceParameters The source parameters.
- * @param isTreeModelDataAllowedToBeCaptured Flag for TreePattern
constructor.
- * @param extractorPathKey Key for extractor path (e.g., "extractor.path").
- * @param sourcePathKey Key for source path (e.g., "source.path").
- * @param extractorPatternKey Key for extractor pattern (e.g.,
"extractor.pattern").
- * @param sourcePatternKey Key for source pattern (e.g., "source.pattern").
- * @param defaultPattern The pattern to return if both path and pattern are
null. If this
- * parameter is null, this method returns null.
- * @return The parsed TreePattern, or defaultPattern, or null if
defaultPattern is null and no
- * patterns are specified.
+ * Helper method to parse pattern parameters into a list of patterns without
creating the Union
+ * object immediately.
*/
- private static TreePattern parsePatternUnion(
+ private static List<TreePattern> parsePatternList(
final PipeParameters sourceParameters,
final boolean isTreeModelDataAllowedToBeCaptured,
final String extractorPathKey,
final String sourcePathKey,
final String extractorPatternKey,
- final String sourcePatternKey,
- final TreePattern defaultPattern) {
+ final String sourcePatternKey) {
final String path = sourceParameters.getStringByKeys(extractorPathKey,
sourcePathKey);
final String pattern =
sourceParameters.getStringByKeys(extractorPatternKey, sourcePatternKey);
- // 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.
+ final List<TreePattern> result = new ArrayList<>();
+
+ if (path != null) {
result.addAll(
parseMultiplePatterns(
path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
- // Parse "source.pattern" using the helper method.
+ }
+
+ if (pattern != null) {
result.addAll(
parsePatternsFromPatternParameter(
pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
- return buildUnionPattern(isTreeModelDataAllowedToBeCaptured, result);
}
- // 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)));
+ return result;
+ }
+
+ /**
+ * Removes patterns from the list that are covered by other patterns in the
same list. For
+ * example, if "root.**" and "root.db.**" are present, "root.db.**" is
removed.
+ */
+ private static List<TreePattern> optimizePatterns(final List<TreePattern>
patterns) {
+ if (patterns == null || patterns.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (patterns.size() == 1) {
+ return patterns;
}
- // 3. If only "source.pattern" is specified, parse it using the helper
method.
- if (pattern != null) {
- return buildUnionPattern(
- isTreeModelDataAllowedToBeCaptured,
- parsePatternsFromPatternParameter(
- pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ final List<TreePattern> optimized = new ArrayList<>();
+ // Determine coverage using base paths
+ for (int i = 0; i < patterns.size(); i++) {
+ final TreePattern current = patterns.get(i);
+ boolean isCoveredByOther = false;
+
+ for (int j = 0; j < patterns.size(); j++) {
+ if (i == j) {
+ continue;
+ }
+ final TreePattern other = patterns.get(j);
+
+ // If 'other' covers 'current', 'current' is redundant.
+ // Note: if they mutually cover each other (duplicates), we must
ensure we keep one.
+ // We use index comparison to break ties for exact duplicates.
+ if (covers(other, current)) {
+ if (covers(current, other)) {
+ // Both cover each other (likely identical). Keep the one with
smaller index.
+ if (j < i) {
+ isCoveredByOther = true;
+ break;
+ }
+ } else {
+ // Strict coverage
+ isCoveredByOther = true;
+ break;
+ }
+ }
+ }
+
+ if (!isCoveredByOther) {
+ optimized.add(current);
+ }
+ }
+ return optimized;
+ }
+
+ /**
+ * Prunes patterns from the inclusion list that are fully covered by ANY
pattern in the exclusion
+ * list.
+ */
+ private static List<TreePattern> pruneInclusionPatterns(
+ final List<TreePattern> inclusion, final List<TreePattern> exclusion) {
+ if (inclusion == null || inclusion.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (exclusion == null || exclusion.isEmpty()) {
+ return inclusion;
}
- // 4. If neither "source.path" nor "source.pattern" is specified,
- // return the provided default pattern (which may be null).
- return defaultPattern;
+ final List<TreePattern> prunedInclusion = new ArrayList<>();
+ for (final TreePattern inc : inclusion) {
+ boolean isFullyExcluded = false;
+ for (final TreePattern exc : exclusion) {
+ if (covers(exc, inc)) {
+ isFullyExcluded = true;
+ break;
+ }
+ }
+ if (!isFullyExcluded) {
+ prunedInclusion.add(inc);
+ }
+ }
+ return prunedInclusion;
+ }
+
+ /** Checks if 'coverer' pattern fully covers 'coveree' pattern. */
+ private static boolean covers(final TreePattern coverer, final TreePattern
coveree) {
+ try {
+ final List<PartialPath> covererPaths = coverer.getBaseInclusionPaths();
+ final List<PartialPath> covereePaths = coveree.getBaseInclusionPaths();
+
+ if (covererPaths.isEmpty() || covereePaths.isEmpty()) {
+ return false;
+ }
+
+ // Logic: For 'coverer' to cover 'coveree', ALL paths in 'coveree' must
be included
+ // by at least one path in 'coverer'.
+ for (final PartialPath sub : covereePaths) {
+ boolean isSubCovered = false;
+ for (final PartialPath sup : covererPaths) {
+ if (sup.include(sub)) {
+ isSubCovered = true;
+ break;
+ }
+ }
+ if (!isSubCovered) {
+ return false;
+ }
+ }
+ return true;
+ } catch (final Exception e) {
+ // In case of path parsing errors or unsupported operations, assume no
coverage
+ // to be safe and avoid aggressive pruning.
+ return false;
+ }
Review Comment:
The `covers` method silently catches all exceptions and returns `false`
without logging. While the comment explains this is to "be safe and avoid
aggressive pruning", this could hide legitimate errors such as
IllegalPathException or NullPointerException. Consider logging the exception at
DEBUG or TRACE level to aid in debugging, or at least being more specific about
which exceptions are expected (e.g., catch only IllegalPathException).
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -412,6 +504,10 @@ private static List<TreePattern> parseMultiplePatterns(
*/
private static TreePattern buildUnionPattern(
final boolean isTreeModelDataAllowedToBeCaptured, final
List<TreePattern> patterns) {
Review Comment:
The buildUnionPattern method now has an early return for single-element
lists, but it doesn't validate that the list is non-empty. If an empty list is
passed, it will throw an IndexOutOfBoundsException when trying to access
`patterns.get(0)`. Consider adding a null/empty check before this optimization.
```suggestion
final boolean isTreeModelDataAllowedToBeCaptured, final
List<TreePattern> patterns) {
if (patterns == null || patterns.isEmpty()) {
throw new IllegalArgumentException("patterns list must not be null or
empty");
}
```
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -140,60 +140,72 @@ public static TreePattern
parsePipePatternFromSourceParameters(
final boolean isTreeModelDataAllowedToBeCaptured =
isTreeModelDataAllowToBeCaptured(sourceParameters);
- // 1. Define the default inclusion pattern (matches all, "root.**")
- // This is used if no inclusion patterns are specified.
- final TreePattern defaultInclusionPattern =
- buildUnionPattern(
- isTreeModelDataAllowedToBeCaptured,
- Collections.singletonList(
- new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured,
null)));
-
- // 2. Parse INCLUSION patterns using the helper
- final TreePattern inclusionPattern =
- parsePatternUnion(
+ // 1. Parse INCLUSION patterns into a list
+ List<TreePattern> inclusionPatterns =
+ parsePatternList(
sourceParameters,
isTreeModelDataAllowedToBeCaptured,
- // 'path' keys (IoTDB wildcard)
EXTRACTOR_PATH_KEY,
SOURCE_PATH_KEY,
- // 'pattern' keys (Prefix or IoTDB via format)
EXTRACTOR_PATTERN_KEY,
- SOURCE_PATTERN_KEY,
- // Default pattern if no keys are found
- defaultInclusionPattern);
+ SOURCE_PATTERN_KEY);
+
+ // If no inclusion patterns are specified, use default "root.**"
+ if (inclusionPatterns.isEmpty()) {
+ inclusionPatterns =
+ new ArrayList<>(
+ Collections.singletonList(
+ new IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured,
null)));
+ }
- // 3. Parse EXCLUSION patterns using the helper
- final TreePattern exclusionPattern =
- parsePatternUnion(
+ // 2. Parse EXCLUSION patterns into a list
+ List<TreePattern> exclusionPatterns =
+ parsePatternList(
sourceParameters,
isTreeModelDataAllowedToBeCaptured,
- // 'path.exclusion' keys (IoTDB wildcard)
EXTRACTOR_PATH_EXCLUSION_KEY,
SOURCE_PATH_EXCLUSION_KEY,
- // 'pattern.exclusion' keys (Prefix)
EXTRACTOR_PATTERN_EXCLUSION_KEY,
- SOURCE_PATTERN_EXCLUSION_KEY,
- // Default for exclusion is "match nothing" (null)
- null);
-
- // 4. Combine inclusion and exclusion
- if (exclusionPattern == null) {
- // No exclusion defined, return the inclusion pattern directly
- return inclusionPattern;
- } else {
- // If both inclusion and exclusion patterns support IoTDB operations,
- // use the specialized ExclusionIoTDBTreePattern
- if (inclusionPattern instanceof IoTDBTreePatternOperations
- && exclusionPattern instanceof IoTDBTreePatternOperations) {
- return new WithExclusionIoTDBTreePattern(
- isTreeModelDataAllowedToBeCaptured,
- (IoTDBTreePatternOperations) inclusionPattern,
- (IoTDBTreePatternOperations) exclusionPattern);
- }
- // Both are defined, wrap them in an ExclusionTreePattern
- return new WithExclusionTreePattern(
- isTreeModelDataAllowedToBeCaptured, inclusionPattern,
exclusionPattern);
+ SOURCE_PATTERN_EXCLUSION_KEY);
+
+ // 3. Optimize the lists: remove redundant patterns (e.g., if "root.**"
exists, "root.db" is
+ // redundant)
+ inclusionPatterns = optimizePatterns(inclusionPatterns);
+ exclusionPatterns = optimizePatterns(exclusionPatterns);
+
+ // 4. Prune inclusion patterns: if an inclusion pattern is fully covered
by an exclusion
+ // pattern, remove it
+ inclusionPatterns = pruneInclusionPatterns(inclusionPatterns,
exclusionPatterns);
+
+ // 5. Check if the resulting inclusion pattern is empty
+ if (inclusionPatterns.isEmpty()) {
+ throw new PipeException(
+ "Pipe: The inclusion pattern is empty after pruning by the exclusion
pattern. "
+ + "This pipe pattern will match nothing.");
+ }
Review Comment:
[nitpick] The pruning logic at this stage may result in redundant checks
with the existing `checkAndLogPatternCoverage` method called in
`WithExclusionIoTDBTreePattern` and `WithExclusionTreePattern` constructors
(lines 201-204, 207-208). Both perform coverage checks, but at different
granularities:
- The new pruning checks individual patterns in lists before building unions
- The existing check validates final TreePattern objects after construction
While this provides defense-in-depth, it could lead to different error
messages for the same underlying issue. Consider documenting this relationship
or consolidating the checks to avoid confusion.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -274,65 +286,145 @@ public static TreePattern parsePatternFromString(
}
/**
- * A private helper method to parse a set of 'path' and 'pattern' keys into
a single union
- * TreePattern. This contains the original logic of
parsePipePatternFromSourceParameters.
- *
- * @param sourceParameters The source parameters.
- * @param isTreeModelDataAllowedToBeCaptured Flag for TreePattern
constructor.
- * @param extractorPathKey Key for extractor path (e.g., "extractor.path").
- * @param sourcePathKey Key for source path (e.g., "source.path").
- * @param extractorPatternKey Key for extractor pattern (e.g.,
"extractor.pattern").
- * @param sourcePatternKey Key for source pattern (e.g., "source.pattern").
- * @param defaultPattern The pattern to return if both path and pattern are
null. If this
- * parameter is null, this method returns null.
- * @return The parsed TreePattern, or defaultPattern, or null if
defaultPattern is null and no
- * patterns are specified.
+ * Helper method to parse pattern parameters into a list of patterns without
creating the Union
+ * object immediately.
*/
- private static TreePattern parsePatternUnion(
+ private static List<TreePattern> parsePatternList(
final PipeParameters sourceParameters,
final boolean isTreeModelDataAllowedToBeCaptured,
final String extractorPathKey,
final String sourcePathKey,
final String extractorPatternKey,
- final String sourcePatternKey,
- final TreePattern defaultPattern) {
+ final String sourcePatternKey) {
final String path = sourceParameters.getStringByKeys(extractorPathKey,
sourcePathKey);
final String pattern =
sourceParameters.getStringByKeys(extractorPatternKey, sourcePatternKey);
- // 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.
+ final List<TreePattern> result = new ArrayList<>();
+
+ if (path != null) {
result.addAll(
parseMultiplePatterns(
path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
- // Parse "source.pattern" using the helper method.
+ }
+
+ if (pattern != null) {
result.addAll(
parsePatternsFromPatternParameter(
pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
- return buildUnionPattern(isTreeModelDataAllowedToBeCaptured, result);
}
- // 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)));
+ return result;
+ }
+
+ /**
+ * Removes patterns from the list that are covered by other patterns in the
same list. For
+ * example, if "root.**" and "root.db.**" are present, "root.db.**" is
removed.
+ */
+ private static List<TreePattern> optimizePatterns(final List<TreePattern>
patterns) {
+ if (patterns == null || patterns.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (patterns.size() == 1) {
+ return patterns;
}
- // 3. If only "source.pattern" is specified, parse it using the helper
method.
- if (pattern != null) {
- return buildUnionPattern(
- isTreeModelDataAllowedToBeCaptured,
- parsePatternsFromPatternParameter(
- pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ final List<TreePattern> optimized = new ArrayList<>();
+ // Determine coverage using base paths
+ for (int i = 0; i < patterns.size(); i++) {
+ final TreePattern current = patterns.get(i);
+ boolean isCoveredByOther = false;
+
+ for (int j = 0; j < patterns.size(); j++) {
+ if (i == j) {
+ continue;
+ }
+ final TreePattern other = patterns.get(j);
+
+ // If 'other' covers 'current', 'current' is redundant.
+ // Note: if they mutually cover each other (duplicates), we must
ensure we keep one.
+ // We use index comparison to break ties for exact duplicates.
+ if (covers(other, current)) {
+ if (covers(current, other)) {
+ // Both cover each other (likely identical). Keep the one with
smaller index.
+ if (j < i) {
+ isCoveredByOther = true;
+ break;
+ }
+ } else {
+ // Strict coverage
+ isCoveredByOther = true;
+ break;
+ }
+ }
+ }
+
+ if (!isCoveredByOther) {
+ optimized.add(current);
+ }
+ }
+ return optimized;
+ }
+
+ /**
+ * Prunes patterns from the inclusion list that are fully covered by ANY
pattern in the exclusion
+ * list.
+ */
+ private static List<TreePattern> pruneInclusionPatterns(
+ final List<TreePattern> inclusion, final List<TreePattern> exclusion) {
+ if (inclusion == null || inclusion.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (exclusion == null || exclusion.isEmpty()) {
+ return inclusion;
}
- // 4. If neither "source.path" nor "source.pattern" is specified,
- // return the provided default pattern (which may be null).
- return defaultPattern;
+ final List<TreePattern> prunedInclusion = new ArrayList<>();
+ for (final TreePattern inc : inclusion) {
+ boolean isFullyExcluded = false;
+ for (final TreePattern exc : exclusion) {
+ if (covers(exc, inc)) {
+ isFullyExcluded = true;
+ break;
+ }
+ }
+ if (!isFullyExcluded) {
+ prunedInclusion.add(inc);
+ }
+ }
+ return prunedInclusion;
+ }
Review Comment:
The documentation for `pruneInclusionPatterns` is incomplete. While the
method has a one-line comment, it should have proper JavaDoc documentation
explaining:
- What "fully covered" means in this context
- Examples of when patterns would be pruned
- Edge case behavior (e.g., empty lists)
- The relationship with the `covers` helper method
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java:
##########
@@ -274,65 +286,145 @@ public static TreePattern parsePatternFromString(
}
/**
- * A private helper method to parse a set of 'path' and 'pattern' keys into
a single union
- * TreePattern. This contains the original logic of
parsePipePatternFromSourceParameters.
- *
- * @param sourceParameters The source parameters.
- * @param isTreeModelDataAllowedToBeCaptured Flag for TreePattern
constructor.
- * @param extractorPathKey Key for extractor path (e.g., "extractor.path").
- * @param sourcePathKey Key for source path (e.g., "source.path").
- * @param extractorPatternKey Key for extractor pattern (e.g.,
"extractor.pattern").
- * @param sourcePatternKey Key for source pattern (e.g., "source.pattern").
- * @param defaultPattern The pattern to return if both path and pattern are
null. If this
- * parameter is null, this method returns null.
- * @return The parsed TreePattern, or defaultPattern, or null if
defaultPattern is null and no
- * patterns are specified.
+ * Helper method to parse pattern parameters into a list of patterns without
creating the Union
+ * object immediately.
*/
- private static TreePattern parsePatternUnion(
+ private static List<TreePattern> parsePatternList(
final PipeParameters sourceParameters,
final boolean isTreeModelDataAllowedToBeCaptured,
final String extractorPathKey,
final String sourcePathKey,
final String extractorPatternKey,
- final String sourcePatternKey,
- final TreePattern defaultPattern) {
+ final String sourcePatternKey) {
final String path = sourceParameters.getStringByKeys(extractorPathKey,
sourcePathKey);
final String pattern =
sourceParameters.getStringByKeys(extractorPatternKey, sourcePatternKey);
- // 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.
+ final List<TreePattern> result = new ArrayList<>();
+
+ if (path != null) {
result.addAll(
parseMultiplePatterns(
path, p -> new
IoTDBTreePattern(isTreeModelDataAllowedToBeCaptured, p)));
- // Parse "source.pattern" using the helper method.
+ }
+
+ if (pattern != null) {
result.addAll(
parsePatternsFromPatternParameter(
pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
- return buildUnionPattern(isTreeModelDataAllowedToBeCaptured, result);
}
- // 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)));
+ return result;
+ }
+
+ /**
+ * Removes patterns from the list that are covered by other patterns in the
same list. For
+ * example, if "root.**" and "root.db.**" are present, "root.db.**" is
removed.
+ */
+ private static List<TreePattern> optimizePatterns(final List<TreePattern>
patterns) {
+ if (patterns == null || patterns.isEmpty()) {
+ return new ArrayList<>();
+ }
+ if (patterns.size() == 1) {
+ return patterns;
}
- // 3. If only "source.pattern" is specified, parse it using the helper
method.
- if (pattern != null) {
- return buildUnionPattern(
- isTreeModelDataAllowedToBeCaptured,
- parsePatternsFromPatternParameter(
- pattern, sourceParameters, isTreeModelDataAllowedToBeCaptured));
+ final List<TreePattern> optimized = new ArrayList<>();
+ // Determine coverage using base paths
+ for (int i = 0; i < patterns.size(); i++) {
+ final TreePattern current = patterns.get(i);
+ boolean isCoveredByOther = false;
+
+ for (int j = 0; j < patterns.size(); j++) {
+ if (i == j) {
+ continue;
+ }
+ final TreePattern other = patterns.get(j);
+
+ // If 'other' covers 'current', 'current' is redundant.
+ // Note: if they mutually cover each other (duplicates), we must
ensure we keep one.
+ // We use index comparison to break ties for exact duplicates.
+ if (covers(other, current)) {
+ if (covers(current, other)) {
+ // Both cover each other (likely identical). Keep the one with
smaller index.
+ if (j < i) {
+ isCoveredByOther = true;
+ break;
+ }
+ } else {
+ // Strict coverage
+ isCoveredByOther = true;
+ break;
+ }
+ }
+ }
+
+ if (!isCoveredByOther) {
+ optimized.add(current);
+ }
+ }
+ return optimized;
+ }
Review Comment:
[nitpick] The `optimizePatterns` method has O(n²) complexity where each
pattern is compared against all others, and each comparison involves calling
`covers()` which iterates over paths. While this is acceptable for small
pattern lists (typically a handful), consider documenting the expected maximum
size of pattern lists in the method documentation to set appropriate
expectations.
--
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]
