quanghgx commented on PR #38390: URL: https://github.com/apache/arrow/pull/38390#issuecomment-1807137428
Thank you, @mapleFU, for providing a clear and detailed guideline. I've applied these changes and added back two simple tests. I have one final comment regarding the `WriteTable` function: We currently have two options for handling errors in this function: * (A.) We can let `writer->WriteTable` throw a `ParquetException`. This is because, in [this part of the code](https://github.com/apache/arrow/blob/2ec5d63277e559607cf32a901c2969be33cdfcb4/cpp/src/parquet/arrow/writer.cc#L375), it calls this->properties, which directly returns a `ParquetException` from `writer_`. * (B.) Alternatively, we can add `RETURN_NOT_OK(CheckClosed());` to the beginning of `WriteTable`, similar to `NewRowGroup` and `NewBufferedRowGroup`. This would consistently return `arrow::Status::Invalid`. I personally lean towards option (B.) for the sake of consistency. However, considering that use-after-close is not a typical flow (usually requiring user code revision to fully handle this case), using an exception might be a viable option here. This part is making me uncertain, so I would appreciate your input on this matter. Thanks mappleFU. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
