airborne12 commented on PR #66872: URL: https://github.com/apache/doris/pull/66872#issuecomment-5412993398
Closing. The decision comes down to a behaviour comparison with V1/V2/V3, which I should have run before opening this. **What V1/V2/V3 do when `score()` meets an index that cannot be ranked: nothing.** There is no rejection anywhere in that path. - FE: the only index-related rejection in `CheckScoreUsage` is `usesCommonGrams && !isSnii`. A keyword index, or an analyzed index built without `support_phrase`, is admitted without comment. - BE, keyword index: it is served by `StringTypeInvertedIndexReader`, which has no similarity path at all — the field simply contributes no score. - BE, analyzed index without positions: `IndexReaderHelper::is_need_similarity_score` returns false, `query_info.is_similarity_score` is never set, and again nothing is scored. An unscoreable index is a field that contributes no relevance, not an error. That matches every mainstream engine — Elasticsearch's `filter`/`must_not` clauses return score `0` and never invalidate the query, Lucene's multi-term queries fall back to a constant score, Solr's `fq` restricts "without influencing score" — and it is the contract the rest of Doris already implements. With #67134 in, SNII lines up with that contract almost exactly: | shape | V1/V2/V3 | SNII after #67134 | |---|---|---| | analyzed + positions | ranks | ranks | | analyzed, `support_phrase=false` | silently no score | silently no score (same `is_need_similarity_score`) | | keyword (no parser, no analyzer) | silently no score | **errors** | Adding an FE rejection for SNII would make the first two rows diverge, not converge: the same SQL would return rows on a V3 table and fail at analysis time on an SNII one. That is the opposite of what this PR set out to achieve. The one genuine divergence left is the third row, and it is a BE issue, not an FE one: `is_need_similarity_score` keys off `support_phrase` alone and ignores `should_analyzer`, so a keyword index declared with `support_phrase=true` raises `actual_similarity`, SNII then opens its stats provider and fails, while V2/V3 was never on that code path because a keyword index uses a different reader class. Fixing that means teaching SNII to skip silently, in the BE — I will carry it as follow-up to #67134. Also carrying forward, independent of scoring: the FE/BE index-resolution divergence from the third review thread. `OlapTable.getInvertedIndex(column, subPath, null)` returns the first index for which `isAnalyzedInvertedIndex()` is true regardless of declaration order, while the BE routes an EXACT clause to `InvertedIndexReaderType::STRING_TYPE`, so the two can select different physical indexes for the same leaf. `test_search_exact_multi_index.groovy` shows that multi-index-per-column shape is supported and tested. That one deserves its own issue. Thanks for the review — all three findings were correct, and two of them are what showed this rule belonged one layer down. -- 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]
