findepi commented on code in PR #17449:
URL: https://github.com/apache/datafusion/pull/17449#discussion_r2335832348


##########
datafusion/physical-plan/src/union.rs:
##########
@@ -916,4 +924,56 @@ mod tests {
             assert!(lhs_orderings.contains(rhs_ordering), "{}", err_msg);
         }
     }
+
+    #[test]
+    fn test_union_empty_inputs() {
+        // Test that UnionExec::new fails with empty inputs
+        let result = UnionExec::new(vec![]);
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("UnionExec requires at least one input"));
+    }
+
+    #[test]
+    fn test_union_schema_empty_inputs() {
+        // Test that union_schema fails with empty inputs
+        let result = union_schema(&[]);
+        assert!(result.is_err());
+        assert!(result
+            .unwrap_err()
+            .to_string()
+            .contains("Cannot create union schema from empty inputs"));
+    }
+
+    #[test]
+    fn test_union_single_input() -> Result<()> {
+        // Test that UnionExec works with a single input
+        let schema = create_test_schema()?;
+        let memory_exec = Arc::new(TestMemoryExec::try_new(&[], 
schema.clone(), None)?);
+        let union = UnionExec::new(vec![memory_exec])?;
+        
+        // Check that schema is correct
+        assert_eq!(union.schema(), schema);
+        
+        Ok(())
+    }
+
+    #[test]
+    fn test_union_multiple_inputs_still_works() -> Result<()> {

Review Comment:
   ```suggestion
       fn test_union_schema_multiple_inputs() -> Result<()> {
   ```



##########
datafusion/physical-plan/src/union.rs:
##########
@@ -101,19 +101,23 @@ pub struct UnionExec {
 
 impl UnionExec {
     /// Create a new UnionExec
-    pub fn new(inputs: Vec<Arc<dyn ExecutionPlan>>) -> Self {
-        let schema = union_schema(&inputs);
+    pub fn new(inputs: Vec<Arc<dyn ExecutionPlan>>) -> Result<Self> {

Review Comment:
   Good point on the API lifecycle.  On separate note, can we make the new 
`try_new` method return `Box<<dyn ExecutionPlan>>`? This would allow it to 
return the only child in case input vector is a singleton. There is no point 
keeping `UnionExec(a)` in the plan.
   Or maybe, the new method can simply require the input to have at least two 
elements?



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to