alamb commented on code in PR #9578:
URL: https://github.com/apache/arrow-datafusion/pull/9578#discussion_r1522295345


##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -174,133 +225,243 @@ impl Unparser<'_> {
         }
     }
 
-    fn scalar_to_sql(&self, v: &ScalarValue) -> Result<ast::Value> {
+    /// DataFusion ScalarValues sometimes require a ast::Expr to construct.
+    /// For example ScalarValue::Date32(d) corresponds to the ast::Expr 
CAST('datestr' as DATE)
+    fn scalar_to_sql(&self, v: &ScalarValue) -> Result<ast::Expr> {
         match v {
-            ScalarValue::Null => Ok(ast::Value::Null),
-            ScalarValue::Boolean(Some(b)) => 
Ok(ast::Value::Boolean(b.to_owned())),
-            ScalarValue::Boolean(None) => Ok(ast::Value::Null),
-            ScalarValue::Float32(Some(f)) => 
Ok(ast::Value::Number(f.to_string(), false)),
-            ScalarValue::Float32(None) => Ok(ast::Value::Null),
-            ScalarValue::Float64(Some(f)) => 
Ok(ast::Value::Number(f.to_string(), false)),
-            ScalarValue::Float64(None) => Ok(ast::Value::Null),
+            ScalarValue::Null => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Boolean(Some(b)) => {
+                Ok(ast::Expr::Value(ast::Value::Boolean(b.to_owned())))
+            }
+            ScalarValue::Boolean(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Float32(Some(f)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(f.to_string(), false)))
+            }
+            ScalarValue::Float32(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Float64(Some(f)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(f.to_string(), false)))
+            }
+            ScalarValue::Float64(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
             ScalarValue::Decimal128(Some(_), ..) => {
                 not_impl_err!("Unsupported scalar: {v:?}")
             }
-            ScalarValue::Decimal128(None, ..) => Ok(ast::Value::Null),
+            ScalarValue::Decimal128(None, ..) => 
Ok(ast::Expr::Value(ast::Value::Null)),
             ScalarValue::Decimal256(Some(_), ..) => {
                 not_impl_err!("Unsupported scalar: {v:?}")
             }
-            ScalarValue::Decimal256(None, ..) => Ok(ast::Value::Null),
-            ScalarValue::Int8(Some(i)) => Ok(ast::Value::Number(i.to_string(), 
false)),
-            ScalarValue::Int8(None) => Ok(ast::Value::Null),
-            ScalarValue::Int16(Some(i)) => 
Ok(ast::Value::Number(i.to_string(), false)),
-            ScalarValue::Int16(None) => Ok(ast::Value::Null),
-            ScalarValue::Int32(Some(i)) => 
Ok(ast::Value::Number(i.to_string(), false)),
-            ScalarValue::Int32(None) => Ok(ast::Value::Null),
-            ScalarValue::Int64(Some(i)) => 
Ok(ast::Value::Number(i.to_string(), false)),
-            ScalarValue::Int64(None) => Ok(ast::Value::Null),
-            ScalarValue::UInt8(Some(ui)) => 
Ok(ast::Value::Number(ui.to_string(), false)),
-            ScalarValue::UInt8(None) => Ok(ast::Value::Null),
-            ScalarValue::UInt16(Some(ui)) => {
-                Ok(ast::Value::Number(ui.to_string(), false))
+            ScalarValue::Decimal256(None, ..) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Int8(Some(i)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(i.to_string(), false)))
             }
-            ScalarValue::UInt16(None) => Ok(ast::Value::Null),
-            ScalarValue::UInt32(Some(ui)) => {
-                Ok(ast::Value::Number(ui.to_string(), false))
+            ScalarValue::Int8(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Int16(Some(i)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(i.to_string(), false)))
             }
-            ScalarValue::UInt32(None) => Ok(ast::Value::Null),
-            ScalarValue::UInt64(Some(ui)) => {
-                Ok(ast::Value::Number(ui.to_string(), false))
+            ScalarValue::Int16(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Int32(Some(i)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(i.to_string(), false)))
             }
-            ScalarValue::UInt64(None) => Ok(ast::Value::Null),
-            ScalarValue::Utf8(Some(str)) => {
-                Ok(ast::Value::SingleQuotedString(str.to_string()))
+            ScalarValue::Int32(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Int64(Some(i)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(i.to_string(), false)))
             }
-            ScalarValue::Utf8(None) => Ok(ast::Value::Null),
-            ScalarValue::LargeUtf8(Some(str)) => {
-                Ok(ast::Value::SingleQuotedString(str.to_string()))
+            ScalarValue::Int64(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::UInt8(Some(ui)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(ui.to_string(), false)))
             }
-            ScalarValue::LargeUtf8(None) => Ok(ast::Value::Null),
+            ScalarValue::UInt8(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::UInt16(Some(ui)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(ui.to_string(), false)))
+            }
+            ScalarValue::UInt16(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::UInt32(Some(ui)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(ui.to_string(), false)))
+            }
+            ScalarValue::UInt32(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::UInt64(Some(ui)) => {
+                Ok(ast::Expr::Value(ast::Value::Number(ui.to_string(), false)))
+            }
+            ScalarValue::UInt64(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::Utf8(Some(str)) => Ok(ast::Expr::Value(
+                ast::Value::SingleQuotedString(str.to_string()),
+            )),
+            ScalarValue::Utf8(None) => Ok(ast::Expr::Value(ast::Value::Null)),
+            ScalarValue::LargeUtf8(Some(str)) => Ok(ast::Expr::Value(
+                ast::Value::SingleQuotedString(str.to_string()),
+            )),
+            ScalarValue::LargeUtf8(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
             ScalarValue::Binary(Some(_)) => not_impl_err!("Unsupported scalar: 
{v:?}"),
-            ScalarValue::Binary(None) => Ok(ast::Value::Null),
+            ScalarValue::Binary(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
             ScalarValue::FixedSizeBinary(..) => {
                 not_impl_err!("Unsupported scalar: {v:?}")
             }
             ScalarValue::LargeBinary(Some(_)) => {
                 not_impl_err!("Unsupported scalar: {v:?}")
             }
-            ScalarValue::LargeBinary(None) => Ok(ast::Value::Null),
+            ScalarValue::LargeBinary(None) => 
Ok(ast::Expr::Value(ast::Value::Null)),
             ScalarValue::FixedSizeList(_a) => not_impl_err!("Unsupported 
scalar: {v:?}"),
             ScalarValue::List(_a) => not_impl_err!("Unsupported scalar: 
{v:?}"),
             ScalarValue::LargeList(_a) => not_impl_err!("Unsupported scalar: 
{v:?}"),
-            ScalarValue::Date32(Some(_d)) => not_impl_err!("Unsupported 
scalar: {v:?}"),
-            ScalarValue::Date32(None) => Ok(ast::Value::Null),
-            ScalarValue::Date64(Some(_d)) => not_impl_err!("Unsupported 
scalar: {v:?}"),
-            ScalarValue::Date64(None) => Ok(ast::Value::Null),
+            ScalarValue::Date32(Some(d)) => {

Review Comment:
   I wonder if there is some function we can use in arrow-rs do to this 
conversion
   
   Perhaps 
https://docs.rs/arrow/latest/arrow/array/types/struct.Date32Type.html#method.to_naive_date
 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to