martin-g commented on code in PR #18790:
URL: https://github.com/apache/datafusion/pull/18790#discussion_r2536598979
##########
datafusion/proto/src/logical_plan/mod.rs:
##########
@@ -776,11 +776,10 @@ impl AsLogicalPlan for LogicalPlanNode {
builder.build()
}
LogicalPlanType::Union(union) => {
- if union.inputs.len() < 2 {
- return internal_err!(
- "Protobuf deserialization error, Union was require at
least two input."
- );
- }
+ assert_or_internal_err!(
+ union.inputs.len() >= 2,
+ "Protobuf deserialization error, Union was require at
least two input."
Review Comment:
```suggestion
"Protobuf deserialization error, Union requires at least
two inputs."
```
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -1372,9 +1372,7 @@ fn insert_below(
})?;
// make sure we did the actual replacement
- if new_child.is_some() {
- return internal_err!("node had no inputs");
- }
+ assert_or_internal_err!(new_child.is_none(), "node had no inputs");
Review Comment:
```suggestion
assert_or_internal_err!(new_child.is_none(), "node had no inputs");
```
##########
datafusion/spark/src/function/datetime/make_interval.rs:
##########
@@ -533,34 +535,33 @@ mod tests {
.ok_or_else(|| {
internal_datafusion_err!("expected
IntervalMonthDayNanoArray")
})?;
- if arr.len() != number_rows {
- return internal_err!(
- "expected array length {number_rows}, got {}",
- arr.len()
- );
- }
+ assert_eq_or_internal_err!(
+ arr.len(),
+ number_rows,
+ "expected array length {number_rows}"
+ );
for i in 0..number_rows {
let iv = arr.value(i);
- if (iv.months, iv.days, iv.nanoseconds) != (0, 0, 0) {
- return internal_err!(
- "row {i}: expected (0,0,0), got ({},{},{})",
- iv.months,
- iv.days,
- iv.nanoseconds
- );
- }
- }
- }
- ColumnarValue::Scalar(ScalarValue::IntervalMonthDayNano(Some(iv)))
=> {
- if (iv.months, iv.days, iv.nanoseconds) != (0, 0, 0) {
- return internal_err!(
- "expected scalar 0s, got ({},{},{})",
+ assert_eq_or_internal_err!(
+ (iv.months, iv.days, iv.nanoseconds),
+ (0, 0, 0),
+ "row {i}: expected (0,0,0), got ({},{},{})",
Review Comment:
nit: assert_eq_or_internal_err() already prints the left and right values
(https://github.com/apache/datafusion/blob/0304cda4fb18fd0555f5499a07a34f53b3cdf613/datafusion/common/src/error.rs#L838),
so this will duplicate this information.
##########
datafusion/spark/src/function/datetime/make_interval.rs:
##########
@@ -533,34 +535,33 @@ mod tests {
.ok_or_else(|| {
internal_datafusion_err!("expected
IntervalMonthDayNanoArray")
})?;
- if arr.len() != number_rows {
- return internal_err!(
- "expected array length {number_rows}, got {}",
- arr.len()
- );
- }
+ assert_eq_or_internal_err!(
+ arr.len(),
+ number_rows,
+ "expected array length {number_rows}"
+ );
for i in 0..number_rows {
let iv = arr.value(i);
- if (iv.months, iv.days, iv.nanoseconds) != (0, 0, 0) {
- return internal_err!(
- "row {i}: expected (0,0,0), got ({},{},{})",
- iv.months,
- iv.days,
- iv.nanoseconds
- );
- }
- }
- }
- ColumnarValue::Scalar(ScalarValue::IntervalMonthDayNano(Some(iv)))
=> {
- if (iv.months, iv.days, iv.nanoseconds) != (0, 0, 0) {
- return internal_err!(
- "expected scalar 0s, got ({},{},{})",
+ assert_eq_or_internal_err!(
+ (iv.months, iv.days, iv.nanoseconds),
+ (0, 0, 0),
+ "row {i}: expected (0,0,0), got ({},{},{})",
iv.months,
iv.days,
iv.nanoseconds
);
}
}
+ ColumnarValue::Scalar(ScalarValue::IntervalMonthDayNano(Some(iv)))
=> {
+ assert_eq_or_internal_err!(
+ (iv.months, iv.days, iv.nanoseconds),
+ (0, 0, 0),
+ "expected scalar 0s, got ({},{},{})",
Review Comment:
Same here
--
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]
