API to help with the pattern of 'replaces the values of the REE array#9891
API to help with the pattern of 'replaces the values of the REE array#9891Rich-T-kid wants to merge 2 commits intoapache:mainfrom
Conversation
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks good to me -- thank you @Rich-T-kid
I have a sugestion on how to make this slightly more efficient by reusing the field -- but that can be done as a follow on PR too potentially
| let new_array = array.with_values(values); | ||
| Ok(new_array) | ||
| } | ||
| DataType::RunEndEncoded(k, _) => match k.data_type() { |
There was a problem hiding this comment.
This is unrelated to adding with_values, right? It is adding REE support to date_part 🤔
There was a problem hiding this comment.
Yea I noticed it while I was going through spots in the codebase that used this pattern. its was a pretty small change so I included it in this PR
|
@alamb I revised the PR. Thank you 😄 |
Rich-T-kid
force-pushed
the
rich-T-kid/REE-values-take
branch
from
0bc52ac to
70e5f4e
Compare
| /// This macro is useful for implementing functions that should work on the logical values | ||
| /// of a REE array while preserving the run-end encoding structure. | ||
| #[macro_export] | ||
| macro_rules! ree_map { |
There was a problem hiding this comment.
What about having a AnyRunArray trait like we do for AnyDictionaryArray? This way users can downcast to AnyRunArray instead of needing to cast to each individual key type which the macro still requires
3 participants
Which issue does this PR close?
Rationale for this change
over time the pattern of replacing REE values without creating a new instance has emerged, buts approached have been scatered. this PR attempts to consolidate them into a single place.
What changes are included in this PR?
Includes updating call sites that used to manually replace REE values by constructing new ones, switching them to use a macro instead.
Are these changes tested?
yes
Are there any user-facing changes?
no