lszskye commented on code in PR #432:
URL: https://github.com/apache/paimon-cpp/pull/432#discussion_r4322419734


##########
src/paimon/common/global_index/btree/lazy_filtered_btree_reader.cpp:
##########
@@ -31,38 +31,211 @@
 #include "paimon/common/sst/block_cache.h"
 #include "paimon/common/sst/sst_file_reader.h"
 #include "paimon/common/utils/crc32c.h"
+#include "paimon/global_index/bitmap_global_index_result.h"
 #include "paimon/io/buffered_input_stream.h"
 #include "paimon/utils/roaring_bitmap64.h"
 
 namespace paimon {
+namespace {
+
+using FullRangeBounds = std::optional<std::pair<Literal, Literal>>;
+
+Result<FullRangeBounds> GetFullRangeBounds(const 
std::vector<GlobalIndexIOMeta>& files,
+                                           int64_t total_row_count,
+                                           const 
std::shared_ptr<KeySerializer>& key_serializer,
+                                           MemoryPool* pool) {
+    if (total_row_count <= 0 || files.empty()) {
+        return std::optional<std::pair<Literal, Literal>>();
+    }
+    int64_t remaining = total_row_count;
+    std::optional<Literal> min_key;
+    std::optional<Literal> max_key;
+    for (const GlobalIndexIOMeta& file : files) {
+        // A file's min/max does not identify its row IDs. Only skip the 
entire reader when its
+        // scalar entries cover the complete local row-ID domain without gaps.
+        if (file.row_count < 0 || file.row_count > remaining) {
+            return std::optional<std::pair<Literal, Literal>>();
+        }
+        remaining -= file.row_count;
+        if (file.metadata == nullptr) {
+            return std::optional<std::pair<Literal, Literal>>();
+        }
+        PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<SortedIndexFileMeta> meta,
+                               SortedIndexFileMeta::Deserialize(file.metadata, 
pool));
+        if (meta->HasNulls() || meta->FirstKey() == nullptr || meta->LastKey() 
== nullptr) {
+            return std::optional<std::pair<Literal, Literal>>();
+        }
+        PAIMON_ASSIGN_OR_RAISE(Literal first,
+                               
key_serializer->Deserialize(MemorySlice::Wrap(meta->FirstKey())));
+        PAIMON_ASSIGN_OR_RAISE(Literal last,
+                               
key_serializer->Deserialize(MemorySlice::Wrap(meta->LastKey())));
+        if (!min_key) {
+            min_key = first;
+        } else {
+            PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
first.CompareTo(min_key.value()));
+            if (compare < 0) {
+                min_key = first;
+            }
+        }
+        if (!max_key) {
+            max_key = last;
+        } else {
+            PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
last.CompareTo(max_key.value()));
+            if (compare > 0) {
+                max_key = last;
+            }
+        }
+    }
+    if (remaining != 0) {
+        return std::optional<std::pair<Literal, Literal>>();
+    }
+    return std::optional<std::pair<Literal, Literal>>(
+        std::make_pair(std::move(min_key.value()), 
std::move(max_key.value())));
+}
+
+std::optional<RoaringBitmap64> CreateRowIdFilter(
+    const std::optional<std::vector<Range>>& row_ranges) {
+    if (!row_ranges) {
+        return std::nullopt;
+    }
+    RoaringBitmap64 filter;
+    for (const Range& range : row_ranges.value()) {
+        filter.AddRange(range.from, range.to + 1);
+    }
+    return filter;
+}
+
+}  // namespace
 
 Result<std::shared_ptr<LazyFilteredBTreeReader>> 
LazyFilteredBTreeReader::Create(
     std::optional<int32_t> read_buffer_size, const 
std::vector<GlobalIndexIOMeta>& files,
     const std::shared_ptr<arrow::DataType>& key_type,
     const std::shared_ptr<GlobalIndexFileReader>& file_reader,
     const std::shared_ptr<CacheManager>& cache_manager, int64_t 
fallback_scan_max_size,
+    int64_t total_row_count, const std::optional<std::vector<Range>>& 
row_ranges,
     const std::shared_ptr<MemoryPool>& pool, const std::shared_ptr<Executor>& 
executor) {
+    if (total_row_count < 0) {
+        return Status::Invalid("Total row count must be non-negative.");
+    }
     PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<KeySerializer> key_serializer,
                            KeySerializer::Create(key_type, pool));
     PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<SortedFileMetaSelector> 
file_selector,
                            SortedFileMetaSelector::Create(files, 
key_serializer));
+    PAIMON_ASSIGN_OR_RAISE(FullRangeBounds full_range_bounds,
+                           GetFullRangeBounds(files, total_row_count, 
key_serializer, pool.get()));
     return std::shared_ptr<LazyFilteredBTreeReader>(new 
LazyFilteredBTreeReader(
         read_buffer_size, std::move(file_selector), key_serializer, 
file_reader, cache_manager,
-        fallback_scan_max_size, pool, executor));
+        fallback_scan_max_size, total_row_count, std::move(full_range_bounds),
+        CreateRowIdFilter(row_ranges), pool, executor));
 }
 
 LazyFilteredBTreeReader::LazyFilteredBTreeReader(
     std::optional<int32_t> read_buffer_size, 
std::unique_ptr<SortedFileMetaSelector> file_selector,
     const std::shared_ptr<KeySerializer>& key_serializer,
     const std::shared_ptr<GlobalIndexFileReader>& file_reader,
     const std::shared_ptr<CacheManager>& cache_manager, int64_t 
fallback_scan_max_size,
-    const std::shared_ptr<MemoryPool>& pool, const std::shared_ptr<Executor>& 
executor)
-    : SortedFileGlobalIndexReader(std::move(file_selector), 
fallback_scan_max_size, executor),
+    int64_t total_row_count, std::optional<std::pair<Literal, Literal>> 
full_range_bounds,
+    std::optional<RoaringBitmap64> row_id_filter, const 
std::shared_ptr<MemoryPool>& pool,
+    const std::shared_ptr<Executor>& executor)
+    : SortedFileGlobalIndexReader(std::move(file_selector), 
fallback_scan_max_size, total_row_count,
+                                  executor),
       read_buffer_size_(read_buffer_size),
       pool_(pool),
       key_serializer_(key_serializer),
       file_reader_(file_reader),
-      cache_manager_(cache_manager) {}
+      cache_manager_(cache_manager),
+      total_row_count_(total_row_count),
+      full_range_bounds_(std::move(full_range_bounds)),
+      row_id_filter_(std::move(row_id_filter)) {}
+
+Result<std::shared_ptr<GlobalIndexResult>> LazyFilteredBTreeReader::VisitEqual(
+    const Literal& literal) {
+    PAIMON_ASSIGN_OR_RAISE(bool all_match,
+                           AllKeysMatch([&literal](const Literal& key) -> 
Result<bool> {
+                               if (literal.IsNull()) {
+                                   return false;
+                               }
+                               PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
key.CompareTo(literal));
+                               return compare == 0;
+                           }));
+    return all_match ? FullResult() : 
SortedFileGlobalIndexReader::VisitEqual(literal);
+}
+
+Result<std::shared_ptr<GlobalIndexResult>> 
LazyFilteredBTreeReader::VisitLessThan(
+    const Literal& literal) {
+    PAIMON_ASSIGN_OR_RAISE(bool all_match,
+                           AllKeysMatch([&literal](const Literal& key) -> 
Result<bool> {
+                               if (literal.IsNull()) {
+                                   return false;
+                               }
+                               PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
key.CompareTo(literal));
+                               return compare < 0;
+                           }));
+    return all_match ? FullResult() : 
SortedFileGlobalIndexReader::VisitLessThan(literal);
+}
+
+Result<std::shared_ptr<GlobalIndexResult>> 
LazyFilteredBTreeReader::VisitLessOrEqual(
+    const Literal& literal) {
+    PAIMON_ASSIGN_OR_RAISE(bool all_match,
+                           AllKeysMatch([&literal](const Literal& key) -> 
Result<bool> {
+                               if (literal.IsNull()) {
+                                   return false;
+                               }
+                               PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
key.CompareTo(literal));
+                               return compare <= 0;
+                           }));
+    return all_match ? FullResult() : 
SortedFileGlobalIndexReader::VisitLessOrEqual(literal);
+}
+
+Result<std::shared_ptr<GlobalIndexResult>> 
LazyFilteredBTreeReader::VisitGreaterThan(
+    const Literal& literal) {
+    PAIMON_ASSIGN_OR_RAISE(bool all_match,
+                           AllKeysMatch([&literal](const Literal& key) -> 
Result<bool> {
+                               if (literal.IsNull()) {
+                                   return false;
+                               }
+                               PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
key.CompareTo(literal));
+                               return compare > 0;
+                           }));
+    return all_match ? FullResult() : 
SortedFileGlobalIndexReader::VisitGreaterThan(literal);
+}
+
+Result<std::shared_ptr<GlobalIndexResult>> 
LazyFilteredBTreeReader::VisitGreaterOrEqual(
+    const Literal& literal) {
+    PAIMON_ASSIGN_OR_RAISE(bool all_match,
+                           AllKeysMatch([&literal](const Literal& key) -> 
Result<bool> {
+                               if (literal.IsNull()) {
+                                   return false;
+                               }
+                               PAIMON_ASSIGN_OR_RAISE(int32_t compare, 
key.CompareTo(literal));
+                               return compare >= 0;
+                           }));
+    return all_match ? FullResult() : 
SortedFileGlobalIndexReader::VisitGreaterOrEqual(literal);
+}
+
+Result<std::shared_ptr<GlobalIndexResult>> 
LazyFilteredBTreeReader::FullResult() const {
+    if (total_row_count_ <= 0) {

Review Comment:
   This branch appears unreachable through the current call paths: Create() 
rejects negative row counts, and a zero row count leaves full_range_bounds_ 
unset, causing AllKeysMatch() to return false and preventing calls to 
FullResult().



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