wangyong9999 commented on code in PR #314:
URL: https://github.com/apache/paimon-cpp/pull/314#discussion_r3996340952


##########
src/paimon/format/parquet/parquet_input_stream.h:
##########
@@ -78,21 +80,22 @@ class ParquetInputStream : public ArrowInputStreamAdapter {
     using ArrowInputStreamAdapter::ReadAt;
 
     arrow::Result<int64_t> ReadAt(int64_t position, int64_t nbytes, void* out) 
override {
-        if (!cache_ || file_uri_.empty() || nbytes <= 0 ||
-            nbytes > std::numeric_limits<int32_t>::max()) {
+        if (!cache_ || file_uri_.empty() || position < 0 || nbytes <= 0 || 
position > file_size_ ||
+            nbytes > file_size_ - position || nbytes > 
std::numeric_limits<int32_t>::max()) {
             return ArrowInputStreamAdapter::ReadAt(position, nbytes, out);
         }
+        bool is_index = false;
         auto range = index_ranges_.upper_bound(position);
-        if (range == index_ranges_.begin()) {
-            return ArrowInputStreamAdapter::ReadAt(position, nbytes, out);
+        if (range != index_ranges_.begin()) {
+            --range;
+            const int64_t offset = position - range->first;
+            is_index = offset <= range->second && nbytes <= range->second - 
offset;
         }
-        --range;
-        if (position - range->first > range->second ||
-            nbytes > range->second - (position - range->first)) {
+        if (!is_index && !cache_data_) {
             return ArrowInputStreamAdapter::ReadAt(position, nbytes, out);
         }
         auto key = CacheKey::ForKind(file_uri_, position, 
static_cast<int32_t>(nbytes),

Review Comment:
   Verified the final tree at 0bc1b4fb after preserving the concurrent review 
update. The exact-range layer and option are absent; only OffsetIndex 
memoization, page selection and direct-plan decisions remain. I also removed 
unused ColumnIndex memoization and avoided repeated binary searches for ranges 
within already visited pages. Parquet 226/226 and filesystem 64/64 pass, 
including the existing short-read regression. The revised description records 
the local main/candidate/candidate/main measurements, including regressions and 
the lack of a general speedup. Resolving the withdrawn-cache concern; 
cross-lifetime block backing remains explicitly outside this PR.



-- 
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]

Reply via email to