Repository: gluesql/gluesql
Pull request #207 (2021-04-20 15:03:24)

Support binary arithmetic operations between Value::I64 and Value::F64
Arithmetic operation between Value and Literal or Literal and Literal are supported properly.
However, between `Value::I64` and `Value::F64`, it wasn't.

Now it supports `I64(1) + F64(2.0)` like arithmetic operations between `I64` and `F64`.


Patch:
diff --git a/src/data/value/error.rs b/src/data/value/error.rs
index 3ddd6c94..97f585be 100644
--- a/src/data/value/error.rs
+++ b/src/data/value/error.rs
@@ -14,17 +14,17 @@ pub enum ValueError {
     #[error("failed to parse number")]
     FailedToParseNumber,
 
-    #[error("add on non numeric value")]
-    AddOnNonNumeric,
+    #[error("add on non-numeric values: {0} + {1}")]
+    AddOnNonNumeric(String, String),
 
-    #[error("subtract on non numeric value")]
-    SubtractOnNonNumeric,
+    #[error("subtract on non-numeric values: {0} - {1}")]
+    SubtractOnNonNumeric(String, String),
 
-    #[error("multiply on non numeric value")]
-    MultiplyOnNonNumeric,
+    #[error("multiply on non-numeric values: {0} * {1}")]
+    MultiplyOnNonNumeric(String, String),
 
-    #[error("divide on non numeric value")]
-    DivideOnNonNumeric,
+    #[error("divide on non-numeric values: {0} / {1}")]
+    DivideOnNonNumeric(String, String),
 
     #[error("floating numbers cannot be grouped by")]
     FloatCannotBeGroupedBy,
diff --git a/src/data/value/mod.rs b/src/data/value/mod.rs
index 5e865c2b..9a2d04c7 100644
--- a/src/data/value/mod.rs
+++ b/src/data/value/mod.rs
@@ -140,8 +140,13 @@ impl Value {
         match (self, other) {
             (I64(a), I64(b)) => Ok(I64(a + b)),
             (F64(a), F64(b)) => Ok(F64(a + b)),
-            (Null, _) | (_, Null) => Ok(Null),
-            _ => Err(ValueError::AddOnNonNumeric.into()),
+            (I64(a), F64(b)) | (F64(b), I64(a)) => Ok(F64(*a as f64 + b)),
+            (Null, I64(_)) | (Null, F64(_)) | (I64(_), Null) | (F64(_), Null) | (Null, Null) => {
+                Ok(Null)
+            }
+            _ => Err(
+                ValueError::AddOnNonNumeric(format!("{:?}", self), format!("{:?}", other)).into(),
+            ),
         }
     }
 
@@ -150,9 +155,17 @@ impl Value {
 
         match (self, other) {
             (I64(a), I64(b)) => Ok(I64(a - b)),
+            (I64(a), F64(b)) => Ok(F64(*a as f64 - b)),
+            (F64(a), I64(b)) => Ok(F64(a - *b as f64)),
             (F64(a), F64(b)) => Ok(F64(a - b)),
-            (Null, _) | (_, Null) => Ok(Null),
-            _ => Err(ValueError::SubtractOnNonNumeric.into()),
+            (Null, I64(_)) | (Null, F64(_)) | (I64(_), Null) | (F64(_), Null) | (Null, Null) => {
+                Ok(Null)
+            }
+            _ => Err(ValueError::SubtractOnNonNumeric(
+                format!("{:?}", self),
+                format!("{:?}", other),
+            )
+            .into()),
         }
     }
 
@@ -162,8 +175,15 @@ impl Value {
         match (self, other) {
             (I64(a), I64(b)) => Ok(I64(a * b)),
             (F64(a), F64(b)) => Ok(F64(a * b)),
-            (Null, _) | (_, Null) => Ok(Null),
-            _ => Err(ValueError::MultiplyOnNonNumeric.into()),
+            (I64(a), F64(b)) | (F64(b), I64(a)) => Ok(F64(*a as f64 * b)),
+            (Null, I64(_)) | (Null, F64(_)) | (I64(_), Null) | (F64(_), Null) | (Null, Null) => {
+                Ok(Null)
+            }
+            _ => Err(ValueError::MultiplyOnNonNumeric(
+                format!("{:?}", self),
+                format!("{:?}", other),
+            )
+            .into()),
         }
     }
 
@@ -172,9 +192,16 @@ impl Value {
 
         match (self, other) {
             (I64(a), I64(b)) => Ok(I64(a / b)),
+            (I64(a), F64(b)) => Ok(F64(*a as f64 / b)),
+            (F64(a), I64(b)) => Ok(F64(a / *b as f64)),
             (F64(a), F64(b)) => Ok(F64(a / b)),
-            (Null, _) | (_, Null) => Ok(Null),
-            _ => Err(ValueError::DivideOnNonNumeric.into()),
+            (Null, I64(_)) | (Null, F64(_)) | (I64(_), Null) | (F64(_), Null) | (Null, Null) => {
+                Ok(Null)
+            }
+            _ => Err(
+                ValueError::DivideOnNonNumeric(format!("{:?}", self), format!("{:?}", other))
+                    .into(),
+            ),
         }
     }
 
@@ -219,6 +246,64 @@ mod tests {
         assert_eq!(Str("Glue".to_owned()), Str("Glue".to_owned()));
     }
 
+    #[test]
+    fn arithmetic() {
+        macro_rules! test {
+            ($op: ident $a: expr, $b: expr => $c: expr) => {
+                assert_eq!($a.$op(&$b), Ok($c));
+            };
+        }
+
+        test!(add I64(1),   I64(2)   => I64(3));
+        test!(add I64(1),   F64(2.0) => F64(3.0));
+        test!(add F64(1.0), I64(2)   => F64(3.0));
+        test!(add F64(1.0), F64(2.0) => F64(3.0));
+
+        test!(subtract I64(3),   I64(2)   => I64(1));
+        test!(subtract I64(3),   F64(2.0) => F64(1.0));
+        test!(subtract F64(3.0), I64(2)   => F64(1.0));
+        test!(subtract F64(3.0), F64(2.0) => F64(1.0));
+
+        test!(multiply I64(3),   I64(2)   => I64(6));
+        test!(multiply I64(3),   F64(2.0) => F64(6.0));
+        test!(multiply F64(3.0), I64(2)   => F64(6.0));
+        test!(multiply F64(3.0), F64(2.0) => F64(6.0));
+
+        test!(divide I64(6),   I64(2)   => I64(3));
+        test!(divide I64(6),   F64(2.0) => F64(3.0));
+        test!(divide F64(6.0), I64(2)   => F64(3.0));
+        test!(divide F64(6.0), F64(2.0) => F64(3.0));
+
+        macro_rules! null_test {
+            ($op: ident $a: expr, $b: expr) => {
+                matches!($a.$op(&$b), Ok(Null));
+            };
+        }
+
+        null_test!(add      I64(1),   Null);
+        null_test!(add      F64(1.0), Null);
+        null_test!(subtract I64(1),   Null);
+        null_test!(subtract F64(1.0), Null);
+        null_test!(multiply I64(1),   Null);
+        null_test!(multiply F64(1.0), Null);
+        null_test!(divide   I64(1),   Null);
+        null_test!(divide   F64(1.0), Null);
+
+        null_test!(add      Null, I64(1));
+        null_test!(add      Null, F64(1.0));
+        null_test!(subtract Null, I64(1));
+        null_test!(subtract Null, F64(1.0));
+        null_test!(multiply Null, I64(1));
+        null_test!(multiply Null, F64(1.0));
+        null_test!(divide   Null, I64(1));
+        null_test!(divide   Null, F64(1.0));
+
+        null_test!(add      Null, Null);
+        null_test!(subtract Null, Null);
+        null_test!(multiply Null, Null);
+        null_test!(divide   Null, Null);
+    }
+
     #[test]
     fn cast() {
         use sqlparser::ast::DataType::*;


Test patch:
diff --git a/src/tests/arithmetic.rs b/src/tests/arithmetic.rs
index 2ed1f9e6..4e3ad476 100644
--- a/src/tests/arithmetic.rs
+++ b/src/tests/arithmetic.rs
@@ -61,19 +61,35 @@ test_case!(arithmetic, async move {
 
     let test_cases = vec![
         (
-            ValueError::AddOnNonNumeric.into(),
+            ValueError::AddOnNonNumeric(
+                format!("{:?}", Value::Str("A".to_owned())),
+                format!("{:?}", Value::I64(1)),
+            )
+            .into(),
             "SELECT * FROM Arith WHERE name + id < 1",
         ),
         (
-            ValueError::SubtractOnNonNumeric.into(),
+            ValueError::SubtractOnNonNumeric(
+                format!("{:?}", Value::Str("A".to_owned())),
+                format!("{:?}", Value::I64(1)),
+            )
+            .into(),
             "SELECT * FROM Arith WHERE name - id < 1",
         ),
         (
-            ValueError::MultiplyOnNonNumeric.into(),
+            ValueError::MultiplyOnNonNumeric(
+                format!("{:?}", Value::Str("A".to_owned())),
+                format!("{:?}", Value::I64(1)),
+            )
+            .into(),
             "SELECT * FROM Arith WHERE name * id < 1",
         ),
         (
-            ValueError::DivideOnNonNumeric.into(),
+            ValueError::DivideOnNonNumeric(
+                format!("{:?}", Value::Str("A".to_owned())),
+                format!("{:?}", Value::I64(1)),
+            )
+            .into(),
             "SELECT * FROM Arith WHERE name / id < 1",
         ),
         (