alamb commented on code in PR #17449:
URL: https://github.com/apache/datafusion/pull/17449#discussion_r2326058904
##########
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:
this is technically an API change -- maybe to make it easier on others, we
can make a new function called `try_new` that has the error checking, and
deprecate the existing `new` function per
https://datafusion.apache.org/contributor-guide/api-health.html#deprecation-guidelines
##########
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());
Review Comment:
I think the assertion check for is_err is redundant as `unwrap_err` will
panic if result is not an err
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
