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/12/21 15:03:34 UTC

[GitHub] [arrow] pitrou opened a new pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

pitrou opened a new pull request #12011:
URL: https://github.com/apache/arrow/pull/12011


   Transferring C pointer values between Python and R would involve an intermediate conversion to a double, which might lose precision.
   
   Instead, pass an external pointer from R, which gets converted into a Python capsule.


-- 
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] pitrou commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   @paleolimbot 


-- 
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] paleolimbot commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       I think the only good reason to support strings, int64, or raw arrays, is because other language bridges might not be able to convert external pointers and/or pycapsule objects. With reticulate we had a pretty good chance of submitting a PR upstream (although it seems to work out of the box); other language bridges may not  be easy or fast to change.




-- 
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] paleolimbot commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/src/bridge.cpp
##########
@@ -21,13 +21,6 @@
 
 #include <arrow/c/bridge.h>
 
-// [[arrow::export]]

Review comment:
       Just a note to add this back...I'll take care of it when I do the R ticket handling a potentially old Python version (either remove it and error or use it for an old pyarrow).

##########
File path: r/tests/testthat/test-bridge.R
##########
@@ -25,16 +25,6 @@ test_that("Pointer wrapper accepts external pointers", {
   delete_arrow_schema(ptr)
 })
 
-test_that("Pointer wrapper accepts double-casted pointers", {

Review comment:
       Same as above (I'll remove this in the R ticket if we're no longer using it).




-- 
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] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       By the way, in the other direction, if a user has an up-to-date PyArrow but an old R-Arrow, it probably means R will give Python a double?




-- 
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] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       I did not use that exact wording (in case someone calls it in another context that R), but I added a warning about that.




-- 
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] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       I don't know, is it actually useful? This is not really a user-facing API. End users will call the higher-level bridge functions.




-- 
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] amol- commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

Posted by GitBox <gi...@apache.org>.
amol- commented on a change in pull request #12011:
URL: https://github.com/apache/arrow/pull/12011#discussion_r773234552



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       Should we also support strings as we did in R? I think there is a value in being consistent




-- 
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] github-actions[bot] commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


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


-- 
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] pitrou commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   @paleolimbot Do you have other concerns here?


-- 
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] ursabot edited a comment on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   Benchmark runs are scheduled for baseline = f904e39ad59bf75da1e74c2a45a9fdfae48d7d62 and contender = 3fc64515275c6515a9a2b7c733a9f8625e9a2978. 3fc64515275c6515a9a2b7c733a9f8625e9a2978 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/442b73401d324e43bb67183076424f7e...e29a3bbedaa94129a9e6fe8bcfc4c343/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b711e4eed22c4739943ced5bf7611942...741c29075f4f468ab8d51e174c4a308d/)
   [Failed :arrow_down:0.45% :arrow_up:0.4%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/39f46a72af7147ec8700edeb7532852a...9317c081f32a45d3b08afbf16754e82d/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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] pitrou commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   Also cc @amol- 


-- 
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] paleolimbot commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       True! I forgot about that. As long as the error message is along the lines of "update the R package" it's probably ok! When I do the R version I could do that, too.




-- 
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] ursabot commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   Benchmark runs are scheduled for baseline = f904e39ad59bf75da1e74c2a45a9fdfae48d7d62 and contender = 3fc64515275c6515a9a2b7c733a9f8625e9a2978. 3fc64515275c6515a9a2b7c733a9f8625e9a2978 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/442b73401d324e43bb67183076424f7e...e29a3bbedaa94129a9e6fe8bcfc4c343/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b711e4eed22c4739943ced5bf7611942...741c29075f4f468ab8d51e174c4a308d/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/39f46a72af7147ec8700edeb7532852a...9317c081f32a45d3b08afbf16754e82d/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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] paleolimbot commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       Should we provide backward compatibility here? I imagine this will error with `An integer is required` unless a user has an up-to-date pyarrow. I imagine we should do something like this:
   
   ``` r
   pa <- reticulate::import("pyarrow")
   if (package_version(pa$`__version__`) >= "6.0.0.9000") {
     # the new code
   } else {
     # the old code
   }
   ```
   
   ...which I'd be happy to do on a separate JIRA ticket if it's not a good fit for 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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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



[GitHub] [arrow] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       Ah, you're right. We can do that on a separate JIRA then, IMHO.




-- 
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] paleolimbot commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: r/R/python.R
##########
@@ -23,11 +23,7 @@ py_to_r.pyarrow.lib.Array <- function(x, ...) {
     delete_arrow_array(array_ptr)
   })
 
-  x$`_export_to_c`(
-    external_pointer_addr_double(array_ptr),
-    external_pointer_addr_double(schema_ptr)
-  )
-
+  x$`_export_to_c`(array_ptr, schema_ptr)

Review comment:
       Done! ARROW-15173




-- 
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] amol- commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

Posted by GitBox <gi...@apache.org>.
amol- commented on a change in pull request #12011:
URL: https://github.com/apache/arrow/pull/12011#discussion_r773241105



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       Well if I invoke `export_to_c` and `import_to_c` I end up invoking it. I'm asking just because we supported it in R, so thought it made sense to allow consistent behaviour between the two. In the end it's just one `elif` that doesn't seem to involve much maintenance burden.




-- 
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] paleolimbot commented on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   No concerns...looks great!


-- 
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] ursabot edited a comment on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   Benchmark runs are scheduled for baseline = f904e39ad59bf75da1e74c2a45a9fdfae48d7d62 and contender = 3fc64515275c6515a9a2b7c733a9f8625e9a2978. 3fc64515275c6515a9a2b7c733a9f8625e9a2978 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/442b73401d324e43bb67183076424f7e...e29a3bbedaa94129a9e6fe8bcfc4c343/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b711e4eed22c4739943ced5bf7611942...741c29075f4f468ab8d51e174c4a308d/)
   [Failed :arrow_down:0.45% :arrow_up:0.4%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/39f46a72af7147ec8700edeb7532852a...9317c081f32a45d3b08afbf16754e82d/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       (in general, accepting more implicit conversions also creates a larger risk of crashes / memory corruption)




-- 
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] pitrou commented on a change in pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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



##########
File path: python/pyarrow/types.pxi
##########
@@ -82,6 +84,26 @@ cdef bytes _datatype_to_pep3118(CDataType* type):
             return char
 
 
+cdef void* _as_c_pointer(v, allow_null=False) except *:
+    """
+    Convert a Python object to a raw C pointer.
+
+    Used mainly for the C data interface.
+    Integers are accepted as well as capsule objects with a NULL name.
+    (the latter for compatibility with raw pointers exported by reticulate)
+    """
+    cdef void* c_ptr
+    if isinstance(v, int):
+        c_ptr = <void*> <uintptr_t > v
+    elif PyCapsule_CheckExact(v):
+        c_ptr = PyCapsule_GetPointer(v, NULL)
+    else:

Review comment:
       But in which situation would you get a string in the first place?




-- 
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] pitrou closed pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #12011:
URL: https://github.com/apache/arrow/pull/12011


   


-- 
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] ursabot edited a comment on pull request #12011: ARROW-15169: [Python][R] Avoid unsafe Python-R pointer transfer

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


   Benchmark runs are scheduled for baseline = f904e39ad59bf75da1e74c2a45a9fdfae48d7d62 and contender = 3fc64515275c6515a9a2b7c733a9f8625e9a2978. 3fc64515275c6515a9a2b7c733a9f8625e9a2978 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/442b73401d324e43bb67183076424f7e...e29a3bbedaa94129a9e6fe8bcfc4c343/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b711e4eed22c4739943ced5bf7611942...741c29075f4f468ab8d51e174c4a308d/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/39f46a72af7147ec8700edeb7532852a...9317c081f32a45d3b08afbf16754e82d/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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