apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
5.91k stars 1.12k forks source link

Review use of panic in `datafusion-physical-expr` crate #3316

Open andygrove opened 2 years ago

andygrove commented 2 years ago

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_distinct.rs":222             let binary_array = states[0].as_any().downcast_ref::<BinaryArray>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":275                 let array = values.as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":283                 let array = values.as_any().downcast_ref::<Float32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":291                 let array = values.as_any().downcast_ref::<Int64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":299                 let array = values.as_any().downcast_ref::<Int32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":307                 let array = values.as_any().downcast_ref::<Int16Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":315                 let array = values.as_any().downcast_ref::<Int8Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":323                 let array = values.as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":331                 let array = values.as_any().downcast_ref::<UInt32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":339                 let array = values.as_any().downcast_ref::<UInt16Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":347                 let array = values.as_any().downcast_ref::<UInt8Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs":396             v => unreachable!("unexpected return type {:?}", v),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/average.rs":172         let counts = states[0].as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/average.rs":248         let counts = states[0].as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/count.rs":129         let counts = states[0].as_any().downcast_ref::<Int64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/count.rs":176         let counts = states[0].as_any().downcast_ref::<Int64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/count_distinct.rs":194                 _ => unreachable!(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":256             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":262             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":281             let delta1 = value1.unwrap() - self.mean1;
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":283             let delta2 = value2.unwrap() - self.mean2;
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":285             let new_c = delta1 * (value2.unwrap() - new_mean2) + self.algo_const;
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":297         let counts = states[0].as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":298         let means1 = states[1].as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":299         let means2 = states[2].as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/covariance.rs":300         let cs = states[3].as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/min_max.rs":148         let array = $VALUES.as_any().downcast_ref::<$ARRAYTYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/min_max.rs":158         let array = $VALUES.as_any().downcast_ref::<$ARRAYTYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/min_max.rs":164         let array = $VALUES.as_any().downcast_ref::<$ARRAYTYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/min_max.rs":179             let array = $VALUES.as_any().downcast_ref::<Decimal128Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/mod.rs":100         unreachable!()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/sum.rs":147         let array = $VALUES.as_any().downcast_ref::<$ARRAYTYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/sum.rs":160     let array = values.as_any().downcast_ref::<Decimal128Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/sum_distinct.rs":175             sum_value = sum::sum(&sum_value, distinct_value).unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/tdigest.rs":45             v => panic!("invalid type {:?}", v),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/variance.rs":226             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/variance.rs":246         let counts = states[0].as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/variance.rs":247         let means = states[1].as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/aggregate/variance.rs":248         let m2s = states[2].as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/crypto_expressions.rs":232         write!(&mut s, "{:02x}", b).unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":233     Ok(value.unwrap().timestamp_nanos())
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":262                 .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":354                     .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":381                 let array = $ARRAY.as_any().downcast_ref::<Date32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":385                 let array = $ARRAY.as_any().downcast_ref::<Date64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":393                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":400                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":407                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs":414                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary/kernels.rs":33         let left = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary/kernels.rs":34         let right = $RIGHT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary/kernels.rs":55         let array = $LEFT.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary/kernels.rs":60             let right: $TYPE = scalar.try_into().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary.rs":120         let ll = $LEFT.as_any().downcast_ref::<$DT>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary.rs":130         let ll = $LEFT.as_any().downcast_ref::<$DT>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary.rs":131         let rr = $RIGHT.as_any().downcast_ref::<$DT>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary.rs":482             array = not(&array).unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/binary.rs":519                 array = not(&array).unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/case.rs":122         let expr = self.expr.as_ref().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/case.rs":336             let data_type = &when_then.1.data_type(input_schema).unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/case.rs":345         Some(else_phy_expr) => else_phy_expr.data_type(input_schema).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":137             let array = array.as_any().downcast_ref::<Date32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":139                 date32_add(days, scalar, sign).unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":143             let array = array.as_any().downcast_ref::<Date64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":145                 date64_add(ms, scalar, sign).unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":152                 .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":155                 |ts_s| seconds_add(ts_s, scalar, sign).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":162                 .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":166                     |ts_ms| milliseconds_add(ts_ms, scalar, sign).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":174                 .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":178                     |ts_us| microseconds_add(ts_us, scalar, sign).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":186                 .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/datetime.rs":190                     |ts_ns| nanoseconds_add(ts_ns, scalar, sign).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":67             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":69             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":71             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":74             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":76             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":78             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/delta.rs":80             .unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/get_indexed_field.rs":93                     array.as_any().downcast_ref::<ListArray>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/get_indexed_field.rs":118                     let iter = concat(vec.as_slice()).unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/get_indexed_field.rs":124                 let as_struct_array = array.as_any().downcast_ref::<StructArray>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":105         let array = $ARRAY.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":116                     datatype => unreachable!("InList can't reach other data type {} for {}.", datatype, s),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":119                     unimplemented!("InList does not yet support nested columns.")
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":130         let array = $ARRAY.as_any().downcast_ref::<$ARRAY_TYPE>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":141                     datatype => unreachable!("InList can't reach other data type {} for {}.", datatype, s),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":144                     unimplemented!("InList does not yet support nested columns.")
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":162                         match vop.map(|v| !$SET_VALUES.contains(&v.try_into().unwrap())) {
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":171                     .map(|vop| vop.map(|v| !$SET_VALUES.contains(&v.try_into().unwrap())))
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":180                         match vop.map(|v| $SET_VALUES.contains(&v.try_into().unwrap())) {
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":189                     .map(|vop| vop.map(|v| $SET_VALUES.contains(&v.try_into().unwrap())))
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":205                     unreachable!(
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":346                 datatype => unreachable!(
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":352                 unimplemented!("InList does not yet support nested columns.")
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":371                 unreachable!("InList can't reach other data type {} for {}.", datatype, v)
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":392                 unreachable!("InList can't reach other data type {} for {}.", datatype, v)
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":453             .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":466                     datatype => unimplemented!("Unexpected type {} for InList", datatype),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":469                     unimplemented!("InList does not yet support nested columns.")
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":550                     let array = array.as_any().downcast_ref::<BooleanArray>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":560                     let array = array.as_any().downcast_ref::<Int8Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":570                     let array = array.as_any().downcast_ref::<Int16Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":580                     let array = array.as_any().downcast_ref::<Int32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":590                     let array = array.as_any().downcast_ref::<Int64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":600                     let array = array.as_any().downcast_ref::<UInt8Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":610                     let array = array.as_any().downcast_ref::<UInt16Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":620                     let array = array.as_any().downcast_ref::<UInt32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":630                     let array = array.as_any().downcast_ref::<UInt64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":640                     let array = array.as_any().downcast_ref::<Float32Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":650                     let array = array.as_any().downcast_ref::<Float64Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":663                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":670                         .unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":674                     let array = array.as_any().downcast_ref::<Decimal128Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/in_list.rs":804                         array.as_any().downcast_ref::<Decimal128Array>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/expressions/literal.rs":80         _ => unreachable!(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/functions.rs":348                 _ => unreachable!(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/functions.rs":473                 _ => unreachable!(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/planner.rs":331         .map(|list_expr| list_expr.data_type(input_schema).unwrap())
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/planner.rs":345                     try_cast(list_expr, input_schema, data_type.clone()).unwrap()
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/regex_expressions.rs":72         static ref CAPTURE_GROUPS_RE: Regex = Regex::new(r"(\\)(\d*)").unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/string_expressions.rs":295                                 let v = v.as_any().downcast_ref::<StringArray>().unwrap();
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/string_expressions.rs":299                         _ => unreachable!(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/string_expressions.rs":317                     _ => unreachable!(""),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/string_expressions.rs":577             integer.map(|integer| format!("{:x}", integer.to_usize().unwrap()))
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/unicode_expressions.rs":181                                         *fill_chars.get(l % fill_chars.len()).unwrap(),
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/unicode_expressions.rs":310                                     .push(*fill_chars.get(l % fill_chars.len()).unwrap());
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/window/cume_dist.rs":81         unreachable!(
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/window/nth_value.rs":150         unreachable!("first, last, and nth_value evaluation must be called with evaluate_partition_with_rank")
"/home/andy/git/apache/arrow-datafusion/datafusion/physical-expr/src/window/rank.rs":115         unreachable!("rank evaluation must be called with evaluate_partition_with_rank")

Describe the solution you'd like Review code that can panic and see where it makes sense to return a Result instead. For example, It is generally better to use ? than unwrapon results.

The goal is not to remove all panics but review and make sure we are using them appropriately. Bonus points for adding documentation for invariants.

Describe alternatives you've considered None

Additional context Generated by https://github.com/andygrove/no-need-to-panic

iajoiner commented 2 years ago

I will take this one.

alamb commented 10 months ago

I think knowing when it is "ok" to panic takes some non trivial experience in arrow code, so I dont' think this is a good first issue.