Copilot commented on code in PR #16632:
URL: https://github.com/apache/iotdb/pull/16632#discussion_r2446581183
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tablet/parser/TabletInsertionEventTreePatternParser.java:
##########
@@ -113,13 +128,33 @@ else if (pattern.mayOverlapWithDevice(deviceId)) {
continue;
}
- if (pattern.matchesMeasurement(deviceId, measurement)) {
+ if (pattern.matchesMeasurement(deviceId, measurement) &&
!excluded.test(measurement)) {
originColumnIndex2FilteredColumnIndexMapperList[i] = filteredCount++;
}
}
}
}
+ private boolean isMeasurementExcluded(
+ final org.apache.tsfile.file.metadata.IDeviceID device, final String
measurement) {
+ if (Objects.isNull(exclusionPatterns) || exclusionPatterns.isEmpty()) {
+ return false;
+ }
+ for (final TreePattern ex : exclusionPatterns) {
+ if (Objects.isNull(ex)) {
+ continue;
+ }
+ // If the exclusion covers the device, exclude all measurements
+ if (ex.coversDevice(device)) {
+ return true;
+ }
+ if (ex.mayOverlapWithDevice(device) && ex.matchesMeasurement(device,
measurement)) {
+ return true;
+ }
Review Comment:
Lines 148-150 duplicate the device coverage check that would be in
`isDeviceExcluded()`. This creates redundant logic across the codebase.
Consider extracting a separate `isDeviceExcluded()` method and calling it
first, or removing the device coverage check here to maintain consistency with
other parsers and reduce duplication.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tsfile/parser/scan/TsFileInsertionEventScanParser.java:
##########
@@ -556,6 +565,39 @@ private boolean recordAlignedChunk(final List<Chunk>
valueChunkList, final byte
return false;
}
+ private boolean isDeviceExcluded(final IDeviceID device) {
+ if (Objects.isNull(exclusionPatterns) || exclusionPatterns.isEmpty()) {
+ return false;
+ }
+ for (final TreePattern ex : exclusionPatterns) {
+ if (Objects.isNull(ex)) {
+ continue;
+ }
+ if (ex.coversDevice(device)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean isMeasurementExcluded(final IDeviceID device, final String
measurement) {
+ if (Objects.isNull(exclusionPatterns) || exclusionPatterns.isEmpty()) {
+ return false;
+ }
+ for (final TreePattern ex : exclusionPatterns) {
+ if (Objects.isNull(ex)) {
+ continue;
+ }
+ if (ex.coversDevice(device)) {
+ return true;
+ }
+ if (ex.mayOverlapWithDevice(device) && ex.matchesMeasurement(device,
measurement)) {
+ return true;
+ }
Review Comment:
The logic at lines 591-593 is duplicated from `isDeviceExcluded()`. If the
exclusion pattern covers the device, both methods should return true. This
check is redundant in `isMeasurementExcluded()` since if a device is fully
excluded, all its measurements should be excluded. Consider removing lines
591-593 or calling `isDeviceExcluded(device)` first to avoid duplicating this
logic.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tsfile/parser/query/TsFileInsertionEventQueryParser.java:
##########
@@ -391,6 +414,39 @@ public TabletInsertionEvent next() {
return tabletInsertionIterable;
}
+ private boolean isDeviceExcluded(final IDeviceID device) {
+ if (Objects.isNull(exclusionPatterns) || exclusionPatterns.isEmpty()) {
+ return false;
+ }
+ for (final TreePattern ex : exclusionPatterns) {
+ if (Objects.isNull(ex)) {
+ continue;
+ }
+ if (ex.coversDevice(device)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean isMeasurementExcluded(final IDeviceID device, final String
measurement) {
+ if (Objects.isNull(exclusionPatterns) || exclusionPatterns.isEmpty()) {
+ return false;
+ }
+ for (final TreePattern ex : exclusionPatterns) {
+ if (Objects.isNull(ex)) {
+ continue;
+ }
+ if (ex.coversDevice(device)) {
+ return true;
+ }
Review Comment:
The logic at lines 440-442 duplicates the check from `isDeviceExcluded()`.
If an exclusion pattern covers the entire device, all measurements should be
excluded. This redundant check in `isMeasurementExcluded()` can be eliminated
by calling `isDeviceExcluded(device)` first or removing these lines to avoid
code duplication.
```suggestion
if (isDeviceExcluded(device)) {
return true;
}
for (final TreePattern ex : exclusionPatterns) {
if (Objects.isNull(ex)) {
continue;
}
```
--
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]
