lxy-9602 commented on code in PR #431:
URL: https://github.com/apache/paimon-cpp/pull/431#discussion_r4311134591
##########
src/paimon/common/global_index/union_global_index_reader.h:
##########
@@ -35,8 +37,15 @@ namespace paimon {
/// collected in submission order. Otherwise, sub-readers are evaluated
sequentially.
class UnionGlobalIndexReader : public GlobalIndexReader {
public:
+ /// Constructs a UnionGlobalIndexReader.
+ /// @param row_ranges The global row ids covered by each reader, in the
order of `readers`.
+ /// @param search_ranges The global row ids each reader ranks in a
full-text search, within its
+ /// row range, in the order of `readers`;
std::nullopt if it ranks none.
+ /// If empty, each reader ranks all of its rows.
UnionGlobalIndexReader(std::vector<std::shared_ptr<GlobalIndexReader>>&&
readers,
- const std::shared_ptr<Executor>& executor);
+ const std::shared_ptr<Executor>& executor,
+ std::vector<Range>&& row_ranges = {},
+ std::vector<std::optional<Range>>&& search_ranges =
{});
Review Comment:
Please don’t add default function parameters in the production code. Also,
for newly added parameters, could you please place them before `executor`.
##########
src/paimon/common/global_index/union_global_index_reader.cpp:
##########
@@ -133,9 +216,74 @@ Result<std::shared_ptr<ScoredGlobalIndexResult>>
UnionGlobalIndexReader::VisitVe
Result<std::shared_ptr<ScoredGlobalIndexResult>>
UnionGlobalIndexReader::VisitFullTextSearch(
const std::shared_ptr<FullTextSearch>& full_text_search) {
- return UnionScored([full_text_search](const
std::shared_ptr<GlobalIndexReader>& reader) {
- return reader->VisitFullTextSearch(full_text_search);
- });
+ if (!full_text_search) {
+ return Status::Invalid("VisitFullTextSearch: null FullTextSearch
pointer");
+ }
+ if (full_text_search->limit <= 0) {
+ return Status::Invalid("full-text search requires a positive limit");
+ }
+ // If the readers rank disjoint rows, the top `limit` rows of each reader
include all of its
+ // rows scoring above the `limit`-th highest score of all readers, so
merging them loses none
+ // of those rows. Rows tied at that score may already have been cut by a
reader's engine.
+ auto reader_searches =
std::make_shared<std::vector<std::shared_ptr<FullTextSearch>>>();
+ reader_searches->reserve(readers_.size());
+ for (size_t index = 0; index < readers_.size(); ++index) {
+ reader_searches->push_back(FullTextSearchForReader(index,
full_text_search));
+ }
+ auto results =
ExecuteAllReadersWithIndex<Result<std::shared_ptr<ScoredGlobalIndexResult>>>(
+ [reader_searches](size_t index, const
std::shared_ptr<GlobalIndexReader>& reader)
+ -> Result<std::shared_ptr<ScoredGlobalIndexResult>> {
+ const std::shared_ptr<FullTextSearch>& reader_search =
(*reader_searches)[index];
+ if (!reader_search) {
+ return EmptyScoredResult();
+ }
+ return reader->VisitFullTextSearch(reader_search);
+ });
+ std::vector<std::shared_ptr<ScoredGlobalIndexResult>> scored_results;
+ scored_results.reserve(results.size());
+ for (auto& result_or_status : results) {
+ PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<ScoredGlobalIndexResult> result,
+ std::move(result_or_status));
+ if (result != nullptr) {
+ scored_results.push_back(std::move(result));
+ }
+ }
+ if (scored_results.empty()) {
+ return std::shared_ptr<ScoredGlobalIndexResult>();
+ }
Review Comment:
I’m a bit curious here: when `scored_results.empty()` happens, Java seems to
return `EmptyScoredResult`, but the current code returns `nullptr`. Returning
`nullptr` indicate that the index wasn’t actually used, or that we can’t
determine the outcome.
--
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]
