mingmwang commented on code in PR #2168:
URL: https://github.com/apache/arrow-datafusion/pull/2168#discussion_r844527086
##########
ballista/rust/core/src/serde/mod.rs:
##########
@@ -508,15 +508,10 @@ mod tests {
&self,
children: Vec<Arc<dyn ExecutionPlan>>,
) -> datafusion::error::Result<Arc<dyn ExecutionPlan>> {
- match children.len() {
- 1 => Ok(Arc::new(TopKExec {
- input: children[0].clone(),
- k: self.k,
- })),
- _ => Err(DataFusionError::Internal(
- "TopKExec wrong number of children".to_string(),
- )),
- }
+ Ok(Arc::new(TopKExec {
Review Comment:
I removed the length check in the `with_new_children()` method and move the
check to `with_new_children_if_necessary()` so that we can avoid the duplicate
logic in all the trait implementations. In future if someone misuse this and
still call the `with_new_children()` and pass the wrong children param, it will
lose the check, but if this is the case, the real problem is he should not
call `with_new_children()` and everyone should call
`with_new_children_if_necessary()`.
--
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]
