JingsongLi commented on code in PR #725:
URL: https://github.com/apache/paimon-rust/pull/725#discussion_r3818127065
##########
crates/paimon/src/lumina/reader.rs:
##########
@@ -64,8 +64,24 @@ fn collect_results(
}
}
impl Ord for ScoredRow {
+ // Reversed on score so the heap top is the weakest candidate; among
+ // equal scores the larger row id sorts first and is therefore evicted
+ // first. Mirrors `vector_search::ScoredRow` so both backends keep the
+ // same rows for a tied score.
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
- other.score.total_cmp(&self.score)
+ other
+ .score
+ .total_cmp(&self.score)
+ .then_with(|| self.row_id.cmp(&other.row_id))
+ }
+ }
+
+ impl ScoredRow {
+ fn is_stronger_than(&self, other: &Self) -> bool {
+ self.score
+ .total_cmp(&other.score)
Review Comment:
[P2] Do not promote NaN scores above finite candidates
`f32::total_cmp` ranks a positive NaN above every finite score, so this
changes more than the tie-break. With `top_k = 1` and native results `[(row 8,
0.5), (row 7, NaN)]`, the current code ignores the later NaN because `NaN >
0.5` is false, while this comparator evicts row 8 and returns the NaN-scored
row. A non-finite stored vector can produce such a distance; the PK-vector
metric code uses `java_float_compare` specifically to keep all NaNs worst.
Please skip/reject non-finite scores or use NaN-worst comparison semantics
here, and change the regression test so a finite candidate wins regardless of
arrival order.
--
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]
