jiangzhx commented on code in PR #5585:
URL: https://github.com/apache/arrow-datafusion/pull/5585#discussion_r1136557850


##########
datafusion/core/src/dataframe.rs:
##########
@@ -437,19 +416,27 @@ impl DataFrame {
         ))];
         for field in original_schema_fields {
             let mut array_datas = vec![];
-            for record_batch in describe_record_batch.iter() {
-                // safe unwrap since aggregate record batches should have at 
least 1 record
-                let column = 
record_batch.get(0).unwrap().column_by_name(field.name());
-                match column {
-                    Some(c) => {
-                        if field.data_type().is_numeric() {
-                            array_datas.push(cast(c, &DataType::Float64)?);
-                        } else {
-                            array_datas.push(cast(c, &DataType::Utf8)?);
+            for result in describe_record_batch.iter() {
+                match result {
+                    Ok(df) => {
+                        let record_batch = df.clone().collect().await?;
+                        if record_batch.len() == 1 {

Review Comment:
   Thanks, I made some changes. If you have time, could you please help with 
another code review?



##########
datafusion/core/src/dataframe.rs:
##########
@@ -437,19 +416,27 @@ impl DataFrame {
         ))];
         for field in original_schema_fields {
             let mut array_datas = vec![];
-            for record_batch in describe_record_batch.iter() {
-                // safe unwrap since aggregate record batches should have at 
least 1 record
-                let column = 
record_batch.get(0).unwrap().column_by_name(field.name());
-                match column {
-                    Some(c) => {
-                        if field.data_type().is_numeric() {
-                            array_datas.push(cast(c, &DataType::Float64)?);
-                        } else {
-                            array_datas.push(cast(c, &DataType::Utf8)?);
+            for result in describe_record_batch.iter() {
+                match result {
+                    Ok(df) => {
+                        let record_batch = df.clone().collect().await?;
+                        if record_batch.len() == 1 {

Review Comment:
   > Thiking, if the code can be simplified a bit, now the code looks 
overloaded with conditions.
   
   Thanks, I made some changes. If you have time, could you please help with 
another code review?



-- 
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