From 0107d0527d5a2ae93bdaef21685886270e9cb6a8 Mon Sep 17 00:00:00 2001
From: Kirk Mitchener <kmitchener@gmail.com>
Date: Tue, 20 Sep 2022 13:46:24 -0400
Subject: [PATCH 1/2] fix

---
 datafusion/expr/src/binary_rule.rs | 42 +++++++++++-------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

diff --git a/datafusion/expr/src/binary_rule.rs b/datafusion/expr/src/binary_rule.rs
index 8f4cf3356e444..8f7cd098f9eb5 100644
--- a/datafusion/expr/src/binary_rule.rs
+++ b/datafusion/expr/src/binary_rule.rs
@@ -308,25 +308,18 @@ fn mathematics_numerical_coercion(
         (Decimal128(_, _), Decimal128(_, _)) => {
             coercion_decimal_mathematics_type(mathematics_op, lhs_type, rhs_type)
         }
-        (Decimal128(_, _), _) => {
-            let converted_decimal_type = coerce_numeric_type_to_decimal(rhs_type);
-            match converted_decimal_type {
-                None => None,
-                Some(right_decimal_type) => coercion_decimal_mathematics_type(
-                    mathematics_op,
-                    lhs_type,
-                    &right_decimal_type,
-                ),
-            }
+        (Null, dec_type @ Decimal128(_, _)) | (dec_type @ Decimal128(_, _), Null) => {
+            Some(dec_type.clone())
         }
-        (_, Decimal128(_, _)) => {
-            let converted_decimal_type = coerce_numeric_type_to_decimal(lhs_type);
+        (dec_type @ Decimal128(_, _), other_type)
+        | (other_type, dec_type @ Decimal128(_, _)) => {
+            let converted_decimal_type = coerce_numeric_type_to_decimal(other_type);
             match converted_decimal_type {
                 None => None,
-                Some(left_decimal_type) => coercion_decimal_mathematics_type(
+                Some(other_decimal_type) => coercion_decimal_mathematics_type(
                     mathematics_op,
-                    &left_decimal_type,
-                    rhs_type,
+                    dec_type,
+                    &other_decimal_type,
                 ),
             }
         }
@@ -624,19 +617,12 @@ fn eq_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
 }
 
 /// coercion rules from NULL type. Since NULL can be casted to most of types in arrow,
-/// either lhs or rhs is NULL, if NULL can be casted to type of the other side, the coecion is valid.
+/// either lhs or rhs is NULL, if NULL can be casted to type of the other side, the coercion is valid.
 fn null_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
     match (lhs_type, rhs_type) {
-        (DataType::Null, _) => {
-            if can_cast_types(&DataType::Null, rhs_type) {
-                Some(rhs_type.clone())
-            } else {
-                None
-            }
-        }
-        (_, DataType::Null) => {
-            if can_cast_types(&DataType::Null, lhs_type) {
-                Some(lhs_type.clone())
+        (DataType::Null, other_type) | (other_type, DataType::Null) => {
+            if can_cast_types(&DataType::Null, other_type) {
+                Some(other_type.clone())
             } else {
                 None
             }
@@ -679,6 +665,7 @@ mod tests {
             DataType::Float64,
             DataType::Decimal128(38, 10),
             DataType::Decimal128(20, 8),
+            DataType::Null,
         ];
         let result_types = [
             DataType::Decimal128(20, 3),
@@ -689,6 +676,7 @@ mod tests {
             DataType::Decimal128(32, 15),
             DataType::Decimal128(38, 10),
             DataType::Decimal128(25, 8),
+            DataType::Decimal128(20, 3),
         ];
         let comparison_op_types = [
             Operator::NotEq,
@@ -778,7 +766,7 @@ mod tests {
     }
 
     #[test]
-    fn test_dictionary_type_coersion() {
+    fn test_dictionary_type_coercion() {
         use DataType::*;
 
         let lhs_type = Dictionary(Box::new(Int8), Box::new(Int32));

From c775f702176ec623f1183d49fd2244923f5633e9 Mon Sep 17 00:00:00 2001
From: Kirk Mitchener <kmitchener@gmail.com>
Date: Tue, 20 Sep 2022 15:26:34 -0400
Subject: [PATCH 2/2] revert refactor -- handedness matters to decimal rules

---
 datafusion/expr/src/binary_rule.rs | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/datafusion/expr/src/binary_rule.rs b/datafusion/expr/src/binary_rule.rs
index 8f7cd098f9eb5..1e83e53baaa88 100644
--- a/datafusion/expr/src/binary_rule.rs
+++ b/datafusion/expr/src/binary_rule.rs
@@ -311,15 +311,25 @@ fn mathematics_numerical_coercion(
         (Null, dec_type @ Decimal128(_, _)) | (dec_type @ Decimal128(_, _), Null) => {
             Some(dec_type.clone())
         }
-        (dec_type @ Decimal128(_, _), other_type)
-        | (other_type, dec_type @ Decimal128(_, _)) => {
-            let converted_decimal_type = coerce_numeric_type_to_decimal(other_type);
+        (Decimal128(_, _), _) => {
+            let converted_decimal_type = coerce_numeric_type_to_decimal(rhs_type);
             match converted_decimal_type {
                 None => None,
-                Some(other_decimal_type) => coercion_decimal_mathematics_type(
+                Some(right_decimal_type) => coercion_decimal_mathematics_type(
                     mathematics_op,
-                    dec_type,
-                    &other_decimal_type,
+                    lhs_type,
+                    &right_decimal_type,
+                ),
+            }
+        }
+        (_, Decimal128(_, _)) => {
+            let converted_decimal_type = coerce_numeric_type_to_decimal(lhs_type);
+            match converted_decimal_type {
+                None => None,
+                Some(left_decimal_type) => coercion_decimal_mathematics_type(
+                    mathematics_op,
+                    &left_decimal_type,
+                    rhs_type,
                 ),
             }
         }