lszskye commented on code in PR #394: URL: https://github.com/apache/paimon-cpp/pull/394#discussion_r4237332965
########## src/paimon/common/global_index/sorted_file_global_index_reader.cpp: ########## @@ -0,0 +1,273 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "paimon/common/global_index/sorted_file_global_index_reader.h" + +#include <limits> +#include <optional> +#include <utility> + +#include "paimon/common/global_index/union_global_index_reader.h" +#include "paimon/common/predicate/like_optimization.h" +#include "paimon/global_index/bitmap_global_index_result.h" +#include "paimon/utils/roaring_bitmap64.h" + +namespace paimon { +namespace { + +std::shared_ptr<GlobalIndexResult> EmptyResult() { + return std::make_shared<BitmapGlobalIndexResult>([]() { return RoaringBitmap64(); }); +} + +} // namespace + +SortedFileGlobalIndexReader::SortedFileGlobalIndexReader( + std::unique_ptr<SortedFileMetaSelector> file_selector, int64_t fallback_scan_max_size, + std::shared_ptr<Executor> executor) + : file_selector_(std::move(file_selector)), + fallback_scan_max_size_(fallback_scan_max_size), + executor_(std::move(executor)) {} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitIsNotNull() { + return VisitParallel( + [this]() { return file_selector_->VisitIsNotNull(); }, + [](const std::shared_ptr<GlobalIndexReader>& reader) { return reader->VisitIsNotNull(); }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitIsNull() { + return VisitParallel( + [this]() { return file_selector_->VisitIsNull(); }, + [](const std::shared_ptr<GlobalIndexReader>& reader) { return reader->VisitIsNull(); }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitEqual( + const Literal& literal) { + return VisitParallel([this, &literal]() { return file_selector_->VisitEqual(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitEqual(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitNotEqual( + const Literal& literal) { + return VisitParallel([this, &literal]() { return file_selector_->VisitNotEqual(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitNotEqual(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitLessThan( + const Literal& literal) { + if (literal.IsNull() || fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &literal]() { return file_selector_->VisitLessThan(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitLessThan(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitLessOrEqual( + const Literal& literal) { + if (literal.IsNull() || fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &literal]() { return file_selector_->VisitLessOrEqual(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitLessOrEqual(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitGreaterThan( + const Literal& literal) { + if (literal.IsNull() || fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &literal]() { return file_selector_->VisitGreaterThan(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitGreaterThan(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitGreaterOrEqual( + const Literal& literal) { + if (literal.IsNull() || fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &literal]() { return file_selector_->VisitGreaterOrEqual(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitGreaterOrEqual(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitIn( + const std::vector<Literal>& literals) { + return VisitParallel([this, &literals]() { return file_selector_->VisitIn(literals); }, + [&literals](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitIn(literals); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitNotIn( + const std::vector<Literal>& literals) { + return VisitParallel([this, &literals]() { return file_selector_->VisitNotIn(literals); }, + [&literals](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitNotIn(literals); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitStartsWith( + const Literal& prefix) { + if (prefix.IsNull() || prefix.GetType() != FieldType::STRING) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitParallel([this, &prefix]() { return file_selector_->VisitStartsWith(prefix); }, + [&prefix](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitStartsWith(prefix); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitEndsWith( + const Literal& suffix) { + if (suffix.IsNull() || suffix.GetType() != FieldType::STRING || fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &suffix]() { return file_selector_->VisitEndsWith(suffix); }, + [&suffix](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitEndsWith(suffix); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitContains( + const Literal& literal) { + if (literal.IsNull() || literal.GetType() != FieldType::STRING || + fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel( + [this, &literal]() { return file_selector_->VisitContains(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitContains(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitLike( + const Literal& literal) { + if (literal.IsNull() || literal.GetType() != FieldType::STRING) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + std::optional<OptimizedLike> optimized = LikeOptimization::TryOptimize(literal); + if (optimized.has_value()) { + switch (optimized->function->GetType()) { + case Function::Type::EQUAL: + return VisitEqual(optimized->literal); + case Function::Type::STARTS_WITH: + return VisitStartsWith(optimized->literal); + case Function::Type::ENDS_WITH: + return VisitEndsWith(optimized->literal); + case Function::Type::CONTAINS: + return VisitContains(optimized->literal); + default: + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + } + if (fallback_scan_max_size_ <= 0) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } + return VisitFallbackParallel([this, &literal]() { return file_selector_->VisitLike(literal); }, + [&literal](const std::shared_ptr<GlobalIndexReader>& reader) { + return reader->VisitLike(literal); + }); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitParallel( + SelectAction select_files, ReaderAction visitor) { + PAIMON_ASSIGN_OR_RAISE(std::vector<GlobalIndexIOMeta> selected_files, select_files()); + return VisitSelectedFiles(selected_files, std::move(visitor)); +} + +Result<std::shared_ptr<GlobalIndexResult>> SortedFileGlobalIndexReader::VisitFallbackParallel( + SelectAction select_files, ReaderAction visitor) { + PAIMON_ASSIGN_OR_RAISE(std::vector<GlobalIndexIOMeta> selected_files, select_files()); + if (selected_files.empty()) { + return EmptyResult(); + } + if (!FallbackScanEnabled(selected_files)) { + return std::shared_ptr<GlobalIndexResult>(nullptr); + } Review Comment: Returning nullptr here means that the index cannot evaluate the predicate within the fallback scan budget, not that there are no matching rows. However, UnionGlobalIndexReader::Union() currently skips nullptr results. Since GlobalIndexScanImpl unions readers across different row ranges, this can cause false negatives. For example, when querying >= "Y": - Range A contains [A, B] and returns an empty bitmap after metadata pruning. - Range B contains [Y, Z], but its selected index files exceed the scan budget, so it returns nullptr. The outer Union returns an empty bitmap, incorrectly excluding the matching rows in range B. -- 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]
