HaoYang670 commented on code in PR #1577:
URL: https://github.com/apache/arrow-rs/pull/1577#discussion_r851843714


##########
arrow/src/compute/kernels/substring.rs:
##########
@@ -35,21 +36,39 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
     let data = values.as_slice();
     let zero = OffsetSize::zero();
 
-    let cal_new_start: Box<dyn Fn(OffsetSize, OffsetSize) -> OffsetSize> = if 
start
-        >= zero
-    {
-        // count from the start of string
-        Box::new(|old_start: OffsetSize, end: OffsetSize| (old_start + 
start).min(end))
-    } else {
-        // count from the end of string
-        Box::new(|old_start: OffsetSize, end: OffsetSize| (end + 
start).max(old_start))
+    // check the `idx` is a valid char boundary or not
+    // If yes, return `idx`, else return error
+    let check_char_boundary = |idx: OffsetSize| {
+        let idx_usize = idx.to_usize().unwrap();
+        if idx_usize == data.len() || data[idx_usize] >> 6 != 0b10_u8 {
+            Ok(idx)
+        } else {
+            Err(ArrowError::ComputeError(format!("The index {} is invalid 
because {:#010b} is not the head byte of a char.", idx_usize, data[idx_usize])))
+        }
     };
 
-    let cal_new_length: Box<dyn Fn(OffsetSize, OffsetSize) -> OffsetSize> =
-        if let Some(length) = length {
-            Box::new(|start: OffsetSize, end: OffsetSize| (*length).min(end - 
start))
-        } else {
-            Box::new(|start: OffsetSize, end: OffsetSize| end - start)
+    // function for calculating the start index of each string

Review Comment:
   Done!



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to