Fix comments

xinlifoobar · Jul 18, 2024 · f3b0791 · f3b0791
1 parent dbd1ebe
commit f3b0791
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 11 deletions.
diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
@@ -109,7 +109,10 @@ impl ValueNormalizer {
         if self.normalize {
             crate::utils::normalize_value(&value)
         } else {
-            crate::utils::value_to_string(&value)
+            match crate::utils::value_to_string(&value) {
+                Some(s) => Ok(s),
+                None => internal_err!("Unsupport value type to string: {:?}", value),
+            }
         }
     }
 }

diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs
@@ -1153,10 +1153,10 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         let value_string = match &value[0] {
             SQLExpr::Identifier(i) => ident_to_string(i),
             SQLExpr::Value(v) => match crate::utils::value_to_string(v) {
-                Err(_) => {
+                None => {
                     return plan_err!("Unsupported Value {}", value[0]);
                 }
-                Ok(v) => v,
+                Some(v) => v,
             },
             // for capture signed number e.g. +8, -8
             SQLExpr::UnaryOp { op, expr } => match op {

diff --git a/datafusion/sql/src/utils.rs b/datafusion/sql/src/utils.rs
@@ -264,14 +264,17 @@ pub(crate) fn normalize_ident(id: Ident) -> String {
 }
 
 pub(crate) fn normalize_value(value: &Value) -> Result<String> {
-    value_to_string(value).map(|v| v.to_ascii_lowercase())
+    match value_to_string(value) {
+        Some(s) => Ok(s),
+        None => exec_err!("Unsupported value to normalize: {:?}", value),
+    }
 }
 
-pub(crate) fn value_to_string(value: &Value) -> Result<String> {
+pub(crate) fn value_to_string(value: &Value) -> Option<String> {
     match value {
-        Value::SingleQuotedString(s) => Ok(s.to_string()),
-        Value::DollarQuotedString(s) => Ok(s.to_string()),
-        Value::Number(_, _) | Value::Boolean(_) => Ok(value.to_string()),
+        Value::SingleQuotedString(s) => Some(s.to_string()),
+        Value::DollarQuotedString(s) => Some(s.to_string()),
+        Value::Number(_, _) | Value::Boolean(_) => Some(value.to_string()),
         Value::DoubleQuotedString(_)
         | Value::EscapedStringLiteral(_)
         | Value::NationalStringLiteral(_)
@@ -287,7 +290,7 @@ pub(crate) fn value_to_string(value: &Value) -> Result<String> {
         | Value::TripleDoubleQuotedRawStringLiteral(_)
         | Value::HexStringLiteral(_)
         | Value::Null
-        | Value::Placeholder(_) => plan_err!("Unsupported Value to normalize {}", value),
+        | Value::Placeholder(_) => None,
     }
 }
 

diff --git a/datafusion/sql/tests/sql_integration.rs b/datafusion/sql/tests/sql_integration.rs
@@ -194,8 +194,7 @@ fn test_parse_options_value_normalization() {
                 enable_options_value_normalization,
             },
         );
-        if plan.is_ok() {
-            let plan = plan.unwrap();
+        if let Ok(plan) = plan {
             assert_eq!(expected_plan, format!("{plan:?}"));
 
             match plan {