-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add into_primitive_dict_builder
to DictionaryArray
#3715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Thank you @viirya
let key_array = as_primitive_array::<K>(self.keys()).clone(); | ||
let value_array = as_primitive_array::<V>(self.values()).clone(); | ||
|
||
drop(self.data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed to drop the ref counts to be able to call into_builder()
without copying data (as self has a ref count)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, this drops self ref count.
match err { | ||
Ok(_) => panic!("Should not get builder from cloned array"), | ||
Err(returned) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also write this like let returned = err.unwrap_err()
fa623d7
to
b8dbf61
Compare
Benchmark runs are scheduled for baseline = e52574c and contender = 72474a6. 72474a6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #3710.
Rationale for this change
Similar to
into_builder
ofPrimitiveArray
,into_primitive_dict_builder
is for cow support forDictionaryArray
.With this API, it is easy to add
unary_dict_mut
.What changes are included in this PR?
Added
into_primitive_dict_builder
API toDictionaryArray
for mutating its keys and values.Are there any user-facing changes?