github-actions[bot] commented on code in PR #66877:
URL: https://github.com/apache/doris/pull/66877#discussion_r3842539393
##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -1100,11 +1126,35 @@ Status SniiIndexReader::_try_count_only_fastpath(
RETURN_IF_ERROR(
::doris::snii::query::count_only_term_df(*logical_reader,
physical_term, &count));
}
+ // df bounds the fabricated bitmap, so it has to be inside a document
domain
+ // that is itself real. Two steps, because they fail differently.
+ const auto& stats = logical_reader->stats();
+ if (count > stats.doc_count || count > stats.indexed_doc_count) {
Review Comment:
[P1] Prove the posting, not only df bounds
These checks only prove that `DictEntry::df` is numerically in range; they
do not prove the row-accurate posting result. The existing fixture can keep
posting `{2,7}`/encoded `n=2`, set `df=1`, and rebuild the DICT CRC, so honest
Core/segment counts of 10 still publish 1 instead of 2. Conversely, with
`df=n=2` and the same honest domains, a CRC-valid posting `{2,12}` publishes 2
here while the normal segment intersection leaves one row. Please either
validate both `df == payload cardinality` and every posting member against the
real segment domain, or gate this shortcut on a format capability that
independently guarantees both invariants, with regressions for both shapes.
##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -1100,11 +1126,35 @@ Status SniiIndexReader::_try_count_only_fastpath(
RETURN_IF_ERROR(
::doris::snii::query::count_only_term_df(*logical_reader,
physical_term, &count));
}
+ // df bounds the fabricated bitmap, so it has to be inside a document
domain
+ // that is itself real. Two steps, because they fail differently.
+ const auto& stats = logical_reader->stats();
+ if (count > stats.doc_count || count > stats.indexed_doc_count) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_FILE_CORRUPTED, false>(
+ "SNII count fast path: term df {} exceeds document domain (doc
count {}, "
+ "indexed doc count {})",
+ count, stats.doc_count, stats.indexed_doc_count);
+ }
+ // Both limits above are CRC-valid fields of the SAME image as df, so an
image
+ // whose stats were inflated together with df clears them: on a real 10-row
+ // segment, df = doc_count = indexed_doc_count = 100 fabricates 100 ids,
and
+ // SegmentIterator -- which seeds _row_bitmap with [0, num_rows) and
intersects
+ // -- silently reports 10. The segment's own row count is the one bound the
+ // image cannot move. One-sided on purpose: an index covering FEWER rows
than
+ // the segment still fabricates ids inside [0, num_rows), so only an
oversized
+ // domain is corruption. This mirrors the equality SniiSegmentReader::
+ // load_inherited_index already demands of a rewrite.
+ if (stats.doc_count > _rows_of_segment) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_FILE_CORRUPTED, false>(
+ "SNII count fast path: index doc count {} exceeds the segment
row count {}",
+ stats.doc_count, _rows_of_segment);
+ }
- // Null handling. df is the exact match count REGARDLESS of nulls: the
- // writer adds no tokens for a null doc (scalar add_nulls; a NULL array row
- // is an empty range), so postings -- and therefore df -- never include
- // null rows, exactly matching MATCH's "null never matches" semantics. The
+ // Null handling. df is the exact match count REGARDLESS of nulls: the
writer
Review Comment:
[P1] Prove null-frame and posting consistency
These bounds validate only Core, but the nullable branch consumes a
separately CRC-valid frame through `_read_null_bitmap()`, skipping the
document-count, cardinality, and docid checks in
`LogicalIndexReader::read_null_docids()`. With segment/Core 10 and Core
`{indexed=8,null=2}`, a frame `{0..8}` makes accurate `df=2` fabricate
`{9,10}`; the segment clamp returns 1 while posting `{2,7}` masked by that
frame returns 0. Even a fully correlated frame `{1,3}` is insufficient if a
CRC-valid scalar posting `{1,7}` has `df=n=2`: fabrication relocates to `{0,2}`
and returns 2, while normal null masking returns 1. Please validate the exact
loaded/cached frame against Core and require scalar posting/null disjointness
(or an independent format capability) before publishing, return corruption on
mismatch, and cover both cases.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
