andygrove commented on code in PR #307:
URL: https://github.com/apache/datafusion-comet/pull/307#discussion_r1581875517


##########
core/src/execution/datafusion/expressions/cast.rs:
##########
@@ -142,6 +226,311 @@ impl Cast {
     }
 }
 
+fn cast_string_to_i8(str: &str, eval_mode: EvalMode) -> 
CometResult<Option<i8>> {
+    Ok(cast_string_to_int_with_range_check(
+        str,
+        eval_mode,
+        "TINYINT",
+        i8::MIN as i32,
+        i8::MAX as i32,
+    )?
+    .map(|v| v as i8))
+}
+
+fn cast_string_to_i16(str: &str, eval_mode: EvalMode) -> 
CometResult<Option<i16>> {
+    Ok(cast_string_to_int_with_range_check(
+        str,
+        eval_mode,
+        "SMALLINT",
+        i16::MIN as i32,
+        i16::MAX as i32,
+    )?
+    .map(|v| v as i16))
+}
+
+fn cast_string_to_i32(str: &str, eval_mode: EvalMode) -> 
CometResult<Option<i32>> {
+    let mut accum = CastStringToInt32::default();
+    do_cast_string_to_int(&mut accum, str, eval_mode, "INT")?;
+    Ok(accum.result)
+}
+
+fn cast_string_to_i64(str: &str, eval_mode: EvalMode) -> 
CometResult<Option<i64>> {
+    let mut accum = CastStringToInt64::default();
+    do_cast_string_to_int(&mut accum, str, eval_mode, "BIGINT")?;
+    Ok(accum.result)
+}
+
+fn cast_string_to_int_with_range_check(
+    str: &str,
+    eval_mode: EvalMode,
+    type_name: &str,
+    min: i32,
+    max: i32,
+) -> CometResult<Option<i32>> {
+    let mut accum = CastStringToInt32::default();
+    do_cast_string_to_int(&mut accum, str, eval_mode, type_name)?;
+    match accum.result {
+        None => Ok(None),
+        Some(v) if v >= min && v <= max => Ok(Some(v)),
+        _ if eval_mode == EvalMode::Ansi => Err(invalid_value(str, "STRING", 
type_name)),
+        _ => Ok(None),
+    }
+}
+
+/// We support parsing strings to i32 and i64 to match Spark's logic. Support 
for i8 and i16 is
+/// implemented by first parsing as i32 and then downcasting. The 
CastStringToInt trait is
+/// introduced so that we can have the parsing logic delegate either to an i32 
or i64 accumulator
+/// and avoid the need to use macros here.
+trait CastStringToInt {
+    fn accumulate(
+        &mut self,
+        eval_mode: EvalMode,
+        type_name: &str,
+        str: &str,
+        digit: u32,
+    ) -> CometResult<()>;
+
+    fn reset(&mut self);
+
+    fn finish(
+        &mut self,
+        eval_mode: EvalMode,
+        type_name: &str,
+        str: &str,
+        negative: bool,
+    ) -> CometResult<()>;
+}
+struct CastStringToInt32 {
+    negative: bool,
+    result: Option<i32>,
+    radix: i32,
+}
+
+impl Default for CastStringToInt32 {
+    fn default() -> Self {
+        Self {
+            negative: false,
+            result: Some(0),
+            radix: 10,
+        }
+    }
+}
+
+impl CastStringToInt for CastStringToInt32 {
+    fn accumulate(
+        &mut self,
+        eval_mode: EvalMode,
+        type_name: &str,
+        str: &str,
+        digit: u32,
+    ) -> CometResult<()> {
+        // We are going to process the new digit and accumulate the result. 
However, before doing
+        // this, if the result is already smaller than the 
stopValue(Integer.MIN_VALUE / radix),
+        // then result * 10 will definitely be smaller than minValue, and we 
can stop
+        if let Some(r) = self.result {
+            let stop_value = i32::MIN / self.radix;
+            if r < stop_value {
+                self.reset();
+                return none_or_err(eval_mode, type_name, str);
+            }
+        }
+        // Since the previous result is less than or equal to 
stopValue(Integer.MIN_VALUE / radix),
+        // we can just use `result > 0` to check overflow. If result 
overflows, we should stop
+        let v = self.result.unwrap_or(0) * self.radix;
+        match v.checked_sub(digit as i32) {
+            Some(x) if x <= 0 => self.result = Some(x),
+            _ => {
+                self.reset();
+                return none_or_err(eval_mode, type_name, str);
+            }
+        }
+        Ok(())
+    }
+    fn reset(&mut self) {
+        self.result = None;
+    }
+
+    fn finish(
+        &mut self,
+        eval_mode: EvalMode,
+        type_name: &str,
+        str: &str,
+        negative: bool,
+    ) -> CometResult<()> {
+        if !negative {
+            if let Some(r) = self.result {
+                let negated = r.checked_neg().unwrap_or(-1);
+                if negated < 0 {
+                    self.reset();
+                    return none_or_err(eval_mode, type_name, str);
+                }
+                self.result = Some(negated);
+            }
+        }
+        Ok(())
+    }
+}
+
+struct CastStringToInt64 {
+    negative: bool,
+    result: Option<i64>,
+    radix: i64,
+}
+
+impl Default for CastStringToInt64 {
+    fn default() -> Self {
+        Self {
+            negative: false,
+            result: Some(0),
+            radix: 10,
+        }
+    }
+}
+
+impl CastStringToInt for CastStringToInt64 {
+    fn accumulate(
+        &mut self,
+        eval_mode: EvalMode,
+        type_name: &str,
+        str: &str,
+        digit: u32,
+    ) -> CometResult<()> {

Review Comment:
   I reimplemented this and it no longer uses options



-- 
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...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to