shuwenwei commented on code in PR #284:
URL: https://github.com/apache/tsfile/pull/284#discussion_r1830448159
##########
java/tsfile/src/main/java/org/apache/tsfile/read/reader/series/FileSeriesReader.java:
##########
@@ -56,8 +56,10 @@ protected void initChunkReader(IChunkMetadata chunkMetaData)
throws IOException
chunkLoader.loadChunk((ChunkMetadata)
(alignedChunkMetadata.getTimeChunkMetadata()));
List<Chunk> valueChunkList = new ArrayList<>();
for (IChunkMetadata metadata :
alignedChunkMetadata.getValueChunkMetadataList()) {
- valueChunkList.add(chunkLoader.loadChunk((ChunkMetadata) metadata));
- currentChunkMeasurementNames.add(metadata.getMeasurementUid());
+ if (metadata != null) {
Review Comment:
> > If there are value columns (s1, s2, s3), in the first
AlignedChunkMetadata, s2 corresponds to metadata as null, at this time the
returned valueChunkList is (s1, s3). In the subsequent AlignedChunkMetadata, s3
corresponds to metadata as null, at this time the returned valueChunkList is
(s1, s2). Perhaps the two returns should be (s1, null, s3) (s1, s2, null)
>
> Does `currentChunkMeasurementNames` already solve this problem well?
Yes, I checked the usage of this class, and in SingleDeviceTsBlockReader,
the issue is resolved by using getCurrentChunkMeasurementNames method. Another
usage of this class is in TsFileReader, where the QueryExpression is conducted
based on the input Path. It seems that even if each Path is from the same
aligned series of value columns, it still proceeds in multiple FileSeriesReader
instances, hence the AlignedChunkMetadata here may only have one value column's
chunk metadata, so it also seems to be correct.
<img width="1229" alt="截屏2024-11-06 14 22 50"
src="https://github.com/user-attachments/assets/7aaf4d89-9626-4d7f-8521-7ffdf50bbba5";>
--
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]
