You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "wjones127 (via GitHub)" <gi...@apache.org> on 2023/07/02 04:48:58 UTC

[GitHub] [arrow-rs] wjones127 opened a new pull request, #4475: feat: support RecordBatchReader on boxed trait objects

wjones127 opened a new pull request, #4475:
URL: https://github.com/apache/arrow-rs/pull/4475

   # Which issue does this PR close?
   
   Closes #4474.
   
   This also contains a drive-by change to make the FFI export require Send, since we implemented send for the FFI struct earlier in #4232.
   
   # Rationale for this change
    
   Makes it possible to take generic `impl RecordBatchReader` arguments that support boxed and unboxed ones.
   
   # What changes are included in this PR?
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   # Are there any user-facing changes?
   
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking change` label.
   -->
   


-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] wjones127 commented on a diff in pull request #4475: feat: support RecordBatchReader on boxed trait objects

Posted by "wjones127 (via GitHub)" <gi...@apache.org>.
wjones127 commented on code in PR #4475:
URL: https://github.com/apache/arrow-rs/pull/4475#discussion_r1249211662


##########
arrow/src/ffi_stream.rs:
##########
@@ -157,7 +157,7 @@ impl Drop for FFI_ArrowArrayStream {
 
 impl FFI_ArrowArrayStream {
     /// Creates a new [`FFI_ArrowArrayStream`].
-    pub fn new(batch_reader: Box<dyn RecordBatchReader>) -> Self {
+    pub fn new(batch_reader: Box<dyn RecordBatchReader + Send>) -> Self {

Review Comment:
   This is sort of a bug fix to get in compliant with the released API, but in a direct sense it is breaking, so maybe this should be labelled as breaking change? Not sure 🤔 



-- 
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: github-unsubscribe@arrow.apache.org

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

[GitHub] [arrow-rs] tustvold merged pull request #4475: feat: support RecordBatchReader on boxed trait objects

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #4475:
URL: https://github.com/apache/arrow-rs/pull/4475


-- 
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: github-unsubscribe@arrow.apache.org

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