From ebaa85f5bbd4fd28a479ae8e0b6f29e1c04dd654 Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Fri, 29 Apr 2022 21:06:02 +0800
Subject: [PATCH 1/9] Support struct_expr generate struct in sql

---
 .../core/src/physical_plan/functions.rs       |   2 +
 datafusion/core/tests/sql/expr.rs             |  17 +++
 datafusion/expr/src/built_in_function.rs      |   4 +
 datafusion/expr/src/function.rs               |  16 ++-
 datafusion/expr/src/lib.rs                    |   1 +
 datafusion/expr/src/struct_expressions.rs     |  35 ++++++
 datafusion/physical-expr/src/lib.rs           |   1 +
 .../physical-expr/src/struct_expressions.rs   | 109 ++++++++++++++++++
 datafusion/proto/proto/datafusion.proto       |   1 +
 datafusion/proto/src/from_proto.rs            |   1 +
 datafusion/proto/src/to_proto.rs              |   1 +
 11 files changed, 186 insertions(+), 2 deletions(-)
 create mode 100644 datafusion/expr/src/struct_expressions.rs
 create mode 100644 datafusion/physical-expr/src/struct_expressions.rs

diff --git a/datafusion/core/src/physical_plan/functions.rs b/datafusion/core/src/physical_plan/functions.rs
index 20917fa9b4d1..98dc9a16dce8 100644
--- a/datafusion/core/src/physical_plan/functions.rs
+++ b/datafusion/core/src/physical_plan/functions.rs
@@ -50,6 +50,7 @@ use datafusion_physical_expr::conditional_expressions;
 use datafusion_physical_expr::datetime_expressions;
 use datafusion_physical_expr::math_expressions;
 use datafusion_physical_expr::string_expressions;
+use datafusion_physical_expr::struct_expressions;
 use std::sync::Arc;
 
 /// Create a physical (function) expression.
@@ -299,6 +300,7 @@ pub fn create_physical_fun(
 
         // string functions
         BuiltinScalarFunction::Array => Arc::new(array_expressions::array),
+        BuiltinScalarFunction::Struct => Arc::new(struct_expressions::struct_expr),
         BuiltinScalarFunction::Ascii => Arc::new(|args| match args[0].data_type() {
             DataType::Utf8 => {
                 make_scalar_function(string_expressions::ascii::<i32>)(args)
diff --git a/datafusion/core/tests/sql/expr.rs b/datafusion/core/tests/sql/expr.rs
index 5f0da3458bab..e6921ae57a56 100644
--- a/datafusion/core/tests/sql/expr.rs
+++ b/datafusion/core/tests/sql/expr.rs
@@ -510,6 +510,23 @@ async fn test_array_literals() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_struct_literals() -> Result<()> {
+    test_expression!(
+        "STRUCT(1,2,3,4,5)",
+        "{\"f_0\": 1, \"f_1\": 2, \"f_2\": 3, \"f_3\": 4, \"f_4\": 5}"
+    );
+    test_expression!("STRUCT(Null)", "{\"f_0\": null}");
+    test_expression!("STRUCT(1,Null)", "{\"f_0\": \"1\", \"f_1\": null}");
+    test_expression!("STRUCT(true, false)", "{\"f_0\": true, \"f_1\": false}");
+    test_expression!(
+        "STRUCT('str1', 'str2')",
+        "{\"f_0\": \"str1\", \"f_1\": \"str2\"}"
+    );
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_interval_expressions() -> Result<()> {
     // day nano intervals
diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs
index 17df179ed400..052fa7ace3f6 100644
--- a/datafusion/expr/src/built_in_function.rs
+++ b/datafusion/expr/src/built_in_function.rs
@@ -158,6 +158,8 @@ pub enum BuiltinScalarFunction {
     Upper,
     /// regexp_match
     RegexpMatch,
+    ///struct
+    Struct,
 }
 
 impl BuiltinScalarFunction {
@@ -236,6 +238,7 @@ impl BuiltinScalarFunction {
             BuiltinScalarFunction::Trim => Volatility::Immutable,
             BuiltinScalarFunction::Upper => Volatility::Immutable,
             BuiltinScalarFunction::RegexpMatch => Volatility::Immutable,
+            BuiltinScalarFunction::Struct => Volatility::Immutable,
 
             // Stable builtin functions
             BuiltinScalarFunction::Now => Volatility::Stable,
@@ -329,6 +332,7 @@ impl FromStr for BuiltinScalarFunction {
             "trim" => BuiltinScalarFunction::Trim,
             "upper" => BuiltinScalarFunction::Upper,
             "regexp_match" => BuiltinScalarFunction::RegexpMatch,
+            "struct" => BuiltinScalarFunction::Struct,
             _ => {
                 return Err(DataFusionError::Plan(format!(
                     "There is no built-in function named {}",
diff --git a/datafusion/expr/src/function.rs b/datafusion/expr/src/function.rs
index 385e247bd3a6..0abdbaec3a9b 100644
--- a/datafusion/expr/src/function.rs
+++ b/datafusion/expr/src/function.rs
@@ -21,8 +21,8 @@ use crate::nullif::SUPPORTED_NULLIF_TYPES;
 use crate::type_coercion::data_types;
 use crate::ColumnarValue;
 use crate::{
-    array_expressions, conditional_expressions, Accumulator, BuiltinScalarFunction,
-    Signature, TypeSignature,
+    array_expressions, conditional_expressions, struct_expressions, Accumulator,
+    BuiltinScalarFunction, Signature, TypeSignature,
 };
 use arrow::datatypes::{DataType, Field, TimeUnit};
 use datafusion_common::{DataFusionError, Result};
@@ -222,6 +222,14 @@ pub fn return_type(
             _ => Ok(DataType::Float64),
         },
 
+        BuiltinScalarFunction::Struct => {
+            let fields = input_expr_types
+                .iter()
+                .map(|x| Field::new("item", x.clone(), true))
+                .collect();
+            Ok(DataType::Struct(fields))
+        }
+
         BuiltinScalarFunction::Abs
         | BuiltinScalarFunction::Acos
         | BuiltinScalarFunction::Asin
@@ -256,6 +264,10 @@ pub fn signature(fun: &BuiltinScalarFunction) -> Signature {
             array_expressions::SUPPORTED_ARRAY_TYPES.to_vec(),
             fun.volatility(),
         ),
+        BuiltinScalarFunction::Struct => Signature::variadic(
+            struct_expressions::SUPPORTED_STRUCT_TYPES.to_vec(),
+            fun.volatility(),
+        ),
         BuiltinScalarFunction::Concat | BuiltinScalarFunction::ConcatWithSeparator => {
             Signature::variadic(vec![DataType::Utf8], fun.volatility())
         }
diff --git a/datafusion/expr/src/lib.rs b/datafusion/expr/src/lib.rs
index b513bf52d4a1..5cb2f7286c82 100644
--- a/datafusion/expr/src/lib.rs
+++ b/datafusion/expr/src/lib.rs
@@ -32,6 +32,7 @@ pub mod logical_plan;
 mod nullif;
 mod operator;
 mod signature;
+pub mod struct_expressions;
 mod table_source;
 pub mod type_coercion;
 mod udaf;
diff --git a/datafusion/expr/src/struct_expressions.rs b/datafusion/expr/src/struct_expressions.rs
new file mode 100644
index 000000000000..bbfcac0e2396
--- /dev/null
+++ b/datafusion/expr/src/struct_expressions.rs
@@ -0,0 +1,35 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use arrow::datatypes::DataType;
+
+/// Currently supported types by the struct function.
+pub static SUPPORTED_STRUCT_TYPES: &[DataType] = &[
+    DataType::Boolean,
+    DataType::UInt8,
+    DataType::UInt16,
+    DataType::UInt32,
+    DataType::UInt64,
+    DataType::Int8,
+    DataType::Int16,
+    DataType::Int32,
+    DataType::Int64,
+    DataType::Float32,
+    DataType::Float64,
+    DataType::Utf8,
+    DataType::LargeUtf8,
+];
diff --git a/datafusion/physical-expr/src/lib.rs b/datafusion/physical-expr/src/lib.rs
index 38b0da3b8d1e..34231934ddce 100644
--- a/datafusion/physical-expr/src/lib.rs
+++ b/datafusion/physical-expr/src/lib.rs
@@ -29,6 +29,7 @@ mod physical_expr;
 pub mod regex_expressions;
 mod sort_expr;
 pub mod string_expressions;
+pub mod struct_expressions;
 #[cfg(feature = "unicode_expressions")]
 pub mod unicode_expressions;
 pub mod window;
diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
new file mode 100644
index 000000000000..b4186e0831e1
--- /dev/null
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Struct expressions
+
+use arrow::array::*;
+use arrow::datatypes::{DataType, Field};
+use datafusion_common::{DataFusionError, Result};
+use datafusion_expr::ColumnarValue;
+use std::sync::Arc;
+
+fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
+    // do not accept 0 arguments.
+    if args.is_empty() {
+        return Err(DataFusionError::Internal(
+            "struct requires at least one argument".to_string(),
+        ));
+    }
+
+    let vec: Vec<_> = args
+        .iter()
+        .enumerate()
+        .map(|(i, arg)| -> (Field, ArrayRef) {
+            match arg.data_type() {
+                DataType::Utf8 => (
+                    Field::new(&*format!("f_{}", i), DataType::Utf8, true),
+                    arg.clone(),
+                ),
+                DataType::LargeUtf8 => (
+                    Field::new(&*format!("f_{}", i), DataType::LargeUtf8, true),
+                    arg.clone(),
+                ),
+                DataType::Boolean => (
+                    Field::new(&*format!("f_{}", i), DataType::Boolean, true),
+                    arg.clone(),
+                ),
+                DataType::Float32 => (
+                    Field::new(&*format!("f_{}", i), DataType::Float64, true),
+                    arg.clone(),
+                ),
+                DataType::Float64 => (
+                    Field::new(&*format!("f_{}", i), DataType::Float64, true),
+                    arg.clone(),
+                ),
+                DataType::Int8 => (
+                    Field::new(&*format!("f_{}", i), DataType::Int8, true),
+                    arg.clone(),
+                ),
+                DataType::Int16 => (
+                    Field::new(&*format!("f_{}", i), DataType::Int16, true),
+                    arg.clone(),
+                ),
+                DataType::Int32 => (
+                    Field::new(&*format!("f_{}", i), DataType::Int32, true),
+                    arg.clone(),
+                ),
+                DataType::Int64 => (
+                    Field::new(&*format!("f_{}", i), DataType::Int64, true),
+                    arg.clone(),
+                ),
+                DataType::UInt8 => (
+                    Field::new(&*format!("f_{}", i), DataType::UInt8, true),
+                    arg.clone(),
+                ),
+                DataType::UInt16 => (
+                    Field::new(&*format!("f_{}", i), DataType::UInt16, true),
+                    arg.clone(),
+                ),
+                DataType::UInt32 => (
+                    Field::new(&*format!("f_{}", i), DataType::UInt32, true),
+                    arg.clone(),
+                ),
+                DataType::UInt64 => (
+                    Field::new(&*format!("f_{}", i), DataType::UInt64, true),
+                    arg.clone(),
+                ),
+                data_type => unimplemented!("struct not support {} type", data_type),
+            }
+        })
+        .collect();
+
+    Ok(Arc::new(StructArray::from(vec)))
+}
+
+/// put values in an array.
+pub fn struct_expr(values: &[ColumnarValue]) -> Result<ColumnarValue> {
+    let arrays: Vec<ArrayRef> = values
+        .iter()
+        .map(|x| match x {
+            ColumnarValue::Array(array) => array.clone(),
+            ColumnarValue::Scalar(scalar) => scalar.to_array().clone(),
+        })
+        .collect();
+    Ok(ColumnarValue::Array(array_struct(arrays.as_slice())?))
+}
diff --git a/datafusion/proto/proto/datafusion.proto b/datafusion/proto/proto/datafusion.proto
index 1651a70191ea..df0d51e36cc9 100644
--- a/datafusion/proto/proto/datafusion.proto
+++ b/datafusion/proto/proto/datafusion.proto
@@ -185,6 +185,7 @@ enum ScalarFunction {
   Upper=62;
   Coalesce=63;
   Power=64;
+  StructFun=65;
 }
 
 message ScalarFunctionNode {
diff --git a/datafusion/proto/src/from_proto.rs b/datafusion/proto/src/from_proto.rs
index 37466dae207d..e2f2d6216626 100644
--- a/datafusion/proto/src/from_proto.rs
+++ b/datafusion/proto/src/from_proto.rs
@@ -467,6 +467,7 @@ impl From<&protobuf::ScalarFunction> for BuiltinScalarFunction {
             ScalarFunction::RegexpMatch => Self::RegexpMatch,
             ScalarFunction::Coalesce => Self::Coalesce,
             ScalarFunction::Power => Self::Power,
+            ScalarFunction::StructFun => Self::Struct,
         }
     }
 }
diff --git a/datafusion/proto/src/to_proto.rs b/datafusion/proto/src/to_proto.rs
index 03a9f6b10432..fa2251cd0f03 100644
--- a/datafusion/proto/src/to_proto.rs
+++ b/datafusion/proto/src/to_proto.rs
@@ -1070,6 +1070,7 @@ impl TryFrom<&BuiltinScalarFunction> for protobuf::ScalarFunction {
             BuiltinScalarFunction::RegexpMatch => Self::RegexpMatch,
             BuiltinScalarFunction::Coalesce => Self::Coalesce,
             BuiltinScalarFunction::Power => Self::Power,
+            BuiltinScalarFunction::Struct => Self::StructFun,
         };
 
         Ok(scalar_function)

From 5e1abbcd04848107590d95a0d41fb369629bed6d Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Tue, 3 May 2022 16:26:12 +0800
Subject: [PATCH 2/9] fix comment

---
 datafusion/core/tests/sql/expr.rs             | 10 +++----
 datafusion/expr/src/function.rs               | 10 +++----
 .../physical-expr/src/struct_expressions.rs   | 29 ++++++++++---------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/datafusion/core/tests/sql/expr.rs b/datafusion/core/tests/sql/expr.rs
index e6921ae57a56..793459f66849 100644
--- a/datafusion/core/tests/sql/expr.rs
+++ b/datafusion/core/tests/sql/expr.rs
@@ -514,14 +514,14 @@ async fn test_array_literals() -> Result<()> {
 async fn test_struct_literals() -> Result<()> {
     test_expression!(
         "STRUCT(1,2,3,4,5)",
-        "{\"f_0\": 1, \"f_1\": 2, \"f_2\": 3, \"f_3\": 4, \"f_4\": 5}"
+        "{\"c_0\": 1, \"c_1\": 2, \"c_2\": 3, \"c_3\": 4, \"c_4\": 5}"
     );
-    test_expression!("STRUCT(Null)", "{\"f_0\": null}");
-    test_expression!("STRUCT(1,Null)", "{\"f_0\": \"1\", \"f_1\": null}");
-    test_expression!("STRUCT(true, false)", "{\"f_0\": true, \"f_1\": false}");
+    test_expression!("STRUCT(Null)", "{\"c_0\": null}");
+    test_expression!("STRUCT(1,Null)", "{\"c_0\": \"1\", \"c_1\": null}");
+    test_expression!("STRUCT(true, false)", "{\"c_0\": true, \"c_1\": false}");
     test_expression!(
         "STRUCT('str1', 'str2')",
-        "{\"f_0\": \"str1\", \"f_1\": \"str2\"}"
+        "{\"c_0\": \"str1\", \"c_1\": \"str2\"}"
     );
 
     Ok(())
diff --git a/datafusion/expr/src/function.rs b/datafusion/expr/src/function.rs
index 0abdbaec3a9b..19e486eb4b42 100644
--- a/datafusion/expr/src/function.rs
+++ b/datafusion/expr/src/function.rs
@@ -223,11 +223,11 @@ pub fn return_type(
         },
 
         BuiltinScalarFunction::Struct => {
-            let fields = input_expr_types
-                .iter()
-                .map(|x| Field::new("item", x.clone(), true))
-                .collect();
-            Ok(DataType::Struct(fields))
+            // let fields = input_expr_types
+            //     .iter()
+            //     .map(|x| Field::new("item", x.clone(), true))
+            //     .collect();
+            Ok(DataType::Struct(vec![]))
         }
 
         BuiltinScalarFunction::Abs
diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index b4186e0831e1..2f2110cc25d2 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -35,57 +35,58 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
         .iter()
         .enumerate()
         .map(|(i, arg)| -> (Field, ArrayRef) {
+            let field_name = format!("c_{}", i).clone();
             match arg.data_type() {
                 DataType::Utf8 => (
-                    Field::new(&*format!("f_{}", i), DataType::Utf8, true),
+                    Field::new(field_name.as_str(), DataType::Utf8, true),
                     arg.clone(),
                 ),
                 DataType::LargeUtf8 => (
-                    Field::new(&*format!("f_{}", i), DataType::LargeUtf8, true),
+                    Field::new(field_name.as_str(), DataType::LargeUtf8, true),
                     arg.clone(),
                 ),
                 DataType::Boolean => (
-                    Field::new(&*format!("f_{}", i), DataType::Boolean, true),
+                    Field::new(field_name.as_str(), DataType::Boolean, true),
                     arg.clone(),
                 ),
                 DataType::Float32 => (
-                    Field::new(&*format!("f_{}", i), DataType::Float64, true),
+                    Field::new(field_name.as_str(), DataType::Float64, true),
                     arg.clone(),
                 ),
                 DataType::Float64 => (
-                    Field::new(&*format!("f_{}", i), DataType::Float64, true),
+                    Field::new(field_name.as_str(), DataType::Float64, true),
                     arg.clone(),
                 ),
                 DataType::Int8 => (
-                    Field::new(&*format!("f_{}", i), DataType::Int8, true),
+                    Field::new(field_name.as_str(), DataType::Int8, true),
                     arg.clone(),
                 ),
                 DataType::Int16 => (
-                    Field::new(&*format!("f_{}", i), DataType::Int16, true),
+                    Field::new(field_name.as_str(), DataType::Int16, true),
                     arg.clone(),
                 ),
                 DataType::Int32 => (
-                    Field::new(&*format!("f_{}", i), DataType::Int32, true),
+                    Field::new(field_name.as_str(), DataType::Int32, true),
                     arg.clone(),
                 ),
                 DataType::Int64 => (
-                    Field::new(&*format!("f_{}", i), DataType::Int64, true),
+                    Field::new(field_name.as_str(), DataType::Int64, true),
                     arg.clone(),
                 ),
                 DataType::UInt8 => (
-                    Field::new(&*format!("f_{}", i), DataType::UInt8, true),
+                    Field::new(field_name.as_str(), DataType::UInt8, true),
                     arg.clone(),
                 ),
                 DataType::UInt16 => (
-                    Field::new(&*format!("f_{}", i), DataType::UInt16, true),
+                    Field::new(field_name.as_str(), DataType::UInt16, true),
                     arg.clone(),
                 ),
                 DataType::UInt32 => (
-                    Field::new(&*format!("f_{}", i), DataType::UInt32, true),
+                    Field::new(field_name.as_str(), DataType::UInt32, true),
                     arg.clone(),
                 ),
                 DataType::UInt64 => (
-                    Field::new(&*format!("f_{}", i), DataType::UInt64, true),
+                    Field::new(field_name.as_str(), DataType::UInt64, true),
                     arg.clone(),
                 ),
                 data_type => unimplemented!("struct not support {} type", data_type),
@@ -96,7 +97,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
     Ok(Arc::new(StructArray::from(vec)))
 }
 
-/// put values in an array.
+/// put values in a struct array.
 pub fn struct_expr(values: &[ColumnarValue]) -> Result<ColumnarValue> {
     let arrays: Vec<ArrayRef> = values
         .iter()

From b11f2ef7d3c39283c02d5a02b858d99fc79f94ba Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Tue, 3 May 2022 16:27:13 +0800
Subject: [PATCH 3/9] remove useless code

---
 datafusion/expr/src/function.rs | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/datafusion/expr/src/function.rs b/datafusion/expr/src/function.rs
index 19e486eb4b42..4c0494803354 100644
--- a/datafusion/expr/src/function.rs
+++ b/datafusion/expr/src/function.rs
@@ -222,13 +222,7 @@ pub fn return_type(
             _ => Ok(DataType::Float64),
         },
 
-        BuiltinScalarFunction::Struct => {
-            // let fields = input_expr_types
-            //     .iter()
-            //     .map(|x| Field::new("item", x.clone(), true))
-            //     .collect();
-            Ok(DataType::Struct(vec![]))
-        }
+        BuiltinScalarFunction::Struct => Ok(DataType::Struct(vec![])),
 
         BuiltinScalarFunction::Abs
         | BuiltinScalarFunction::Acos

From 5b675a02855843a66fa5b0ac76eac26c5d03adc1 Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Tue, 3 May 2022 17:19:41 +0800
Subject: [PATCH 4/9] fix fmt

---
 datafusion/physical-expr/src/struct_expressions.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index 2f2110cc25d2..c4273fd55e0a 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -35,7 +35,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
         .iter()
         .enumerate()
         .map(|(i, arg)| -> (Field, ArrayRef) {
-            let field_name = format!("c_{}", i).clone();
+            let field_name = format!("c_{}", i);
             match arg.data_type() {
                 DataType::Utf8 => (
                     Field::new(field_name.as_str(), DataType::Utf8, true),

From f94ecee385d26d3b2f4b195fe5ff7141ba534565 Mon Sep 17 00:00:00 2001
From: Yang Jiang <37145547+Ted-Jiang@users.noreply.github.com>
Date: Thu, 5 May 2022 22:38:32 +0800
Subject: [PATCH 5/9] Update datafusion/physical-expr/src/struct_expressions.rs

Co-authored-by: Andy Grove <andygrove73@gmail.com>
---
 datafusion/physical-expr/src/struct_expressions.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index c4273fd55e0a..078ccc3b2215 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -35,7 +35,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
         .iter()
         .enumerate()
         .map(|(i, arg)| -> (Field, ArrayRef) {
-            let field_name = format!("c_{}", i);
+            let field_name = format!("c{}", i);
             match arg.data_type() {
                 DataType::Utf8 => (
                     Field::new(field_name.as_str(), DataType::Utf8, true),

From 7c4e878efd92b76fa6c5314a33fd6e1341936629 Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Thu, 5 May 2022 22:42:40 +0800
Subject: [PATCH 6/9] fix

---
 datafusion/core/tests/sql/expr.rs             | 10 +--
 .../physical-expr/src/struct_expressions.rs   | 66 +++++--------------
 2 files changed, 20 insertions(+), 56 deletions(-)

diff --git a/datafusion/core/tests/sql/expr.rs b/datafusion/core/tests/sql/expr.rs
index 793459f66849..4e840697641f 100644
--- a/datafusion/core/tests/sql/expr.rs
+++ b/datafusion/core/tests/sql/expr.rs
@@ -514,14 +514,14 @@ async fn test_array_literals() -> Result<()> {
 async fn test_struct_literals() -> Result<()> {
     test_expression!(
         "STRUCT(1,2,3,4,5)",
-        "{\"c_0\": 1, \"c_1\": 2, \"c_2\": 3, \"c_3\": 4, \"c_4\": 5}"
+        "{\"c0\": 1, \"c1\": 2, \"c2\": 3, \"c3\": 4, \"c4\": 5}"
     );
-    test_expression!("STRUCT(Null)", "{\"c_0\": null}");
-    test_expression!("STRUCT(1,Null)", "{\"c_0\": \"1\", \"c_1\": null}");
-    test_expression!("STRUCT(true, false)", "{\"c_0\": true, \"c_1\": false}");
+    test_expression!("STRUCT(Null)", "{\"c0\": null}");
+    test_expression!("STRUCT(1,Null)", "{\"c0\": \"1\", \"c1\": null}");
+    test_expression!("STRUCT(true, false)", "{\"c0\": true, \"c1\": false}");
     test_expression!(
         "STRUCT('str1', 'str2')",
-        "{\"c_0\": \"str1\", \"c_1\": \"str2\"}"
+        "{\"c0\": \"str1\", \"c1\": \"str2\"}"
     );
 
     Ok(())
diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index c4273fd55e0a..8881c3ec2ac3 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -35,58 +35,22 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
         .iter()
         .enumerate()
         .map(|(i, arg)| -> (Field, ArrayRef) {
-            let field_name = format!("c_{}", i);
+            let field_name = format!("c{}", i);
             match arg.data_type() {
-                DataType::Utf8 => (
-                    Field::new(field_name.as_str(), DataType::Utf8, true),
-                    arg.clone(),
-                ),
-                DataType::LargeUtf8 => (
-                    Field::new(field_name.as_str(), DataType::LargeUtf8, true),
-                    arg.clone(),
-                ),
-                DataType::Boolean => (
-                    Field::new(field_name.as_str(), DataType::Boolean, true),
-                    arg.clone(),
-                ),
-                DataType::Float32 => (
-                    Field::new(field_name.as_str(), DataType::Float64, true),
-                    arg.clone(),
-                ),
-                DataType::Float64 => (
-                    Field::new(field_name.as_str(), DataType::Float64, true),
-                    arg.clone(),
-                ),
-                DataType::Int8 => (
-                    Field::new(field_name.as_str(), DataType::Int8, true),
-                    arg.clone(),
-                ),
-                DataType::Int16 => (
-                    Field::new(field_name.as_str(), DataType::Int16, true),
-                    arg.clone(),
-                ),
-                DataType::Int32 => (
-                    Field::new(field_name.as_str(), DataType::Int32, true),
-                    arg.clone(),
-                ),
-                DataType::Int64 => (
-                    Field::new(field_name.as_str(), DataType::Int64, true),
-                    arg.clone(),
-                ),
-                DataType::UInt8 => (
-                    Field::new(field_name.as_str(), DataType::UInt8, true),
-                    arg.clone(),
-                ),
-                DataType::UInt16 => (
-                    Field::new(field_name.as_str(), DataType::UInt16, true),
-                    arg.clone(),
-                ),
-                DataType::UInt32 => (
-                    Field::new(field_name.as_str(), DataType::UInt32, true),
-                    arg.clone(),
-                ),
-                DataType::UInt64 => (
-                    Field::new(field_name.as_str(), DataType::UInt64, true),
+                DataType::Utf8
+                | DataType::LargeUtf8
+                | DataType::Boolean
+                | DataType::Float32
+                | DataType::Float64
+                | DataType::Int8
+                | DataType::Int16
+                | DataType::Int32
+                | DataType::Int64
+                | DataType::UInt8
+                | DataType::UInt16
+                | DataType::UInt32
+                | DataType::UInt64 => (
+                    Field::new(field_name.as_str(), arg.data_type().clone(), true),
                     arg.clone(),
                 ),
                 data_type => unimplemented!("struct not support {} type", data_type),

From 6ac645cc74578613f5b1174805fb91776e477d9b Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Sat, 7 May 2022 13:53:53 +0800
Subject: [PATCH 7/9] use error instead of panic

---
 datafusion/physical-expr/src/struct_expressions.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index 8881c3ec2ac3..1333cad7ac67 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -34,7 +34,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
     let vec: Vec<_> = args
         .iter()
         .enumerate()
-        .map(|(i, arg)| -> (Field, ArrayRef) {
+        .map(|(i, arg)| -> Result<(Field, ArrayRef)> {
             let field_name = format!("c{}", i);
             match arg.data_type() {
                 DataType::Utf8
@@ -49,13 +49,17 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
                 | DataType::UInt8
                 | DataType::UInt16
                 | DataType::UInt32
-                | DataType::UInt64 => (
+                | DataType::UInt64 => Ok((
                     Field::new(field_name.as_str(), arg.data_type().clone(), true),
                     arg.clone(),
-                ),
-                data_type => unimplemented!("struct not support {} type", data_type),
+                )),
+                data_type => Err(DataFusionError::NotImplemented(format!(
+                    "Struct is not implemented for type '{:?}'.",
+                    data_type
+                ))),
             }
         })
+        .map(|x| x.unwrap())
         .collect();
 
     Ok(Arc::new(StructArray::from(vec)))

From 9fb17a5af69737e0c28c69b7e8fa8a7c0cb30b31 Mon Sep 17 00:00:00 2001
From: yangjiang <yangjiang@ebay.com>
Date: Sat, 7 May 2022 18:26:22 +0800
Subject: [PATCH 8/9] fix ut

---
 datafusion/core/tests/sql/expr.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/datafusion/core/tests/sql/expr.rs b/datafusion/core/tests/sql/expr.rs
index 4e840697641f..b8a7e46bcb2c 100644
--- a/datafusion/core/tests/sql/expr.rs
+++ b/datafusion/core/tests/sql/expr.rs
@@ -517,7 +517,8 @@ async fn test_struct_literals() -> Result<()> {
         "{\"c0\": 1, \"c1\": 2, \"c2\": 3, \"c3\": 4, \"c4\": 5}"
     );
     test_expression!("STRUCT(Null)", "{\"c0\": null}");
-    test_expression!("STRUCT(1,Null)", "{\"c0\": \"1\", \"c1\": null}");
+    test_expression!("STRUCT(2)", "{\"c0\": 2}");
+    test_expression!("STRUCT('1',Null)", "{\"c0\": \"1\", \"c1\": null}");
     test_expression!("STRUCT(true, false)", "{\"c0\": true, \"c1\": false}");
     test_expression!(
         "STRUCT('str1', 'str2')",

From 73a89014b7870bcf17b6847657ff388c718ed1bb Mon Sep 17 00:00:00 2001
From: Yang Jiang <37145547+Ted-Jiang@users.noreply.github.com>
Date: Sun, 8 May 2022 10:48:31 +0800
Subject: [PATCH 9/9] fix

Co-authored-by: Andy Grove <andygrove73@gmail.com>
---
 datafusion/physical-expr/src/struct_expressions.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/datafusion/physical-expr/src/struct_expressions.rs b/datafusion/physical-expr/src/struct_expressions.rs
index 1333cad7ac67..c749038a82e7 100644
--- a/datafusion/physical-expr/src/struct_expressions.rs
+++ b/datafusion/physical-expr/src/struct_expressions.rs
@@ -59,8 +59,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
                 ))),
             }
         })
-        .map(|x| x.unwrap())
-        .collect();
+        .collect::<Result<Vec<_>>>()?;
 
     Ok(Arc::new(StructArray::from(vec)))
 }
