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/05/14 09:23:06 UTC
[GitHub] [arrow] Marwes opened a new pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Marwes opened a new pull request #7176:
URL: https://github.com/apache/arrow/pull/7176
Allows parquet to be easily be written directly to RAM without needing a
file to roundtrip.
----------------------------------------------------------------
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] kszucs commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-654800827
Note that 1.0 release is about the format stability rather than API stability. Given the number of daily downloads from crates.io I don't think we should really worry about breaking the rust API, but slowly we should prefer proper depreciation cycles.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-773349999
We have an https://docs.rs/parquet/3.0.0/parquet/file/writer/struct.InMemoryWriteableCursor.html, which achieves a similar goal as this PR. I'm closing this instead.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-773349999
We have an https://docs.rs/parquet/3.0.0/parquet/file/writer/struct.InMemoryWriteableCursor.html, which achieves a similar goal as this PR. I'm closing this instead.
----------------------------------------------------------------
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 #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #7176:
URL: https://github.com/apache/arrow/pull/7176
----------------------------------------------------------------
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 edited a comment on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao edited a comment on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-632943566
add @sadikovi also who authored this part of 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] andygrove commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-669973140
Hi @Marwes ... just checking in on status. Are you able to make the proposed changes?
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-632943566
+ @sadikovi also who authored this part of 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] Marwes commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
Marwes commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-633604830
It is unfortunate that the API becomes a bit worse due to this but I can't think of a better way atm (there is probably a better way though). However being forced to use a file as an intermediate when all you want to is to write to memory is even more awkward, not to say extremely slow :(.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-635692656
@Marwes it's better to be cautious to break backward compatibility. Can you provide a code snippet for the use case you want to enable?
----------------------------------------------------------------
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] Marwes commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
Marwes commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-670161388
Sorry, I suspect I won't have time to do any further changes to this for at least a month.
----------------------------------------------------------------
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 #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-628516758
https://issues.apache.org/jira/browse/ARROW-8796
----------------------------------------------------------------
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] andygrove commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-654252387
@sunchao Did you have a chance to look at the code sample provided?
----------------------------------------------------------------
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 #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #7176:
URL: https://github.com/apache/arrow/pull/7176
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-655010984
Sorry for the late response. Yes agree that API compatibility is not a big concern here. I think the change is useful and my only concern is the new constraint on how writer is going to be used. We can work on improving that later though.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-632941938
Seems this will be a breaking change as it requires writers to be used in a different way?
----------------------------------------------------------------
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] Marwes commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
Marwes commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-628512678
The same thing ought to be possible for the reader, though it uses `&` in some places that must be changed to `&mut`
----------------------------------------------------------------
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] kszucs commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-664298168
The proposed alternative looks much simpler, so I think it would worth tackling that change in this PR.
----------------------------------------------------------------
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] Marwes commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
Marwes commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-635801955
https://github.com/apache/arrow/pull/7176/files#diff-64ae792f16372598b2b117c58f952626R727-R731
----------------------------------------------------------------
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 #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-759438272
@Marwes -- Given the imminent Arrow 3.0 release, I am trying to clean up older Rust PRs and see if the authors have plans to move them forward.
Do you plan on working on this PR in the near future? If not, should we close this PR until there is time to make progress? Thanks again for your contributions so far.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-656898683
Just took another look. If we are going with this, we can potentially make the API simpler. For instance, current it is:
```rust
fn next_row_group(&mut self) -> Result<Box<RowGroupWriter + '_>>;
```
As the returned box already is tracked by non-static lifetime, we can perhaps change the API to:
```rust
fn next_row_group(&mut self) -> Result<&mut RowGroupWriter>;
```
and track the current group writer within the file writer. With this we can also potentially remove `close_row_group`. Similar things can be done for column writer. We can do this in a separate PR though.
----------------------------------------------------------------
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 pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-759495316
@alamb I intend on picking this up in the next week or 2
----------------------------------------------------------------
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] wesm commented on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-656352940
Can this be merged?
----------------------------------------------------------------
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 edited a comment on pull request #7176: ARROW-8796: [Rust] feat: Allow writers to use &mut Vec
Posted by GitBox <gi...@apache.org>.
sunchao edited a comment on pull request #7176:
URL: https://github.com/apache/arrow/pull/7176#issuecomment-632941938
Seems this will be a breaking change as it requires writers to be used in a different way? and I think the new way is not as intuitive as the old one.
----------------------------------------------------------------
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