From e5f89e8298fa9708f2a938635cdfd135476d0d9d Mon Sep 17 00:00:00 2001
From: remzi <13716567376yh@gmail.com>
Date: Thu, 13 Oct 2022 11:48:47 +0800
Subject: [PATCH 1/2] add precision check for zero

Signed-off-by: remzi <13716567376yh@gmail.com>
---
 datafusion/sql/src/planner.rs | 35 ++++++++++++++++++++++++++++-------
 datafusion/sql/src/utils.rs   |  4 ++--
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index 400bbe4fcaa9..d3b498c91ec9 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -2862,13 +2862,34 @@ mod tests {
     }
 
     #[test]
-    fn test_int_decimal_scale_larger_precision() {
-        let sql = "SELECT CAST(10 AS DECIMAL(5, 10))";
-        let err = logical_plan(sql).expect_err("query should have failed");
-        assert_eq!(
-            r##"Internal("For decimal(precision, scale) precision must be less than or equal to 38 and scale can't be greater than precision. Got (5, 10)")"##,
-            format!("{:?}", err)
-        );
+    fn cast_to_invalid_decimal_type() {
+        // precision == 0
+        {
+            let sql = "SELECT CAST(10 AS DECIMAL(0))";
+            let err = logical_plan(sql).expect_err("query should have failed");
+            assert_eq!(
+                r##"Internal("Decimal(precision = 0, scale = 0) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                format!("{:?}", err)
+            );
+        }
+        // precision > 38
+        {
+            let sql = "SELECT CAST(10 AS DECIMAL(39))";
+            let err = logical_plan(sql).expect_err("query should have failed");
+            assert_eq!(
+                r##"Internal("Decimal(precision = 39, scale = 0) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                format!("{:?}", err)
+            );
+        }
+        // precision < scale
+        {
+            let sql = "SELECT CAST(10 AS DECIMAL(5, 10))";
+            let err = logical_plan(sql).expect_err("query should have failed");
+            assert_eq!(
+                r##"Internal("Decimal(precision = 5, scale = 10) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                format!("{:?}", err)
+            );
+        }
     }
 
     #[test]
diff --git a/datafusion/sql/src/utils.rs b/datafusion/sql/src/utils.rs
index 952ef31106fd..eecb34038007 100644
--- a/datafusion/sql/src/utils.rs
+++ b/datafusion/sql/src/utils.rs
@@ -508,9 +508,9 @@ pub(crate) fn make_decimal_type(
     };
 
     // Arrow decimal is i128 meaning 38 maximum decimal digits
-    if precision > DECIMAL128_MAX_PRECISION || scale > precision {
+    if precision == 0 || precision > DECIMAL128_MAX_PRECISION || scale > precision {
         Err(DataFusionError::Internal(format!(
-            "For decimal(precision, scale) precision must be less than or equal to 38 and scale can't be greater than precision. Got ({}, {})",
+            "Decimal(precision = {}, scale = {}) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.",
             precision, scale
         )))
     } else {

From d85c1aeefc6eab5a210faa87e597c10bf5133696 Mon Sep 17 00:00:00 2001
From: remzi <13716567376yh@gmail.com>
Date: Thu, 13 Oct 2022 12:04:04 +0800
Subject: [PATCH 2/2] update error msg

Signed-off-by: remzi <13716567376yh@gmail.com>
---
 datafusion/sql/src/planner.rs | 6 +++---
 datafusion/sql/src/utils.rs   | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index d3b498c91ec9..ff899854977c 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -2868,7 +2868,7 @@ mod tests {
             let sql = "SELECT CAST(10 AS DECIMAL(0))";
             let err = logical_plan(sql).expect_err("query should have failed");
             assert_eq!(
-                r##"Internal("Decimal(precision = 0, scale = 0) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                r##"Internal("Decimal(precision = 0, scale = 0) should satisty `0 < precision <= 38`, and `scale <= precision`.")"##,
                 format!("{:?}", err)
             );
         }
@@ -2877,7 +2877,7 @@ mod tests {
             let sql = "SELECT CAST(10 AS DECIMAL(39))";
             let err = logical_plan(sql).expect_err("query should have failed");
             assert_eq!(
-                r##"Internal("Decimal(precision = 39, scale = 0) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                r##"Internal("Decimal(precision = 39, scale = 0) should satisty `0 < precision <= 38`, and `scale <= precision`.")"##,
                 format!("{:?}", err)
             );
         }
@@ -2886,7 +2886,7 @@ mod tests {
             let sql = "SELECT CAST(10 AS DECIMAL(5, 10))";
             let err = logical_plan(sql).expect_err("query should have failed");
             assert_eq!(
-                r##"Internal("Decimal(precision = 5, scale = 10) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.")"##,
+                r##"Internal("Decimal(precision = 5, scale = 10) should satisty `0 < precision <= 38`, and `scale <= precision`.")"##,
                 format!("{:?}", err)
             );
         }
diff --git a/datafusion/sql/src/utils.rs b/datafusion/sql/src/utils.rs
index eecb34038007..16f24deff8a3 100644
--- a/datafusion/sql/src/utils.rs
+++ b/datafusion/sql/src/utils.rs
@@ -510,7 +510,7 @@ pub(crate) fn make_decimal_type(
     // Arrow decimal is i128 meaning 38 maximum decimal digits
     if precision == 0 || precision > DECIMAL128_MAX_PRECISION || scale > precision {
         Err(DataFusionError::Internal(format!(
-            "Decimal(precision = {}, scale = {}) should satisty `0 <= scale <= precision <= 38`, and `0 < precision`.",
+            "Decimal(precision = {}, scale = {}) should satisty `0 < precision <= 38`, and `scale <= precision`.",
             precision, scale
         )))
     } else {