You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/13 22:17:43 UTC

[GitHub] [arrow] alamb edited a comment on pull request #8300: ARROW-10135: [Rust] [Parquet] Refactor file module to help adding sources

alamb edited a comment on pull request #8300:
URL: https://github.com/apache/arrow/pull/8300#issuecomment-708039466


   @rdettai  --
   
   > This wrapper makes sense because File is cheap to clone and needs to be buffered, but this will not be generally the case for other implems of ParquetReader. 
   
   Yes, actually that was our experience (when we used a buffered implementation, we found that the underlying (large) buffer got copies around a lot. 
   
   I think in general, the cleaner thing (and my preference) would be to simply port my code to use `ChunkReader` (I think it will end up being cleaner and likely more performant). 
   
   > Do you think it is worth it have the ChunkReader be implemented for T:ParquetReader rather than File ?  
   
   The upside is that it would be somewhat more backwards compatible, but unless other reviewers feel strongly I personally think we should just do the breaking change, merge this PR, (maybe even remove `ParquetReader` too) and I'll rewrite our project in terms of `ChunkReader`


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org