jerry-024 commented on code in PR #749:
URL: https://github.com/apache/paimon-rust/pull/749#discussion_r3870779198
##########
crates/paimon/src/io/file_io.rs:
##########
@@ -223,23 +225,38 @@ impl FileIO {
/// List all files recursively under the given directory path.
pub async fn list_status_recursive(&self, path: &str) ->
Result<Vec<FileStatus>> {
+ self.list_status_recursive_with_limit(path, None).await
+ }
+
+ pub(crate) async fn list_status_recursive_with_limit(
+ &self,
+ path: &str,
+ limit: Option<usize>,
+ ) -> Result<Vec<FileStatus>> {
+ if limit == Some(0) {
+ return Ok(Vec::new());
+ }
+
let (op, relative_path) = self.create(path).await?;
// See `list_status`: `relative_path` is a byte-suffix of `path` except
// for Windows local paths, where it only swaps separators (same
length).
let base_path = &path[..path.len() - relative_path.len()];
let list_path = normalize_root(relative_path.as_ref());
- let entries =
- op.list_with(&list_path)
+ let mut entries =
+ op.lister_with(&list_path)
.recursive(true)
.await
.context(IoUnexpectedSnafu {
message: format!("Failed to list files recursively in
'{path}'"),
})?;
let mut statuses = Vec::new();
+ let mut smallest = limit.map(|limit|
BinaryHeap::with_capacity(limit.saturating_add(1)));
Review Comment:
<!-- dlf-review -->
**[MAJOR]** `BinaryHeap::with_capacity(limit.saturating_add(1))` allocates
from the SQL `LIMIT` before the lister yields any object. A query with a very
large limit can therefore request gigabytes of memory—or panic on capacity
overflow—even when the directory is empty.
Please use `BinaryHeap::new()` and let the heap grow only as matching files
are observed. The existing `heap.len() > limit` check already keeps retained
entries bounded.
--
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]
