You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2025/08/26 08:11:55 UTC

(arrow) branch main updated: GH-47301: [Python] Fix FileFragment.open() seg fault behavior for file-like objects (#47302)

This is an automated email from the ASF dual-hosted git repository.

raulcd pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new d9f36a1273 GH-47301: [Python] Fix FileFragment.open() seg fault behavior for file-like objects (#47302)
d9f36a1273 is described below

commit d9f36a12738c0a250cd4433472ec37fc03b9a811
Author: Lester Fan <le...@gmail.com>
AuthorDate: Tue Aug 26 04:11:49 2025 -0400

    GH-47301: [Python] Fix FileFragment.open() seg fault behavior for file-like objects (#47302)
    
    ### Rationale for this change
    
    This PR resolves the issue reported in https://github.com/apache/arrow/issues/47301.
    
    There are [three possible file source types](https://github.com/apache/arrow/blob/80addfab90b65c9127b46cc5c0ff48af4db1afb3/python/pyarrow/_dataset.pyx#L104) in which a `CFileSource` can be created:
    
    1. From a `pa.Buffer`.
    2. From a `path` string.
    3. From a file-like object which has a `read` attribute.
    
    However, `FileFragment.open()` currently only [explicitly handles the first two types](https://github.com/apache/arrow/blob/80addfab90b65c9127b46cc5c0ff48af4db1afb3/python/pyarrow/_dataset.pyx#L2005). When `open` is called with a `FileFragment` created from type (3), the current implementation tries to read the `path` which is set to a string called `"<Buffer>"` ([source](https://github.com/apache/arrow/blob/135357ce3824d1a8e1aba5a19d897b0c02b22ab7/cpp/src/arrow/dataset/file_base.h#L1 [...]
    
    ### What changes are included in this PR?
    
    1. Modify `FileFragment.open()` to handle the three `CFileSource` cases as listed above.
    2. Add a unit test which seg faults without the change in (1) and passes with the change.
    
    ### Are these changes tested?
    
    Yes.
    
    ### Are there any user-facing changes?
    
    Yes; this PR fixes a user facing bug in the `FileFragment` API.
    
    * GitHub Issue: #47301
    
    Authored-by: Lester Fan <le...@gmail.com>
    Signed-off-by: Raúl Cumplido <ra...@gmail.com>
---
 python/pyarrow/_dataset.pyx                  | 13 +++++++++----
 python/pyarrow/includes/libarrow_dataset.pxd |  1 +
 python/pyarrow/tests/test_dataset.py         |  8 ++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/python/pyarrow/_dataset.pyx b/python/pyarrow/_dataset.pyx
index d279881d15..af4a689882 100644
--- a/python/pyarrow/_dataset.pyx
+++ b/python/pyarrow/_dataset.pyx
@@ -2012,13 +2012,18 @@ cdef class FileFragment(Fragment):
             c_string c_path
             NativeFile out = NativeFile()
 
+        # Handle each of the cases in _make_file_source
         if self.buffer is not None:
             return pa.BufferReader(self.buffer)
 
-        c_path = tobytes(self.file_fragment.source().path())
-        with nogil:
-            c_filesystem = self.file_fragment.source().filesystem()
-            opened = GetResultValue(c_filesystem.get().OpenInputFile(c_path))
+        if self.file_fragment.source().filesystem() != nullptr:
+            c_path = tobytes(self.file_fragment.source().path())
+            with nogil:
+                c_filesystem = self.file_fragment.source().filesystem()
+                opened = GetResultValue(c_filesystem.get().OpenInputFile(c_path))
+        else:
+            with nogil:
+                opened = GetResultValue(self.file_fragment.source().Open())
 
         out.set_random_access_file(opened)
         out.is_readable = True
diff --git a/python/pyarrow/includes/libarrow_dataset.pxd b/python/pyarrow/includes/libarrow_dataset.pxd
index 9e5b9f0fcb..12cf827339 100644
--- a/python/pyarrow/includes/libarrow_dataset.pxd
+++ b/python/pyarrow/includes/libarrow_dataset.pxd
@@ -182,6 +182,7 @@ cdef extern from "arrow/dataset/api.h" namespace "arrow::dataset" nogil:
         const shared_ptr[CFileSystem]& filesystem() const
         const shared_ptr[CBuffer]& buffer() const
         const int64_t size() const
+        CResult[shared_ptr[CRandomAccessFile]] Open() const
         # HACK: Cython can't handle all the overloads so don't declare them.
         # This means invalid construction of CFileSource won't be caught in
         # the C++ generation phase (though it will still be caught when
diff --git a/python/pyarrow/tests/test_dataset.py b/python/pyarrow/tests/test_dataset.py
index e7365643b8..32bcebb28d 100644
--- a/python/pyarrow/tests/test_dataset.py
+++ b/python/pyarrow/tests/test_dataset.py
@@ -1364,6 +1364,14 @@ def test_make_parquet_fragment_from_buffer(dataset_reader, pickle_module):
         pickled = pickle_module.loads(pickle_module.dumps(fragment))
         assert dataset_reader.to_table(pickled).equals(table)
 
+        # GH-47301: Ensure fragment.open() works for file-like objects.
+        file_like = pa.BufferReader(buffer)
+        fragment = format_.make_fragment(file_like)
+        opened_file = fragment.open()
+        assert isinstance(opened_file, pa.NativeFile)
+        assert opened_file.readable
+        assert pq.ParquetFile(fragment.open()).read().equals(table)
+
 
 @pytest.mark.parquet
 def _create_dataset_for_fragments(tempdir, chunk_size=None, filesystem=None):