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/26 15:29:32 UTC

[GitHub] [arrow] alamb opened a new pull request #8528: [Rust][Parquet] Ensure it is possible to create custom parquet writers

alamb opened a new pull request #8528:
URL: https://github.com/apache/arrow/pull/8528


   As of https://github.com/apache/arrow/pull/8300, it no longer appears to be able to implement a `ParquetWriter` to provide a custom writer because it is not possible to implement the trait as `TryClone` is not publically exported. 
   
   This PR publically exports that trait and adds an end to end test demonstrating it is possible to create a custom writer
   
   Here is what happens if you try and use `TryClone`: 
   
   {code}
   -*- mode: compilation; default-directory: "~/Software/delorean/" -*-
   Compilation started at Mon Oct 26 10:56:43
   
   cd /Users/alamb/Software/delorean  && cargo clippy --all-targets --workspace -- -D warnings
       Blocking waiting for file lock on build directory
       Checking delorean_parquet v0.1.0 (/Users/alamb/Software/delorean/delorean_parquet)
       Checking delorean_storage v0.1.0 (/Users/alamb/Software/delorean/delorean_storage)
   error[E0603]: module `util` is private
     --> delorean_parquet/src/writer.rs:11:5
      |
   11 |     util::io::TryClone,
      |     ^^^^ private module
      |
   note: the module `util` is defined here
     --> /Users/alamb/.cargo/git/checkouts/arrow-3a9cfebb6b7b2bdc/7155cd5/rust/parquet/src/lib.rs:39:1
      |
   39 | mod util;
      | ^^^^^^^^^
   
   {code}


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

[GitHub] [arrow] alamb edited a comment on pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
alamb edited a comment on pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#issuecomment-716743732


   > LGTM. @alamb `cargo fmt` pls?
   
   @jorgecarleitao 👍 
   ```
   alamb@MacBook-Pro rust % cargo fmt --all
   alamb@MacBook-Pro rust % git status
   On branch alamb/ARROW-10390-custom-parquet-writer
   Your branch is up to date with 'alamb/alamb/ARROW-10390-custom-parquet-writer'.
   
   nothing to commit, working tree clean
   alamb@MacBook-Pro rust % 
   ```


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

[GitHub] [arrow] nevi-me closed pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #8528:
URL: https://github.com/apache/arrow/pull/8528


   


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

[GitHub] [arrow] sunchao commented on a change in pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#discussion_r512158360



##########
File path: rust/parquet/tests/custom_writer.rs
##########
@@ -0,0 +1,83 @@
+use std::fs::File;

Review comment:
       nit: license header

##########
File path: rust/parquet/src/file/writer.rs
##########
@@ -38,7 +38,10 @@ use crate::file::{
     statistics::to_thrift as statistics_to_thrift, FOOTER_SIZE, PARQUET_MAGIC,
 };
 use crate::schema::types::{self, SchemaDescPtr, SchemaDescriptor, TypePtr};
-use crate::util::io::{FileSink, Position, TryClone};
+use crate::util::io::{FileSink, Position};
+
+// Exposed publically so client code can implement ParquetWriter

Review comment:
       nit: perhaps make ParquetWriter a linked text.




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

[GitHub] [arrow] nevi-me commented on a change in pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#discussion_r512063425



##########
File path: rust/parquet/src/file/writer.rs
##########
@@ -114,6 +114,7 @@ pub trait RowGroupWriter {
 // ----------------------------------------------------------------------
 // Serialized impl for file & row group writers
 
+pub use crate::util::io::TryClone;

Review comment:
       I'd prefer if the import is still close to line 41. Here it's hidden, especially as a `pub use`




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

[GitHub] [arrow] alamb commented on pull request #8528: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#issuecomment-716626529


   FYI @rdettai  / @sunchao 


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

[GitHub] [arrow] alamb commented on a change in pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#discussion_r512069291



##########
File path: rust/parquet/src/file/writer.rs
##########
@@ -114,6 +114,7 @@ pub trait RowGroupWriter {
 // ----------------------------------------------------------------------
 // Serialized impl for file & row group writers
 
+pub use crate::util::io::TryClone;

Review comment:
       moved!




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

[GitHub] [arrow] github-actions[bot] commented on pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#issuecomment-716631879


   https://issues.apache.org/jira/browse/ARROW-10390


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

[GitHub] [arrow] alamb commented on pull request #8528: ARROW-10390: [Rust][Parquet] Ensure it is possible to create custom parquet writers

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8528:
URL: https://github.com/apache/arrow/pull/8528#issuecomment-716743732


   > LGTM. @alamb `cargo fmt` pls?
   
   👍 
   ```
   alamb@MacBook-Pro rust % cargo fmt --all
   alamb@MacBook-Pro rust % git status
   On branch alamb/ARROW-10390-custom-parquet-writer
   Your branch is up to date with 'alamb/alamb/ARROW-10390-custom-parquet-writer'.
   
   nothing to commit, working tree clean
   alamb@MacBook-Pro rust % 
   ```


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