Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Nov 27, 2024

Which issue does this PR close?

Rationale for this change

While reviewing #13564 (review) @jayzhan211 pointed out this function wasn't necessary as there is now equivalent functionality in arrow-rs

It is confusing there are two patterns to do the same thing, so let's remove the DataFusion specific pattern

What changes are included in this PR?

  1. Change code to use PrimitiveArray::with_data_type instead
  2. Deprecate adjust_output_array

Are these changes tested?

By CI

Are there any user-facing changes?

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Nov 27, 2024
@alamb alamb force-pushed the alamb/deprecate_adjust_datatype branch from 4ec0a12 to 8bb087c Compare November 27, 2024 21:54
@jayzhan211 jayzhan211 merged commit 9c6c1e1 into apache:main Nov 28, 2024
25 checks passed
@jayzhan211
Copy link
Contributor

Thanks @alamb

let vals = Arc::new(PrimitiveArray::<VAL>::from_iter_values(vals));
let vals = adjust_output_array(&self.data_type, vals).expect("Type is incorrect");
(vals, map_idxs)
let arr = PrimitiveArray::<VAL>::new(ScalarBuffer::from(vals), nulls)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will also be slightly faster as the ScalarBuffer::from avoids copying the values into the Vec: https://docs.rs/arrow/latest/arrow/buffer/struct.ScalarBuffer.html#impl-From%3CVec%3CT%3E%3E-for-ScalarBuffer%3CT%3E

@alamb alamb deleted the alamb/deprecate_adjust_datatype branch November 28, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants