From e0b1620fe42c6d8c6c7f49ba1996541fc841d312 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 17 Dec 2024 12:16:24 -0300 Subject: [PATCH] Limit the number of bins for histograms to avoid crashes. (#655) --- crates/ark/src/data_explorer/histogram.rs | 25 +++++++++++++++++++ .../src/modules/positron/r_data_explorer.R | 5 ++++ 2 files changed, 30 insertions(+) diff --git a/crates/ark/src/data_explorer/histogram.rs b/crates/ark/src/data_explorer/histogram.rs index 4c3d1d062..1a3afb323 100644 --- a/crates/ark/src/data_explorer/histogram.rs +++ b/crates/ark/src/data_explorer/histogram.rs @@ -632,4 +632,29 @@ mod tests { ); }) } + + #[test] + fn test_limit_bins() { + // Regression test for https://github.com/posit-dev/positron/issues/5744 + r_task(|| { + let column = harp::parse_eval_global("rep(c(1:10, 5e7), 10)").unwrap(); + + let hist = profile_histogram( + column.sexp, + &ColumnHistogramParams { + method: ColumnHistogramParamsMethod::FreedmanDiaconis, + // If num_bins wasn't set to 10, that would generate almost 200k bins + num_bins: 10, + quantiles: None, + }, + &default_options(), + ) + .unwrap(); + + assert_match!(hist, ColumnHistogram { bin_edges, bin_counts, .. } => { + assert_eq!(bin_edges.len(), 11); + assert_eq!(bin_counts.len(), 10); + }); + }) + } } diff --git a/crates/ark/src/modules/positron/r_data_explorer.R b/crates/ark/src/modules/positron/r_data_explorer.R index 40647c86b..6a95fa8b1 100644 --- a/crates/ark/src/modules/positron/r_data_explorer.R +++ b/crates/ark/src/modules/positron/r_data_explorer.R @@ -519,5 +519,10 @@ histogram_num_bins <- function(x, method, fixed_num_bins) { } } + # Honor the maximum amount of bins requested by the front-end. + if (!is.null(fixed_num_bins) && num_bins > fixed_num_bins) { + num_bins <- fixed_num_bins + } + as.integer(num_bins) }