You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "doki23 (via GitHub)" <gi...@apache.org> on 2023/12/27 12:56:56 UTC
[PR] optimize the next function of ArrowArrayStreamReader [arrow-rs]
doki23 opened a new pull request, #5248:
URL: https://github.com/apache/arrow-rs/pull/5248
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #5103 .
--
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
Re: [PR] optimize the next function of ArrowArrayStreamReader [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #5248:
URL: https://github.com/apache/arrow-rs/pull/5248
--
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
Re: [PR] optimize the next function of ArrowArrayStreamReader [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5248:
URL: https://github.com/apache/arrow-rs/pull/5248#discussion_r1438336203
##########
arrow/src/ffi_stream.rs:
##########
@@ -356,12 +357,10 @@ impl Iterator for ArrowArrayStreamReader {
return None;
}
- let schema_ref = self.schema();
- // NOTE: this parses the FFI_ArrowSchema again on each iterator call;
- // should probably use from_ffi_and_data_type() instead.
- let schema = FFI_ArrowSchema::try_from(schema_ref.as_ref()).ok()?;
-
- let data = unsafe { from_ffi(array, &schema) }.ok()?;
+ let data = unsafe {
+ from_ffi_and_data_type(array, DataType::Struct(self.schema().fields().clone()))
+ }
+ .ok()?;
Review Comment:
https://github.com/apache/arrow-rs/pull/5256
--
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
Re: [PR] optimize the next function of ArrowArrayStreamReader [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5248:
URL: https://github.com/apache/arrow-rs/pull/5248#discussion_r1437031154
##########
arrow/src/ffi_stream.rs:
##########
@@ -356,12 +357,10 @@ impl Iterator for ArrowArrayStreamReader {
return None;
}
- let schema_ref = self.schema();
- // NOTE: this parses the FFI_ArrowSchema again on each iterator call;
- // should probably use from_ffi_and_data_type() instead.
- let schema = FFI_ArrowSchema::try_from(schema_ref.as_ref()).ok()?;
-
- let data = unsafe { from_ffi(array, &schema) }.ok()?;
+ let data = unsafe {
+ from_ffi_and_data_type(array, DataType::Struct(self.schema().fields().clone()))
+ }
+ .ok()?;
Review Comment:
I know this isn't introduced by this PR, but I wonder why we surpress errors here?
Perhaps @viirya knows?
--
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
Re: [PR] optimize the next function of ArrowArrayStreamReader [arrow-rs]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #5248:
URL: https://github.com/apache/arrow-rs/pull/5248#discussion_r1437210927
##########
arrow/src/ffi_stream.rs:
##########
@@ -356,12 +357,10 @@ impl Iterator for ArrowArrayStreamReader {
return None;
}
- let schema_ref = self.schema();
- // NOTE: this parses the FFI_ArrowSchema again on each iterator call;
- // should probably use from_ffi_and_data_type() instead.
- let schema = FFI_ArrowSchema::try_from(schema_ref.as_ref()).ok()?;
-
- let data = unsafe { from_ffi(array, &schema) }.ok()?;
+ let data = unsafe {
+ from_ffi_and_data_type(array, DataType::Struct(self.schema().fields().clone()))
+ }
+ .ok()?;
Review Comment:
Maybe I thought that as `self.schema` is parsed from imported `FFI_ArrowSchema`, it should be no error to export it to `FFI_ArrowSchema` again? It doesn't looks quite correct and I think we should propagate the error.
--
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