-
Notifications
You must be signed in to change notification settings - Fork 875
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
expose row-group flush in public api #1634
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.
Some minor nits, otherwise looks good to me. Thank you 👍
parquet/src/arrow/arrow_writer.rs
Outdated
@@ -192,8 +196,8 @@ impl<W: 'static + ParquetWriter> ArrowWriter<W> { | |||
|
|||
/// Close and finalize the underlying Parquet writer | |||
pub fn close(&mut self) -> Result<parquet_format::FileMetaData> { | |||
self.flush_completed()?; | |||
self.flush_row_group(self.buffered_rows)?; | |||
self.flush_excess()?; |
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.
I know this existed before, but I think it is actually redundant as write
calls flush_excess
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.
yep, It is redundant, I didn't want to remove that on my own because it could be intentional to have defensive approach when closing writer, but right now that seems unnecessary unless more options for write popup in ArrowWriter
Codecov Report
@@ Coverage Diff @@
## master #1634 +/- ##
=======================================
Coverage 83.02% 83.03%
=======================================
Files 193 193
Lines 55577 55578 +1
=======================================
+ Hits 46145 46147 +2
+ Misses 9432 9431 -1
Continue to review full report at Codecov.
|
Thanks again 👍 |
Which issue does this PR close?
Closes #1626
Rationale for this change
more power for the users to let them decide when to flush row group
Are there any user-facing changes?
not much, just one more method with single line body