airborne12 commented on PR #67134:
URL: https://github.com/apache/doris/pull/67134#issuecomment-5420891320
Updated: the `inverted_index_p0.storage_format.test_common_grams_snii`
regression case would have failed on the previous revision. Two of its
assertions expected `SNII semantic scoring metadata is missing`, and the change
makes both of those queries succeed — but one of them only after fixing a real
bug in the patch.
**A defect the regression case exposed.** The plain shape was signalled by
an EMPTY base-analyzer fingerprint, because only a CommonGrams segment records
one (its term keys depend on the dictionary identity). That empty value then
flowed into the cross-segment check "were these two segments analyzed with the
same analyzer", where `"" != "cg_default_analyzer-fingerprint"` read as *a
different analyzer* and rejected the whole collection. A table holding one
CommonGrams segment and one plain segment for the same field — what you get by
toggling `enable_common_grams_index_build` between loads — was refused with a
message that was not merely unhelpful but false: both segments were analyzed by
the same analyzer, one simply did not build grams.
Their statistics are combinable, and that is the point of the semantic
counts existing at all:
```
CommonGrams segment 'alpha beta the world' physical sum_total_term_freq =
6 (4 tokens + 2 grams)
metadata.scoring_token_count =
4 <- used
plain segment 'alpha beta the worker' physical sum_total_term_freq =
4 <- used, already semantic
collection: N = 2, tokens = 8,
avgdl = 4
```
Summing the physical values would give avgdl 5. Document frequency is
comparable too: `SpimiTermBuffer::add_common_gram_and_plain` writes the gram
AND the token's own plain posting, so plain postings are complete in a
CommonGrams segment and df is measured identically on both sides.
Fixed by requiring BOTH fingerprints to be present before comparing them,
and by never letting an absent identity overwrite a recorded one.
**Test changes:**
- New `CollectionStatisticsTest.SniiMixedCommonGramsAndPlainSegmentsCombine`
drives `CollectionStatistics::collect` over one segment of each shape and
asserts the summed statistics.
- `LegacyAndCommonGramsMixRejectsWholeCollection` went red and was
retargeted rather than deleted. Its intent — a segment with no scoring data
poisons a collection it is mixed into — is still right; only its marker was
stale. It faked "no CommonGrams metadata but T3 with norms", a shape that could
not exist while scoring rode on CommonGrams and is now the ordinary analyzed
segment. It now uses the real marker, absent norms.
- The two regression assertions expect the ranking to match the V3 control
(`assertEquals(rankedIds(...))`) instead of an exception. No new `qt_` outputs
were added, so no `.out` file needs regenerating.
Full inverted-index suites: 3213 tests, 0 failures. clang-format 16 clean;
clang-tidy reports nothing on any added line.
--
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]
