feat: add append_key_value_metadata#3367
Conversation
|
Perhaps we could make this append, as opposed to replacing the key value metadata in WriterProperties? |
|
Make sense, will address this soon. |
tustvold
left a comment
There was a problem hiding this comment.
Just some minor nits, thank you
| key_value_metadata: self.props.key_value_metadata().cloned().map(|mut v| { | ||
| v.extend(self.kv_metadatas.clone()); | ||
| v | ||
| }), |
There was a problem hiding this comment.
| key_value_metadata: self.props.key_value_metadata().cloned().map(|mut v| { | |
| v.extend(self.kv_metadatas.clone()); | |
| v | |
| }), | |
| key_value_metadata: self.props.key_value_metadata().iter().chain(&self.kv_metadatas).cloned().collect(), |
Or something, perhaps?
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
| version: self.props.writer_version().as_num(), | ||
| schema: types::to_thrift(self.schema.as_ref())?, | ||
| key_value_metadata: self.props.key_value_metadata().cloned(), | ||
| key_value_metadata: self.props.key_value_metadata().cloned().map(|kv| { |
There was a problem hiding this comment.
This won't work correctly if props.key_value_metadata isn't present... Which also makes me realise this needs a test. I'll bash something out quickly
|
Benchmark runs are scheduled for baseline = 0f196b8 and contender = f521e11. f521e11 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?
Closes #3356.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?
Yes, add a new method in ArrowWriter:
append_key_value_metadata