lxy-9602 commented on code in PR #431:
URL: https://github.com/apache/paimon-cpp/pull/431#discussion_r4290221086
##########
src/paimon/common/global_index/offset_global_index_reader.h:
##########
@@ -31,11 +31,22 @@ namespace paimon {
/// results. This is used to convert local row IDs into global row IDs.
class OffsetGlobalIndexReader : public GlobalIndexReader {
public:
- /// Constructs an OffsetGlobalIndexReader.
+ /// Constructs an OffsetGlobalIndexReader whose full-text searches rank
all rows it covers.
/// @param wrapped The inner reader to delegate queries to. Its results
are expected to
/// contain local row ids.
- /// @param offset The offset to add to each row id in the results.
- OffsetGlobalIndexReader(std::shared_ptr<GlobalIndexReader>&& wrapped,
int64_t offset);
+ /// @param offset The first global row id covered by the wrapped reader,
added to each row id
+ /// in the results.
+ /// @param to The last global row id (inclusive) covered by the
wrapped reader.
+ OffsetGlobalIndexReader(std::shared_ptr<GlobalIndexReader>&& wrapped,
int64_t offset,
+ int64_t to);
+
Review Comment:
It looks like the three-parameter version isn’t used in production code—can
we remove it?
##########
src/paimon/common/global_index/offset_global_index_reader.h:
##########
@@ -31,11 +31,22 @@ namespace paimon {
/// results. This is used to convert local row IDs into global row IDs.
class OffsetGlobalIndexReader : public GlobalIndexReader {
public:
- /// Constructs an OffsetGlobalIndexReader.
+ /// Constructs an OffsetGlobalIndexReader whose full-text searches rank
all rows it covers.
/// @param wrapped The inner reader to delegate queries to. Its results
are expected to
/// contain local row ids.
- /// @param offset The offset to add to each row id in the results.
- OffsetGlobalIndexReader(std::shared_ptr<GlobalIndexReader>&& wrapped,
int64_t offset);
+ /// @param offset The first global row id covered by the wrapped reader,
added to each row id
+ /// in the results.
+ /// @param to The last global row id (inclusive) covered by the
wrapped reader.
+ OffsetGlobalIndexReader(std::shared_ptr<GlobalIndexReader>&& wrapped,
int64_t offset,
+ int64_t to);
+
Review Comment:
I checked Java’s `chooseIndexRanges`—it seems more complex than the C++
side, and it passes `includeRowIds` rather than using
`OffsetGlobalIndexReader`. I wonder what’s the rationale behind this design?
##########
src/paimon/common/global_index/offset_global_index_reader.h:
##########
@@ -31,11 +31,22 @@ namespace paimon {
/// results. This is used to convert local row IDs into global row IDs.
class OffsetGlobalIndexReader : public GlobalIndexReader {
public:
- /// Constructs an OffsetGlobalIndexReader.
+ /// Constructs an OffsetGlobalIndexReader whose full-text searches rank
all rows it covers.
/// @param wrapped The inner reader to delegate queries to. Its results
are expected to
/// contain local row ids.
- /// @param offset The offset to add to each row id in the results.
- OffsetGlobalIndexReader(std::shared_ptr<GlobalIndexReader>&& wrapped,
int64_t offset);
Review Comment:
The comment for `OffsetGlobalIndexReader` seems to fully describe full-text
search.
I think its purpose may be more than just FTS?
--
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]
