airborne12 commented on code in PR #66877:
URL: https://github.com/apache/doris/pull/66877#discussion_r3843796558


##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -1094,6 +1096,13 @@ Status SniiIndexReader::_try_count_only_fastpath(
         RETURN_IF_ERROR(
                 ::doris::snii::query::count_only_term_df(*logical_reader, 
physical_term, &count));
     }
+    const auto& stats = logical_reader->stats();

Review Comment:
   Confirmed, and fixed in d123e3ac64e.
   
   You are right on all three points I could check:
   
   - Both limits came out of the same CRC-valid Core frame as `df`, so an image 
whose stats were inflated together with `df` cleared them.
   - The query-open path really does not compare Core `doc_count` with the 
segment. The equality exists in this very file, but only on the rewrite path: 
`SniiSegmentReader::load_inherited_index` returns `"segment: inherited logical 
index doc count disagrees with segment"`; `open()` / `open_index()` never run 
it.
   - The consequence is what you describe: `SegmentIterator` seeds 
`_row_bitmap` with `[0, num_rows)` (`segment_iterator.cpp:545`) and intersects, 
so a 100-id fabrication on a 10-row segment reports 10 for a term matching 2.
   
   The segment row count turned out to be already on hand where the reader is 
built — `ColumnReader::_load_index` takes `rows_of_segment` and passes it to 
`AnnIndexReader`; `SniiIndexReader` simply was not given it. It now is, and the 
fast path rejects a document domain larger than the segment.
   
   One deliberate difference from your wording: the check is one-sided 
(`stats.doc_count > _rows_of_segment`), not an equality. An index covering 
fewer rows than the segment still fabricates ids inside `[0, num_rows)`, so 
only an oversized domain is corruption; requiring equality would add a failure 
mode without adding safety here.
   
   New case `CountFastPathRejectsADocumentDomainLargerThanTheSegment` builds 
the jointly inflated image you described (`df == doc_count == indexed_doc_count 
== 100`, real segment 10 rows, posting still the honest `{2, 7}`). 
Mutation-tested: disabling the segment bound turns exactly that case red and 
leaves the other corruption cases green.



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

Reply via email to