ARROW-16719: [Python] Add path/URI + filesystem handling to parquet.read_metadata by kshitij12345 · Pull Request #13629 · apache/arrow

kshitij12345 · 2022-07-17T08:35:02Z

Add filesystem support to pq.read_metadata and pq.read_schema.

…read_metadata

github-actions · 2022-07-17T08:35:20Z

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

github-actions · 2022-07-17T08:35:22Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

kshitij12345 · 2022-07-17T08:37:03Z

+    pq.write_table(table, file_path)
+
+    # Get expected `metadata` from path.
+    metadata = pq.read_metadata(tmpdir / '/data.parquet')


Is there a way to get metadata directly from the table?

Try table.schema.metadata.

I tried that but it leads to segfault. Probably worth an issue? (I don't think the program should crash)

Crash Log

python/pyarrow/tests/parquet/test_metadata.py .........................sFatal Python error: Segmentation fault Current thread 0x00007f066e2be740 (most recent call first): File "/home/kshiteej/Apache/arrow/python/pyarrow/tests/parquet/test_metadata.py", line 549 in test_metadata_schema_filesystem File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/python.py", line 192 in pytest_pyfunc_call File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/python.py", line 1761 in runtest File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 166 in pytest_runtest_call File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 259 in <lambda> File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 338 in from_call File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 258 in call_runtest_hook File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 219 in call_and_report File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 130 in runtestprotocol File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/runner.py", line 111 in pytest_runtest_protocol File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/main.py", line 347 in pytest_runtestloop File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/main.py", line 322 in _main File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/main.py", line 268 in wrap_session File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/main.py", line 315 in pytest_cmdline_main File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/config/__init__.py", line 164 in main File "/home/kshiteej/.conda/envs/pyarrow-dev/lib/python3.9/site-packages/_pytest/config/__init__.py", line 187 in console_main File "/home/kshiteej/.conda/envs/pyarrow-dev/bin/pytest", line 11 in <module> Segmentation fault (core dumped)

Running this locally I can confirm a segfault. I think it happens because the table metadata is (correctly) empty:

>>> import pyarrow as pa >>> import pyarrow.parquet as pq >>> table = pa.table({"a": [1, 2, 3]}) >>> file_path = "/tmp/data.parquet" >>> metadata = table.schema.metadata >>> pq.read_metadata(file_path).equals(metadata) zsh: segmentation fault python

Which would deserve an issue (a warning should be returned without a crash).

Maybe a better option to test ParquetFile metadata would be to inspect individual attributes:

>>> pq.read_metadata(file_path).num_columns == 1 True >>> pq.read_metadata(file_path).num_rows == 3 True

num_columns: 1 num_rows: 3 num_row_groups: 1 format_version: 2.6 serialized_size: 375

From the following metadata attributes, I think num_columns, num_rows, num_row_groups make sense. Should we also check for the other two (I am not sure if they will stay same across versions and platforms)?

Ah, they sneaked in 😅. Removed :)

Is there already a JIRA opened for this segfault?

I don't think so. I will open one.

Issue Link : https://issues.apache.org/jira/browse/ARROW-17142

Thanks for opening the JIRA

kshitij12345 · 2022-07-17T20:49:35Z

macOS CI Failure looks unrelated.

AlenkaF · 2022-07-18T18:40:27Z

Thank you for working on this issue @kshitij12345! LGTM +1

@jorisvandenbossche can you please have a look before we merge this PR?

jorisvandenbossche

Looks good, thanks!
My only comment is about using a context manager to ensure we close the file again

jorisvandenbossche · 2022-07-20T07:01:13Z

+    filesystem, where = _resolve_filesystem_and_path(where, filesystem)
+    if filesystem is not None:
+        where = filesystem.open_input_file(where)
    return ParquetFile(where, memory_map=memory_map,


When opening the file with open_input_file, we should probably use it in a context manager to ensure we also close the file handle again:

with filesystem.open_input_file(where) as source: return ParquetFile(source, ...).metadata

(and the same for read_schema)

I am curious as to what is the minimum supported Python version.

I was planning to do something like which requires Python 3.7 or more.

source = filesystem.open_input_file(where) if filesystem is not None else nullcontext() with source: return ParquetFile( source, memory_map=memory_map, decryption_properties=decryption_properties).schema.to_arrow_schema()

Python 3.7 is our minimum supported version

Thanks for the confirmation. Have addressed the comment.

kshitij12345 · 2022-07-20T15:54:20Z

Failures look unrelated. Should I retrigger the CI?

AlenkaF · 2022-07-20T17:44:41Z

I think some of the failures are related:

=================================== FAILURES ===================================
_______________________ test_metadata_schema_filesystem ________________________
tmpdir = local('/tmp/pytest-of-root/pytest-0/test_metadata_schema_filesyste0')
    def test_metadata_schema_filesystem(tmpdir):
        table = pa.table({"a": [1, 2, 3]})
        # URI writing to local file.
        fname = "data.parquet"
        file_path = 'file:///' + os.path.join(str(tmpdir), fname)
        pq.write_table(table, file_path)
        # Get expected `metadata` from path.
        metadata = pq.read_metadata(tmpdir / fname)
        schema = table.schema
        assert pq.read_metadata(file_path).equals(metadata)
>       assert pq.read_metadata(
            fname, filesystem=f'file:///{tmpdir}').equals(metadata)
opt/conda/envs/arrow/lib/python3.8/site-packages/pyarrow/tests/parquet/test_metadata.py:553: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
opt/conda/envs/arrow/lib/python3.8/site-packages/pyarrow/parquet/__init__.py:3425: in read_metadata
    file = ParquetFile(where, memory_map=memory_map,
opt/conda/envs/arrow/lib/python3.8/site-packages/pyarrow/parquet/__init__.py:287: in __init__
    self.reader.open(
pyarrow/_parquet.pyx:1225: in pyarrow._parquet.ParquetReader.open
    ???
pyarrow/io.pxi:1674: in pyarrow.lib.get_reader
    ???
pyarrow/io.pxi:1665: in pyarrow.lib.get_native_file
    ???
pyarrow/io.pxi:943: in pyarrow.lib.OSFile.__cinit__
    ???
pyarrow/io.pxi:953: in pyarrow.lib.OSFile._open_readable
    ???
pyarrow/error.pxi:144: in pyarrow.lib.pyarrow_internal_check_status
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
>   ???
E   FileNotFoundError: [Errno 2] Failed to open local file 'data.parquet'. Detail: [errno 2] No such file or directory

kshitij12345 · 2022-07-20T17:55:09Z

Thanks for catching that @AlenkaF!

~~And Windows strikes 😓 ! I don't access to a Windows system. I think file:/// is not handled by Windows. Do you have any recommendations or is it ok to skip that particular approach on Windows?~~

Looks to be happening on other platforms as well. My bad, read the CI logs incorrectly.

kshitij12345 · 2022-07-27T12:45:10Z

Gentle ping @jorisvandenbossche @AlenkaF

raulcd

LGTM minor nit comment

raulcd · 2022-07-27T13:13:49Z

+    source = filesystem.open_input_file(
+        where) if filesystem is not None else where
+    ctx = source if filesystem is not None else nullcontext()


I find these lines slightly difficult to follow due to using both source and where variables and the two inline ifs for the same case, would something like:

ctx = nullcontext() if filesystem is not None: ctx = where = filesystem.open_input_file(where)

be slightly more readable?
In thas case we should use where instead of source on the ParquetFile but this was how it was originally too.

Makes sense. Thanks for the suggestion. Have updated the code. PTAL :)

…kshitij12345/arrow into develop/fs/read_schema-metadata

raulcd

Thanks @kshitij12345

kshitij12345 · 2022-08-04T12:58:00Z

Gentle ping :)

jorisvandenbossche

Looks good! Just one comment on the tests that I failed to notice before

jorisvandenbossche · 2022-08-05T09:27:40Z


 import pyarrow as pa
 from pyarrow.tests.parquet.common import _check_roundtrip, make_sample_file
+from pyarrow.filesystem import LocalFileSystem, FileSystem


Sorry for only noticing this late, but we should import here from pyarrow.fs to use the new filesystems in the tests (pyarrow.filesystem is deprecated)

jorisvandenbossche · 2022-08-09T18:48:17Z

@kshitij12345 there are some test failures that actually seem related

jorisvandenbossche · 2022-08-09T18:49:21Z

+        assert pq.read_metadata(
+            fname, filesystem=LocalFileSystem()).equals(metadata)
+        assert pq.read_metadata(
+            fname, filesystem=LocalFileSystem.get_instance()).equals(metadata)


The new LocalFileSystem has no get_instance() method, so I think you can just remove this case (since the line above already includes the case of a plain LocalFileSystem())

Thanks! I was away from keyboard for the week. Will fix it!

…/fs/read_schema-metadata

kshitij12345 · 2022-08-16T13:29:45Z

@jorisvandenbossche CI failure looks irrelevant. PTAL :)

jorisvandenbossche · 2022-08-17T11:20:36Z

I pushed a small additional update to the test (mainly changing to use our internal tempdir fixture instead of tmpdir)

Thanks again for the PR @kshitij12345 !

ursabot · 2022-08-17T16:12:45Z

Benchmark runs are scheduled for baseline = f6127fc and contender = 42ed37e. 42ed37e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️3.44% ⬆️2.89%] test-mac-arm
[Failed ⬇️4.38% ⬆️1.92%] ursa-i9-9960x
[Finished ⬇️5.12% ⬆️2.42%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 42ed37e3 ec2-t3-xlarge-us-east-2
[Finished] 42ed37e3 test-mac-arm
[Failed] 42ed37e3 ursa-i9-9960x
[Finished] 42ed37e3 ursa-thinkcentre-m75q
[Finished] f6127fca ec2-t3-xlarge-us-east-2
[Finished] f6127fca test-mac-arm
[Failed] f6127fca ursa-i9-9960x
[Finished] f6127fca ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ursabot · 2022-08-17T16:13:07Z

['Python', 'R'] benchmarks have high level of regressions.
test-mac-arm
ursa-i9-9960x

kshitij12345 · 2022-08-18T07:08:19Z

@jorisvandenbossche Thank you very much :)

…ead_metadata (apache#13629) Add `filesystem` support to `pq.read_metadata` and `pq.read_schema`. Lead-authored-by: kshitij12345 <[email protected]> Co-authored-by: Kshiteej K <[email protected]> Co-authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Joris Van den Bossche <[email protected]>

ARROW-16719: [Python] Add path/URI /+ filesystem handling to parquet.…

02a7f09

…read_metadata

github-actions Bot added the Component: Python label Jul 17, 2022

update path in test

0abe1df

kshitij12345 commented Jul 17, 2022

View reviewed changes

kshitij12345 marked this pull request as ready for review July 17, 2022 08:38

kshitij12345 added 4 commits July 18, 2022 11:45

pass filesystem arg

4a18fe2

update test

25dde91

remove stray files

3efca03

make linter happy

9580c5a

AlenkaF approved these changes Jul 19, 2022

View reviewed changes

pitrou requested a review from jorisvandenbossche July 19, 2022 12:49

jorisvandenbossche reviewed Jul 20, 2022

View reviewed changes

address review: use open_input_file with contextmanager

7d3b42f

kshitij12345 added 3 commits July 20, 2022 18:08

fix error

970ac49

fix linter : unused import

bc3ec3e

fix : filesystem can be None

67f444e

AlenkaF requested a review from jorisvandenbossche July 22, 2022 12:44

Merge branch 'master' into develop/fs/read_schema-metadata

26199da

raulcd reviewed Jul 27, 2022

View reviewed changes

kshitij12345 added 2 commits July 27, 2022 15:55

address review

c855deb

Merge branch 'develop/fs/read_schema-metadata' of https://github.com/…

d5ebecb

…kshitij12345/arrow into develop/fs/read_schema-metadata

raulcd approved these changes Jul 28, 2022

View reviewed changes

jorisvandenbossche reviewed Aug 5, 2022

View reviewed changes

address review : filesystem -> fs

4d8e0fa

jorisvandenbossche reviewed Aug 9, 2022

View reviewed changes

kshitij12345 added 4 commits August 13, 2022 10:29

remove get_instance case

c4dba6e

Merge branch 'master' of https://github.com/apache/arrow into develop…

105efa9

…/fs/read_schema-metadata

update error msg

6b7bab1

fix linter errors

5a63f4b

use our own tempdir instead of tmpdir

f9e36d8

jorisvandenbossche approved these changes Aug 17, 2022

View reviewed changes

jorisvandenbossche changed the title ~~ARROW-16719: [Python] Add path/URI /+ filesystem handling to parquet.read_metadata~~ ARROW-16719: [Python] Add path/URI + filesystem handling to parquet.read_metadata Aug 17, 2022

jorisvandenbossche merged commit 42ed37e into apache:master Aug 17, 2022

Conversation

kshitij12345 commented Jul 17, 2022

Uh oh!

github-actions Bot commented Jul 17, 2022

Uh oh!

github-actions Bot commented Jul 17, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kshitij12345 Jul 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kshitij12345 commented Jul 17, 2022

Uh oh!

AlenkaF commented Jul 18, 2022

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kshitij12345 commented Jul 20, 2022

Uh oh!

AlenkaF commented Jul 20, 2022

Uh oh!

kshitij12345 commented Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kshitij12345 commented Jul 27, 2022

Uh oh!

raulcd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raulcd left a comment

Choose a reason for hiding this comment

Uh oh!

kshitij12345 commented Aug 4, 2022

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche commented Aug 9, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kshitij12345 commented Aug 16, 2022

Uh oh!

jorisvandenbossche commented Aug 17, 2022

Uh oh!

kshitij12345 Jul 17, 2022 •

edited

Loading

kshitij12345 commented Jul 20, 2022 •

edited

Loading