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 2021/04/05 10:10:30 UTC
[GitHub] [arrow] ritchie46 opened a new pull request #9887: ARROW-12202: [Rust] Fix SEGFAULT/ SIGILL in child-data ffi
ritchie46 opened a new pull request #9887:
URL: https://github.com/apache/arrow/pull/9887
Continue #9778
--
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 #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#issuecomment-831031601
Reopened at https://github.com/apache/arrow-rs/pull/21
--
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] pitrou commented on a change in pull request #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#discussion_r607942405
##########
File path: rust/arrow/src/ffi.rs
##########
@@ -395,6 +397,12 @@ unsafe extern "C" fn release_array(array: *mut FFI_ArrowArray) {
// take ownership of `private_data`, therefore dropping it
Box::from_raw(array.private_data as *mut PrivateData);
+ // release children
+ for i in 0..array.n_children {
+ let child = *array.children.add(i as usize);
+ release_array(child);
Review comment:
You should call the release callback stored on `child`, not `release_array` directly.
--
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 #9887: ARROW-12202: [Rust] Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#issuecomment-813320498
https://issues.apache.org/jira/browse/ARROW-12202
--
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] pitrou commented on a change in pull request #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#discussion_r607941015
##########
File path: rust/arrow/src/ffi.rs
##########
@@ -135,13 +140,14 @@ impl FFI_ArrowSchema {
let n_children = children.len() as i64;
let children_ptr = children.as_ptr() as *mut *mut FFI_ArrowSchema;
- let flags = if nullable { 2 } else { 0 };
+ let flags = if nullable { FLAG_NULLABLE } else { FLAG_NONE };
let private_data = Box::new(SchemaPrivateData { children });
// <https://arrow.apache.org/docs/format/CDataInterface.html#c.ArrowSchema>
FFI_ArrowSchema {
format: CString::new(format).unwrap().into_raw(),
- // For child data a non null string is expected and is called item
+ // TODO: get the field name for general nested data
+ // For List child data a non null string is expected and is called item
name: CString::new("item").unwrap().into_raw(),
Review comment:
This should be released in `release_schema`, like `format` already is.
--
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 #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #9887:
URL: https://github.com/apache/arrow/pull/9887
--
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] pitrou commented on a change in pull request #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#discussion_r607942677
##########
File path: rust/arrow/src/ffi.rs
##########
@@ -395,6 +397,12 @@ unsafe extern "C" fn release_array(array: *mut FFI_ArrowArray) {
// take ownership of `private_data`, therefore dropping it
Box::from_raw(array.private_data as *mut PrivateData);
+ // release children
+ for i in 0..array.n_children {
+ let child = *array.children.add(i as usize);
+ release_array(child);
Review comment:
Note you must do the same thing in `release_schema` as well.
--
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 #9887: ARROW-12202: [Rust] WIP Fix SEGFAULT/ SIGILL in child-data ffi
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9887:
URL: https://github.com/apache/arrow/pull/9887#issuecomment-822358704
The Apache Arrow Rust community is moving the Rust implementation into its own dedicated github repositories [arrow-rs](https://github.com/apache/arrow-rs) and [arrow-datafusion](https://github.com/apache/arrow-datafusion). It is likely we will not merge this PR into this repository
Please see the [mailing-list](https://lists.apache.org/thread.html/rce7c61cb3f703367dc00984530016e3fcb828e5a8035655fbcccf862%40%3Cdev.arrow.apache.org%3E) thread for more details
We expect the process to take a few days and will follow up with a migration plan for the in-flight PRs.
--
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