Make DecimalArray as PrimitiveArray#2857
Conversation
adeca0e to
de451bb
Compare
tustvold
left a comment
There was a problem hiding this comment.
Thank you for this, just got off a 26 hour flight so will review this more thoroughly once my brain is functioning again properly. Only major comment is regards to validation, which I think can be relaxed
There was a problem hiding this comment.
The move to PrimitiveArray implicitly means away from validating the contents for overflow, as any value of the underlying primitive is permitted.
I think we should therefore make this safe, and remove any value validation except for when this method is explicitly called by the user, as an opt-in
|
These clippy errors are not from this change but due to recent change on |
e909952 to
3c1c27e
Compare
|
cc @liukun4515 |
tustvold
left a comment
There was a problem hiding this comment.
I think this is a good improvement and brings us closer to the vision of #2857. I think we should get this in, to start the process of updating the dependencies (something I fully intend to help out with), and continue to refine this in subsequent PRs.
I've taken the liberty of resolving the merge conflicts
|
Benchmark runs are scheduled for baseline = fa1d079 and contender = eeb1261. eeb1261 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
DataFusion upgrade PR - apache/datafusion#3844 |
Which issue does this PR close?
Part of #2637.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?