(datafusion) branch main updated: Fix Binary & Binary View Unparsing (#13427)

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 900552cbda Fix Binary & Binary View Unparsing (#13427)
900552cbda is described below

commit 900552cbdac3ee242178e4dce41339fbabcf1384
Author: Qianqian <13...@users.noreply.github.com>
AuthorDate: Mon Nov 18 12:34:27 2024 -0800

    Fix Binary & Binary View Unparsing (#13427)
    
    * Skip casting to binary when inner expr is value (#60)
    
    * Skip casting to binary when inner expr is value
    
    * Update datafusion/sql/src/unparser/expr.rs
    
    Co-authored-by: Jack Eadie <ja...@gmail.com>
    
    ---------
    
    Co-authored-by: Jack Eadie <ja...@gmail.com>
    
    * Fix binary view cast (#63)
    
    * fix
    
    * Fix clippy error
    
    ---------
    
    Co-authored-by: Jack Eadie <ja...@gmail.com>
---
 datafusion/sql/src/unparser/expr.rs | 78 ++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 19 deletions(-)

diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs
index a6f7c4fd11..3bf4ae3047 100644
--- a/datafusion/sql/src/unparser/expr.rs
+++ b/datafusion/sql/src/unparser/expr.rs
@@ -180,25 +180,7 @@ impl Unparser<'_> {
                 })
             }
             Expr::Cast(Cast { expr, data_type }) => {
-                let inner_expr = self.expr_to_sql_inner(expr)?;
-                match data_type {
-                    DataType::Dictionary(_, _) => match inner_expr {
-                        // Dictionary values don't need to be cast to other types when rewritten back to sql
-                        ast::Expr::Value(_) => Ok(inner_expr),
-                        _ => Ok(ast::Expr::Cast {
-                            kind: ast::CastKind::Cast,
-                            expr: Box::new(inner_expr),
-                            data_type: self.arrow_dtype_to_ast_dtype(data_type)?,
-                            format: None,
-                        }),
-                    },
-                    _ => Ok(ast::Expr::Cast {
-                        kind: ast::CastKind::Cast,
-                        expr: Box::new(inner_expr),
-                        data_type: self.arrow_dtype_to_ast_dtype(data_type)?,
-                        format: None,
-                    }),
-                }
+                Ok(self.cast_to_sql(expr, data_type)?)
             }
             Expr::Literal(value) => Ok(self.scalar_to_sql(value)?),
             Expr::Alias(Alias { expr, name: _, .. }) => self.expr_to_sql_inner(expr),
@@ -901,6 +883,31 @@ impl Unparser<'_> {
         })
     }
 
+    // Explicit type cast on ast::Expr::Value is not needed by underlying engine for certain types
+    // For example: CAST(Utf8("binary_value") AS Binary) and  CAST(Utf8("dictionary_value") AS Dictionary)
+    fn cast_to_sql(&self, expr: &Expr, data_type: &DataType) -> Result<ast::Expr> {
+        let inner_expr = self.expr_to_sql_inner(expr)?;
+        match inner_expr {
+            ast::Expr::Value(_) => match data_type {
+                DataType::Dictionary(_, _) | DataType::Binary | DataType::BinaryView => {
+                    Ok(inner_expr)
+                }
+                _ => Ok(ast::Expr::Cast {
+                    kind: ast::CastKind::Cast,
+                    expr: Box::new(inner_expr),
+                    data_type: self.arrow_dtype_to_ast_dtype(data_type)?,
+                    format: None,
+                }),
+            },
+            _ => Ok(ast::Expr::Cast {
+                kind: ast::CastKind::Cast,
+                expr: Box::new(inner_expr),
+                data_type: self.arrow_dtype_to_ast_dtype(data_type)?,
+                format: None,
+            }),
+        }
+    }
+
     /// 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> {
@@ -2237,6 +2244,39 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_cast_value_to_binary_expr() {
+        let tests = [
+            (
+                Expr::Cast(Cast {
+                    expr: Box::new(Expr::Literal(ScalarValue::Utf8(Some(
+                        "blah".to_string(),
+                    )))),
+                    data_type: DataType::Binary,
+                }),
+                "'blah'",
+            ),
+            (
+                Expr::Cast(Cast {
+                    expr: Box::new(Expr::Literal(ScalarValue::Utf8(Some(
+                        "blah".to_string(),
+                    )))),
+                    data_type: DataType::BinaryView,
+                }),
+                "'blah'",
+            ),
+        ];
+        for (value, expected) in tests {
+            let dialect = CustomDialectBuilder::new().build();
+            let unparser = Unparser::new(&dialect);
+
+            let ast = unparser.expr_to_sql(&value).expect("to be unparsed");
+            let actual = format!("{ast}");
+
+            assert_eq!(actual, expected);
+        }
+    }
+
     #[test]
     fn custom_dialect_use_char_for_utf8_cast() -> Result<()> {
         let default_dialect = CustomDialectBuilder::default().build();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org