[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2955: Update to arrow `19.0.0`

[GitBox <gi...@apache.org>]

alamb commented on code in PR #2955:
URL: https://github.com/apache/arrow-datafusion/pull/2955#discussion_r931329072


##########
datafusion/common/src/scalar.rs:
##########
@@ -917,6 +917,11 @@ impl ScalarValue {
                     ScalarValue::iter_to_decimal_array(scalars, precision, scale)?;
                 Arc::new(decimal_array)
             }
+            DataType::Decimal256(_, _) => {

Review Comment:
   https://github.com/apache/arrow-rs/pull/2094 🎉 



##########
datafusion/common/src/scalar.rs:
##########
@@ -527,15 +527,15 @@ macro_rules! build_values_list {
             for scalar_value in $VALUES {
                 match scalar_value {
                     ScalarValue::$SCALAR_TY(Some(v)) => {
-                        builder.values().append_value(v.clone()).unwrap()
+                        builder.values().append_value(v.clone());

Review Comment:
   Many of the changes in this PR are driven by https://github.com/apache/arrow-rs/pull/2103 (that made the builders to infallible)



##########
datafusion/physical-expr/src/expressions/cast.rs:
##########
@@ -271,12 +271,18 @@ mod tests {
 
     #[test]
     fn test_cast_decimal_to_decimal() -> Result<()> {
-        let array = vec![1234, 2222, 3, 4000, 5000];
+        let array = vec![

Review Comment:
   This was a bug in the test -- the input data had 5 rows but the expected value had 6 values (and apparently `is_valid()` used to return `false` for an index off the end of the array and in arrow 19 it returns `true`)



##########
datafusion/common/src/scalar.rs:
##########
@@ -1112,14 +1117,14 @@ impl ScalarValue {
         scalars: impl IntoIterator<Item = ScalarValue>,
         precision: &usize,
         scale: &usize,
-    ) -> Result<DecimalArray> {
+    ) -> Result<Decimal128Array> {

Review Comment:
   `DecimalArray` now is `DecimalArray128` to add support of `DecimalArray256` -- in https://github.com/apache/arrow-rs/issues/2101



##########
datafusion/core/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -130,58 +130,52 @@ impl<'a, R: Read> AvroArrowArrayReader<'a, R> {
         arrays.and_then(|arr| RecordBatch::try_new(projected_schema, arr).map(Some))
     }
 
-    fn build_boolean_array(
-        &self,
-        rows: RecordSlice,
-        col_name: &str,
-    ) -> ArrowResult<ArrayRef> {
+    fn build_boolean_array(&self, rows: RecordSlice, col_name: &str) -> ArrayRef {

Review Comment:
   This is another example showing the builders have become infallable



-- 
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-unsubscribe@arrow.apache.org

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