Skip to content

Commit 879fb1d

Browse files
committed
raw: address review feedback for reverse_scan fix
- Add explicit error for reverse scan without upper bound - Add forward scan safety check to prevent infinite loops - Simplify region lookup now that end_key is validated
1 parent 7e6311d commit 879fb1d

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

src/raw/client.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -764,11 +764,19 @@ impl<PdC: PdClient> Client<PdC> {
764764
let mut current_limit = limit;
765765
let (start_key, end_key) = range.clone().into_keys();
766766

767+
// Reverse scan requires a bounded upper range to know where to start.
768+
// Locating the last region is not implemented.
769+
if reverse && end_key.is_none() {
770+
return Err(Error::InternalError {
771+
message: "reverse scan requires an upper bound (end_key)".to_string(),
772+
});
773+
}
774+
767775
// For forward scan: current_key tracks the lower bound, moving forward
768776
// For reverse scan: current_key tracks the upper bound, moving backward
769777
let mut current_key: Key = if reverse {
770-
// Start from the upper bound for reverse scan
771-
end_key.clone().unwrap_or_default()
778+
// Safe to unwrap: we validated end_key.is_some() above
779+
end_key.clone().unwrap()
772780
} else {
773781
start_key.clone()
774782
};
@@ -828,6 +836,10 @@ impl<PdC: PdClient> Client<PdC> {
828836
if next_key.is_empty() || end_key.clone().is_some_and(|ek| ek <= next_key) {
829837
break;
830838
}
839+
// Safety: if next_key <= current_key, we've made no progress
840+
if next_key <= current_key {
841+
break;
842+
}
831843
current_key = next_key;
832844
}
833845
}
@@ -856,12 +868,8 @@ impl<PdC: PdClient> Client<PdC> {
856868
// When reverse=true, new_raw_scan_request swaps the keys so that TiKV receives
857869
// end_key as its start_key. We must select the region containing that key.
858870
let region_lookup_key: &Key = if reverse {
859-
match &end_key {
860-
Some(ek) if !ek.is_empty() => ek,
861-
// If no upper bound, fall back to start_key (though this case
862-
// is documented as unsupported for reverse scan)
863-
_ => &start_key,
864-
}
871+
// For reverse scan, end_key is guaranteed to be Some (validated in scan_inner)
872+
end_key.as_ref().unwrap()
865873
} else {
866874
&start_key
867875
};

0 commit comments

Comments
 (0)