Copilot commented on code in PR #565:
URL: https://github.com/apache/sedona-db/pull/565#discussion_r2748322138
##########
rust/sedona/src/show.rs:
##########
@@ -521,7 +533,19 @@ impl DisplayColumn {
/// Create a cell for this column. This is where alignment for numeric
/// output is applied.
fn cell<T: ToString>(&self, content: T) -> Cell {
- let cell = Cell::new(content).set_delimiter('\0');
+ // Never return content longer than WIDTH_MAX because comfy table may
+ // do arithmetic with it without checking for overflow.
+ let content_str = content.to_string();
+ let content_safe = if content_str.chars().count() > WIDTH_MAX as usize
{
+ content_str
+ .chars()
+ .take(WIDTH_MAX as usize)
+ .collect::<String>()
+ } else {
+ content_str
+ };
Review Comment:
Counting all characters before truncation is inefficient for very long
strings. The `.chars().count()` call iterates through the entire string even
when it exceeds WIDTH_MAX. Consider using `.chars().take(WIDTH_MAX as usize +
1).count()` to avoid processing characters beyond the limit, or simply always
truncate with `.chars().take(WIDTH_MAX as usize).collect()` without the
conditional check.
```suggestion
let content_safe: String = content_str
.chars()
.take(WIDTH_MAX as usize)
.collect();
```
--
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]