[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7461: Implement flatten for MakeArray

["alamb (via GitHub)" <gi...@apache.org>]

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


##########
datafusion/expr/src/expr.rs:
##########
@@ -1030,6 +1031,47 @@ impl Expr {
     pub fn contains_outer(&self) -> bool {
         !find_out_reference_exprs(self).is_empty()
     }
+
+    /// Flatten the nested array expressions until the base array is reached.
+    /// For example:
+    /// [[1, 2, 3], [4, 5, 6]] => [1, 2, 3, 4, 5, 6]
+    /// [[[1, 2], [3, 4]], [[5, 6], [7, 8]]] => [1, 2, 3, 4, 5, 6, 7, 8]
+    /// Panics if the expression cannot be flattened.
+    pub fn flatten(&self) -> Self {
+        self.try_flatten().unwrap_or_else(|_| self.clone())
+    }
+
+    /// Flatten the nested array expressions until the base array is reached.
+    /// For example:
+    /// [[1, 2, 3], [4, 5, 6]] => [1, 2, 3, 4, 5, 6]
+    /// [[[1, 2], [3, 4]], [[5, 6], [7, 8]]] => [1, 2, 3, 4, 5, 6, 7, 8]
+    /// Returns an error if the expression cannot be flattened.
+    pub fn try_flatten(&self) -> Result<Self> {

Review Comment:
   This function appears to be trying to simplify the `flatten` function by partially evaluating a `make_array`. Given that I think this kind of logic is likely better in `https://github.com/apache/arrow-datafusion/blob/main/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs` or something similar. 
   
   In fact, I think since we have a `flatten` function already: https://github.com/apache/arrow-datafusion/blob/87527c430852af82ef182e1607804f6ddd817bbb/datafusion/physical-expr/src/array_expressions.rs#L1662-L1665
   
   Such an expression will already be automatically handled by expression simplification
   
   
   
   
   



##########
datafusion/expr/src/expr.rs:
##########
@@ -1030,6 +1031,47 @@ impl Expr {
     pub fn contains_outer(&self) -> bool {
         !find_out_reference_exprs(self).is_empty()
     }
+
+    /// Flatten the nested array expressions until the base array is reached.
+    /// For example:
+    /// [[1, 2, 3], [4, 5, 6]] => [1, 2, 3, 4, 5, 6]
+    /// [[[1, 2], [3, 4]], [[5, 6], [7, 8]]] => [1, 2, 3, 4, 5, 6, 7, 8]
+    /// Panics if the expression cannot be flattened.

Review Comment:
   this code doesn't seem to panic -- instead it ignores errors



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org