Copilot commented on code in PR #16632: URL: https://github.com/apache/iotdb/pull/16632#discussion_r2484881555
########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/ExclusionIoTDBTreePattern.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.commons.pipe.datastructure.pattern; + +import org.apache.iotdb.commons.path.PartialPath; +import org.apache.iotdb.commons.path.PathPatternTree; + +import org.apache.tsfile.file.metadata.IDeviceID; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Represents an exclusion pattern specifically for IoTDB-operation-aware patterns. It holds an + * inclusion and exclusion pattern, both implementing {@link IoTDBPatternOperations}. + * + * <p>The logic is: "Matches inclusion AND NOT exclusion" for all methods. + */ +public class ExclusionIoTDBTreePattern extends TreePattern implements IoTDBPatternOperations { + + private final IoTDBPatternOperations inclusionPattern; + private final IoTDBPatternOperations exclusionPattern; + + public ExclusionIoTDBTreePattern( + final boolean isTreeModelDataAllowedToBeCaptured, + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(isTreeModelDataAllowedToBeCaptured); + this.inclusionPattern = inclusionPattern; + this.exclusionPattern = exclusionPattern; + } + + public ExclusionIoTDBTreePattern( + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(true); Review Comment: The constructor hardcodes `isTreeModelDataAllowedToBeCaptured` to `true` without explanation. This should be documented in a JavaDoc comment explaining why this default is used and when this constructor should be preferred over the three-parameter version. ########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/ExclusionIoTDBTreePattern.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.commons.pipe.datastructure.pattern; + +import org.apache.iotdb.commons.path.PartialPath; +import org.apache.iotdb.commons.path.PathPatternTree; + +import org.apache.tsfile.file.metadata.IDeviceID; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Represents an exclusion pattern specifically for IoTDB-operation-aware patterns. It holds an + * inclusion and exclusion pattern, both implementing {@link IoTDBPatternOperations}. + * + * <p>The logic is: "Matches inclusion AND NOT exclusion" for all methods. + */ +public class ExclusionIoTDBTreePattern extends TreePattern implements IoTDBPatternOperations { + + private final IoTDBPatternOperations inclusionPattern; + private final IoTDBPatternOperations exclusionPattern; + + public ExclusionIoTDBTreePattern( + final boolean isTreeModelDataAllowedToBeCaptured, + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(isTreeModelDataAllowedToBeCaptured); + this.inclusionPattern = inclusionPattern; + this.exclusionPattern = exclusionPattern; + } + + public ExclusionIoTDBTreePattern( + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(true); + this.inclusionPattern = inclusionPattern; + this.exclusionPattern = exclusionPattern; + } + + // ********************************************************************** + // Implementation of abstract methods from TreePattern + // ********************************************************************** + + @Override + public String getPattern() { + return "INCLUSION(" + + inclusionPattern.getPattern() + + "), EXCLUSION(" + + exclusionPattern.getPattern() + + ")"; + } + + @Override + public boolean isRoot() { + return inclusionPattern.isRoot() && !exclusionPattern.isRoot(); + } + + @Override + public boolean isLegal() { + return inclusionPattern.isLegal() && exclusionPattern.isLegal(); + } + + @Override + public boolean coversDb(final String db) { + return inclusionPattern.coversDb(db) && !exclusionPattern.mayOverlapWithDb(db); + } + + @Override + public boolean coversDevice(final IDeviceID device) { + return inclusionPattern.coversDevice(device) && !exclusionPattern.mayOverlapWithDevice(device); + } + + @Override + public boolean mayOverlapWithDb(final String db) { + return inclusionPattern.mayOverlapWithDb(db) && !exclusionPattern.coversDb(db); + } + + @Override + public boolean mayOverlapWithDevice(final IDeviceID device) { + return inclusionPattern.mayOverlapWithDevice(device) && !exclusionPattern.coversDevice(device); + } + + @Override + public boolean matchesMeasurement(final IDeviceID device, final String measurement) { + return inclusionPattern.matchesMeasurement(device, measurement) + && !exclusionPattern.matchesMeasurement(device, measurement); + } + + // ********************************************************************** + // Implementation of abstract methods from IoTDBPatternOperations + // ********************************************************************** + + @Override + public boolean matchPrefixPath(final String path) { + return inclusionPattern.matchPrefixPath(path) && !exclusionPattern.matchPrefixPath(path); + } + + @Override + public boolean matchDevice(final String devicePath) { + return inclusionPattern.matchDevice(devicePath) && !exclusionPattern.matchDevice(devicePath); + } + + @Override + public boolean matchTailNode(final String tailNode) { + return inclusionPattern.matchTailNode(tailNode) && !exclusionPattern.matchTailNode(tailNode); + } + + @Override + public List<PartialPath> getIntersection(final PartialPath partialPath) { + // 1. Calculate Intersection(Input, Inclusion) + final List<PartialPath> inclusionIntersections = inclusionPattern.getIntersection(partialPath); + if (inclusionIntersections.isEmpty()) { + return Collections.emptyList(); + } + + // 2. Calculate Intersection(Input, Exclusion) + final List<PartialPath> exclusionIntersections = exclusionPattern.getIntersection(partialPath); + if (exclusionIntersections.isEmpty()) { + // Optimization: No exclusion intersection, return inclusion intersection directly + return inclusionIntersections; + } + + // 3. Perform the "Subtraction" + // Filter out paths from inclusionIntersections that are fully covered by any path + // in exclusionIntersections. + return inclusionIntersections.stream() + .filter( + incPath -> + exclusionIntersections.stream().noneMatch(excPath -> excPath.include(incPath))) Review Comment: The nested stream operations create an O(n*m) complexity where n is the size of inclusionIntersections and m is the size of exclusionIntersections. For large pattern sets, this could be inefficient. Consider converting exclusionIntersections to a Set or using a more efficient data structure for the containment check. ```suggestion // Optimization: Avoid nested streams by marking excluded paths boolean[] isExcluded = new boolean[inclusionIntersections.size()]; for (PartialPath excPath : exclusionIntersections) { for (int i = 0; i < inclusionIntersections.size(); i++) { if (!isExcluded[i] && excPath.include(inclusionIntersections.get(i))) { isExcluded[i] = true; } } } return java.util.stream.IntStream.range(0, inclusionIntersections.size()) .filter(i -> !isExcluded[i]) .mapToObj(inclusionIntersections::get) ``` ########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java: ########## @@ -75,9 +121,165 @@ public static TreePattern parsePipePatternFromSourceParameters( 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. 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( + 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); + + // 3. Parse EXCLUSION patterns using the helper + final TreePattern exclusionPattern = + parsePatternUnion( + 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 IoTDBPatternOperations + && exclusionPattern instanceof IoTDBPatternOperations) { + return new ExclusionIoTDBTreePattern( + isTreeModelDataAllowedToBeCaptured, + (IoTDBPatternOperations) inclusionPattern, + (IoTDBPatternOperations) exclusionPattern); + } + // Both are defined, wrap them in an ExclusionTreePattern + return new ExclusionTreePattern( + isTreeModelDataAllowedToBeCaptured, inclusionPattern, exclusionPattern); + } + } + + /** + * The main entry point for parsing a pattern string. This method can handle simple patterns + * ("root.a"), union patterns ("root.a,root.b"), and exclusion patterns ("INCLUSION(root.a), + * EXCLUSION(root.b)"). + */ + public static TreePattern parsePatternFromString( + final String patternString, + final boolean isTreeModelDataAllowedToBeCaptured, + final Function<String, TreePattern> basePatternSupplier) { + final String trimmedPattern = (patternString == null) ? "" : patternString.trim(); + if (trimmedPattern.isEmpty()) { + return basePatternSupplier.apply(""); + } + + // 1. Check if it's an Exclusion pattern + if (trimmedPattern.startsWith("INCLUSION(") && trimmedPattern.endsWith(")")) { Review Comment: The parsing logic for exclusion patterns uses hardcoded string literals "INCLUSION(" and ", EXCLUSION(". These should be extracted as named constants to improve maintainability and reduce the risk of typos. ########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/ExclusionIoTDBTreePattern.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.commons.pipe.datastructure.pattern; + +import org.apache.iotdb.commons.path.PartialPath; +import org.apache.iotdb.commons.path.PathPatternTree; + +import org.apache.tsfile.file.metadata.IDeviceID; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Represents an exclusion pattern specifically for IoTDB-operation-aware patterns. It holds an + * inclusion and exclusion pattern, both implementing {@link IoTDBPatternOperations}. + * + * <p>The logic is: "Matches inclusion AND NOT exclusion" for all methods. + */ +public class ExclusionIoTDBTreePattern extends TreePattern implements IoTDBPatternOperations { + + private final IoTDBPatternOperations inclusionPattern; + private final IoTDBPatternOperations exclusionPattern; + + public ExclusionIoTDBTreePattern( + final boolean isTreeModelDataAllowedToBeCaptured, + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(isTreeModelDataAllowedToBeCaptured); + this.inclusionPattern = inclusionPattern; + this.exclusionPattern = exclusionPattern; + } + + public ExclusionIoTDBTreePattern( + final IoTDBPatternOperations inclusionPattern, + final IoTDBPatternOperations exclusionPattern) { + super(true); + this.inclusionPattern = inclusionPattern; + this.exclusionPattern = exclusionPattern; + } + + // ********************************************************************** + // Implementation of abstract methods from TreePattern + // ********************************************************************** + + @Override + public String getPattern() { + return "INCLUSION(" + + inclusionPattern.getPattern() + + "), EXCLUSION(" + + exclusionPattern.getPattern() + + ")"; + } + + @Override + public boolean isRoot() { + return inclusionPattern.isRoot() && !exclusionPattern.isRoot(); + } + + @Override + public boolean isLegal() { + return inclusionPattern.isLegal() && exclusionPattern.isLegal(); + } + + @Override + public boolean coversDb(final String db) { + return inclusionPattern.coversDb(db) && !exclusionPattern.mayOverlapWithDb(db); + } + + @Override + public boolean coversDevice(final IDeviceID device) { + return inclusionPattern.coversDevice(device) && !exclusionPattern.mayOverlapWithDevice(device); + } + + @Override + public boolean mayOverlapWithDb(final String db) { + return inclusionPattern.mayOverlapWithDb(db) && !exclusionPattern.coversDb(db); + } + + @Override + public boolean mayOverlapWithDevice(final IDeviceID device) { + return inclusionPattern.mayOverlapWithDevice(device) && !exclusionPattern.coversDevice(device); + } + + @Override + public boolean matchesMeasurement(final IDeviceID device, final String measurement) { + return inclusionPattern.matchesMeasurement(device, measurement) + && !exclusionPattern.matchesMeasurement(device, measurement); + } + + // ********************************************************************** + // Implementation of abstract methods from IoTDBPatternOperations + // ********************************************************************** + + @Override + public boolean matchPrefixPath(final String path) { + return inclusionPattern.matchPrefixPath(path) && !exclusionPattern.matchPrefixPath(path); + } + + @Override + public boolean matchDevice(final String devicePath) { + return inclusionPattern.matchDevice(devicePath) && !exclusionPattern.matchDevice(devicePath); + } + + @Override + public boolean matchTailNode(final String tailNode) { + return inclusionPattern.matchTailNode(tailNode) && !exclusionPattern.matchTailNode(tailNode); + } + + @Override + public List<PartialPath> getIntersection(final PartialPath partialPath) { + // 1. Calculate Intersection(Input, Inclusion) + final List<PartialPath> inclusionIntersections = inclusionPattern.getIntersection(partialPath); + if (inclusionIntersections.isEmpty()) { + return Collections.emptyList(); + } + + // 2. Calculate Intersection(Input, Exclusion) + final List<PartialPath> exclusionIntersections = exclusionPattern.getIntersection(partialPath); + if (exclusionIntersections.isEmpty()) { + // Optimization: No exclusion intersection, return inclusion intersection directly + return inclusionIntersections; + } + + // 3. Perform the "Subtraction" + // Filter out paths from inclusionIntersections that are fully covered by any path + // in exclusionIntersections. + return inclusionIntersections.stream() + .filter( + incPath -> + exclusionIntersections.stream().noneMatch(excPath -> excPath.include(incPath))) + .collect(Collectors.toList()); + } + + @Override + public PathPatternTree getIntersection(final PathPatternTree patternTree) { + // 1. Calculate Intersection(Input, Inclusion) + final PathPatternTree inclusionIntersectionTree = inclusionPattern.getIntersection(patternTree); + if (inclusionIntersectionTree.isEmpty()) { + return inclusionIntersectionTree; // Return empty tree + } + + // 2. Calculate Intersection(Input, Exclusion) + final PathPatternTree exclusionIntersectionTree = exclusionPattern.getIntersection(patternTree); + if (exclusionIntersectionTree.isEmpty()) { + // Optimization: No exclusion intersection, return inclusion intersection directly + return inclusionIntersectionTree; + } + + // 3. Perform the "Subtraction" + final List<PartialPath> inclusionPaths = inclusionIntersectionTree.getAllPathPatterns(); + final List<PartialPath> exclusionPaths = exclusionIntersectionTree.getAllPathPatterns(); + + final PathPatternTree finalResultTree = new PathPatternTree(); + for (final PartialPath incPath : inclusionPaths) { + // Check if the current inclusion path is covered by *any* exclusion path pattern + boolean excluded = exclusionPaths.stream().anyMatch(excPath -> excPath.include(incPath)); + + if (!excluded) { + finalResultTree.appendPathPattern(incPath); // Add non-excluded path to the result tree + } + } Review Comment: Similar to the previous getIntersection method, this has O(n*m) complexity. The stream is recreated for each inclusion path. Consider optimizing by converting exclusionPaths to a more efficient lookup structure if the list is large. -- 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]
