Refactor GCD and LCM functions (gluesql#1331)

- GCD
The GCD function is now implemented using iteration not recursive function.
An overflow error is prevented when one of the arguments is i64::MIN

- LCM
An overflow error is prevented when one of the arguments is i64::MIN.
An overflow error is prevented when dealing with large prime numbers, i.e) LCM(large prime, large prime).
The case where both arguments are zero, ex) LCM(0, 0) is now properly handled.

core/src/data/value/error.rs

@@ -81,6 +81,12 @@ pub enum ValueError {
     #[error("unary factorial operation overflow")]
     FactorialOverflow,
 
+    #[error("GCD or LCM calculation overflowed on trying to get the absolute value of {0}")]
+    GcdLcmOverflow(i64),
+
+    #[error("LCM calculation resulted in a value out of the i64 range")]
+    LcmResultOutOfRange,
+
     #[error("unary bit_not operation for non numeric value")]
     UnaryBitwiseNotOnNonNumeric,
 

core/src/executor/evaluate/function.rs

@@ -3,7 +3,7 @@ use {
     crate::{
         ast::{DataType, DateTimeField},
         data::{Key, Point, Value, ValueError},
-        result::Result,
+        result::{Error, Result},
     },
     md5::{Digest, Md5},
     rand::{rngs::StdRng, Rng, SeedableRng},
@@ -452,26 +452,42 @@ pub fn gcd<'a>(name: String, left: Evaluated<'_>, right: Evaluated<'_>) -> Resul
     let left = eval_to_int!(name, left);
     let right = eval_to_int!(name, right);
 
-    Ok(Evaluated::from(Value::I64(gcd_i64(left, right))))
+    Ok(Evaluated::from(Value::I64(gcd_i64(left, right)?)))
 }
 
 pub fn lcm<'a>(name: String, left: Evaluated<'_>, right: Evaluated<'_>) -> Result<Evaluated<'a>> {
     let left = eval_to_int!(name, left);
     let right = eval_to_int!(name, right);
 
-    fn lcm(a: i64, b: i64) -> i64 {
-        a * b / gcd_i64(a, b)
+    fn lcm(a: i64, b: i64) -> Result<i64> {
+        let gcd_val: i128 = gcd_i64(a, b)?.into();
+
+        let a: i128 = a.into();
+        let b: i128 = b.into();
+
+        // lcm(a, b) = abs(a * b) / gcd(a, b)   if gcd(a, b) != 0
+        // lcm(a, b) = 0                        if gcd(a, b) == 0
+        let result = (a * b).abs().checked_div(gcd_val).unwrap_or(0);
+
+        i64::try_from(result).map_err(|_| Error::Value(ValueError::LcmResultOutOfRange))
     }
 
-    Ok(Evaluated::from(Value::I64(lcm(left, right))))
+    Ok(Evaluated::from(Value::I64(lcm(left, right)?)))
 }
 
-fn gcd_i64(a: i64, b: i64) -> i64 {
-    if b == 0 {
-        a
-    } else {
-        gcd_i64(b, a % b)
+fn gcd_i64(a: i64, b: i64) -> Result<i64> {
+    let mut a = a
+        .checked_abs()
+        .ok_or(Error::Value(ValueError::GcdLcmOverflow(a)))?;
+    let mut b = b
+        .checked_abs()
+        .ok_or(Error::Value(ValueError::GcdLcmOverflow(b)))?;
+
+    while b > 0 {
+        (a, b) = (b, a % b);
     }
+
+    Ok(a)
 }
 
 // --- list ---

docs/docs/sql-syntax/functions/math/gcd.md

@@ -30,4 +30,5 @@ NULL
 
 ## Errors
 1. If either of the arguments is not of INTEGER type, a `FunctionRequiresIntegerValue` error will be raised.
-2. If the number of arguments provided to the function is not equal to 2, a `FunctionArgsLengthNotMatching` error will be raised.
+2. If the number of arguments provided to the function is not equal to 2, a `FunctionArgsLengthNotMatching` error will be raised.
+3. If either of the arguments is the minimum i64 value (`-9223372036854775808`), an overflow occurs when attempting to take the absolute value. In this case, a `GcdLcmOverflowError` is raised.

docs/docs/sql-syntax/functions/math/lcm.md

@@ -30,4 +30,6 @@ NULL
 
 ## Errors
 1. If either of the arguments is not of INTEGER type, a `FunctionRequiresIntegerValue` error will be raised.
-2. If the number of arguments provided to the function is not equal to 2, a `FunctionArgsLengthNotMatching` error will be raised.
+2. If the number of arguments provided to the function is not equal to 2, a `FunctionArgsLengthNotMatching` error will be raised.
+3. If either of the arguments is the minimum i64 value (`-9223372036854775808`), an overflow occurs when attempting to calculate the gcd, which is then used in the lcm calculation. In this case, an `GcdLcmOverflowError` is raised.
+4. If the calculated result of lcm is outside the valid range of i64 (`-9223372036854775808` to `9223372036854775807`), a `LcmResultOutOfRange` error is raised. This may occur with large prime numbers.

test-suite/src/function/gcd_lcm.rs

@@ -1,7 +1,7 @@
 use {
     crate::*,
     gluesql_core::{
-        error::EvaluateError,
+        error::{EvaluateError, ValueError},
         prelude::{Payload, Value::*},
     },
 };
@@ -96,6 +96,55 @@ test_case!(gcd_lcm, async move {
             "SELECT LCM(right, left) AS test FROM LcmStr",
             Err(EvaluateError::FunctionRequiresIntegerValue("LCM".to_owned()).into()),
         ),
+        // Check edge cases
+        (
+            "SELECT GCD(0, 0) as test",
+            Ok(select_with_null!(test; I64(0))),
+        ),
+        (
+            "VALUES(
+                GCD(-1, -1),
+                GCD(-2, 0),
+                GCD(-14, 7)
+            )",
+            Ok(select_with_null!(
+                column1 | column2 | column3;
+                I64(1)    I64(2)    I64(7)
+            )),
+        ),
+        (
+            // check i64::MIN overflow error
+            "SELECT GCD(-9223372036854775808, -9223372036854775808)",
+            Err(ValueError::GcdLcmOverflow(i64::MIN).into()),
+        ),
+        (
+            "SELECT LCM(0, 0) as test",
+            Ok(select_with_null!(test; I64(0))),
+        ),
+        (
+            "VALUES(
+                LCM(-3, -5),
+                LCM(-13, 0),
+                LCM(-12, 2)
+            )
+            ",
+            Ok(select_with_null!(
+                column1 | column2 | column3;
+                I64(15)   I64(0)    I64(12)
+            )),
+        ),
+        (
+            // check i64::MIN overflow error
+            "SELECT LCM(-9223372036854775808, -9223372036854775808)",
+            Err(ValueError::GcdLcmOverflow(i64::MIN).into()),
+        ),
+        (
+            // 10^10 + 19 and 10^10 + 33 are prime numbers
+            // LCM(10^10+19, 10^10+33) = (10^10+19)*(10^10+33)
+            // this result is out of i64 range.
+            "SELECT LCM(10000000019, 10000000033)",
+            Err(ValueError::LcmResultOutOfRange.into()),
+        ),
     ];
     for (sql, expected) in test_cases {
         test!(sql, expected);