alamb commented on code in PR #22293:
URL: https://github.com/apache/datafusion/pull/22293#discussion_r3305935939


##########
datafusion/functions/src/string/repeat.rs:
##########
@@ -166,15 +166,43 @@ fn compute_repeat(s: &str, count: i64, max_size: usize) 
-> Result<String> {
     if count <= 0 {
         return Ok(String::new());
     }
-    let result_len = s.len().saturating_mul(count as usize);
+    let result_len = repeat_len(s.len(), count, max_size)?;
+    debug_assert!(result_len <= max_size);
+    let count = repeat_count(count, max_size)?;
+    Ok(s.repeat(count))
+}
+
+fn repeat_len(string_len: usize, count: i64, max_size: usize) -> Result<usize> 
{
+    let count = repeat_count(count, max_size)?;
+    let result_len = match string_len.checked_mul(count) {
+        Some(result_len) => result_len,
+        None => {
+            return exec_err!(
+                "string size overflow on repeat, max size is {}, but got {}",
+                max_size,
+                usize::MAX
+            );
+        }
+    };

Review Comment:
   I think you could make this (and the other callsites) more concise like
   
   ```rust
       let result_len = string_len.checked_mul(count)
         .ok_or_else(||exec_err!(
                   "string size overflow on repeat, max size is {}, but got {}",
                   max_size,
                   usize::MAX
               );
           })?;
   



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to