Caideyipi commented on PR #17788: URL: https://github.com/apache/iotdb/pull/17788#issuecomment-4598589664
Review notes: 1. `QueryModificationLoader.java:115` uses `TsFileResource.getTotalModSizeInByte()` as the quota pre-check before trying to cache mods. However, `TsFileResource.getTotalModSizeInByte()` currently counts the full shared mods file length (`TsFileResource.java:453-455`), while the actual iterator only reads shared mods from `sharedModFileOffset` (`TsFileResource.java:1515-1516`). For TsFiles using shared mods, this can make the pre-check fail just because the shared file has grown globally, even when the unread portion for this TsFile is small. That means cache loading will be bypassed and the query will repeatedly fall back to scan. Please consider using the actual readable mods size here, e.g. `exclusiveModFileLength + max(0, sharedModFileLength - sharedModFileOffset)`. 2. `FragmentInstanceContext.java:413` uses `modification -> modification.affects(deviceID)` as the fallback matcher for the device-only overload. This is broader than the cache path (`mods -> getPathModifications(mods, deviceID)`), which matches `deviceID + AlignedPath.VECTOR_PLACEHOLDER`. For example, a deletion on `root.sg.d1.s1` affects device `root.sg.d1`, but it does not match the time-column/vector-placeholder path. So cache success and fallback scan can return different results for the same API. Even if this overload is not on the hot path today, the fallback predicate should be equivalent to the pattern-tree matcher. -- 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]
