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. |
jiacai2050
changed the title
parquet/src/file/writer.rs
Outdated
| 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>
parquet/src/file/writer.rs
Outdated
| @@ -298,7 +301,11 @@ impl<W: Write> SerializedFileWriter<W> { | |||
| row_groups, | |||
| 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