Copilot commented on code in PR #16632: URL: https://github.com/apache/iotdb/pull/16632#discussion_r2470157505
########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/ExclusionIoTDBTreePattern.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.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; + } + + // ********************************************************************** + // 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) { + // NOTE: This is a simple set-difference, which is semantically correct + // ONLY IF partialPath does NOT contain wildcards. + // A true intersection of (A AND NOT B) with C (where C has wildcards) + // is far more complex and may not be representable as a List<PartialPath>. Review Comment: The comment warns about limitations when `partialPath` contains wildcards but the method does not validate or throw an exception in such cases. Consider either adding runtime validation to reject wildcard patterns with a clear error message, or document the undefined behavior more explicitly in the method's JavaDoc. ```suggestion /** * Returns the intersection of this pattern with the given {@link PartialPath}. * <p> * <b>Note:</b> This method only supports {@code partialPath} values that do <i>not</i> contain wildcards. * If wildcards are present, an {@link IllegalArgumentException} is thrown. * A true intersection of (A AND NOT B) with C (where C has wildcards) * is far more complex and may not be representable as a List<PartialPath>. * * @param partialPath the path to intersect with * @return the intersection as a list of {@link PartialPath} * @throws IllegalArgumentException if {@code partialPath} contains wildcards */ public List<PartialPath> getIntersection(final PartialPath partialPath) { if (partialPath.containsWildcard()) { throw new IllegalArgumentException( "getIntersection(PartialPath) does not support PartialPath containing wildcards: " + partialPath); } ``` ########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/ExclusionIoTDBTreePattern.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.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; + } + + // ********************************************************************** + // 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) { + // NOTE: This is a simple set-difference, which is semantically correct + // ONLY IF partialPath does NOT contain wildcards. + // A true intersection of (A AND NOT B) with C (where C has wildcards) + // is far more complex and may not be representable as a List<PartialPath>. + final List<PartialPath> inclusionPaths = inclusionPattern.getIntersection(partialPath); + if (inclusionPaths.isEmpty()) { + return inclusionPaths; + } + final List<PartialPath> exclusionPaths = exclusionPattern.getIntersection(partialPath); + if (exclusionPaths.isEmpty()) { + return inclusionPaths; + } + + // Return (inclusionPaths - exclusionPaths) + return inclusionPaths.stream() + .filter(p -> !exclusionPaths.contains(p)) + .collect(Collectors.toList()); + } + + @Override + public PathPatternTree getIntersection(final PathPatternTree patternTree) { + // A true set difference (A.intersect(B) - C.intersect(B)) + // would require a PathPatternTree.subtract() method, which does not exist. + // This operation is unsupported. + // TODO Review Comment: The TODO comment at line 144 lacks context about what needs to be implemented and why. Consider replacing this with a more descriptive comment explaining what would be needed to support this operation (e.g., 'TODO: Implement PathPatternTree.subtract() method to enable set difference operations') or creating a tracking issue and referencing it here. ```suggestion // TODO: Implement PathPatternTree.subtract() to support set difference operations in getIntersection(PathPatternTree). ``` ########## iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java: ########## @@ -214,6 +414,28 @@ public static TreePattern buildUnionPattern( } } + /** Helper method to find the matching closing parenthesis, respecting backticks. */ Review Comment: The `findMatchingParenthesis` helper method should include JavaDoc documentation explaining its algorithm for handling nested parentheses and backtick-escaped content. This is particularly important given its role in parsing the INCLUSION/EXCLUSION syntax. ```suggestion /** * Finds the index of the matching closing parenthesis for a given opening parenthesis in the input string, * correctly handling nested parentheses and ignoring parentheses that appear within backtick-escaped content. * <p> * This method is used when parsing INCLUSION/EXCLUSION syntax, where patterns may contain nested groups * and segments that are escaped using backticks. The algorithm works as follows: * <ul> * <li>Starts at the character immediately after the given opening parenthesis index.</li> * <li>Keeps track of the current parenthesis nesting depth (initially 1).</li> * <li>Toggles an {@code inBackticks} flag whenever a backtick character ({@code `}) is encountered.</li> * <li>When not inside backticks: * <ul> * <li>Increments depth for each opening parenthesis ({@code (}).</li> * <li>Decrements depth for each closing parenthesis ({@code )}).</li> * </ul> * </li> * <li>When depth reaches zero, returns the current index as the matching closing parenthesis.</li> * <li>If the end of the string is reached without finding a match, returns -1.</li> * </ul> * <p> * Parentheses inside backtick-escaped segments are ignored for the purposes of matching. * * @param text the input string containing parentheses and possibly backtick-escaped segments * @param openParenIndex the index of the opening parenthesis to match * @return the index of the matching closing parenthesis, or -1 if not found */ ``` ########## 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) { Review Comment: The method `parsePatternFromString` lacks documentation explaining its purpose, parameters, and the expected format of the pattern string (e.g., 'INCLUSION(...), EXCLUSION(...)'). Add a comprehensive JavaDoc comment describing the method's behavior, parameter expectations, and return value. -- 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]
