GH-35331: [Python] Expose Parquet sorting metadata #37665

judahrand · 2023-09-11T16:49:28Z

Rationale for this change

Picking up where #35453 left off.

Closes #35331

This PR builds on top of #37469

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Closes: [C++][Python] Expose sorting_columns in RowGroupMetaData for Parquet files #35331

judahrand · 2023-09-20T10:39:18Z

@mapleFU Could you also take a look at this PR when you get a chance?

mapleFU · 2023-09-20T11:01:01Z

Ooops seems CI failed.

I'm a little busy this week, but I'll take a glance later this week :-) Maybe you can call other for review if it's hurry

judahrand · 2023-09-20T11:42:47Z

Ooops seems CI failed.

I'm a little busy this week, but I'll take a glance later this week :-) Maybe you can call other for review if it's hurry

Sorry! Will do.

mapleFU · 2023-09-20T12:58:03Z

python/pyarrow/_parquet.pyx

+    Notes
+    -----
+
+    Column indices are zero-based, refer only to leaf fields, and are in


By the way, in current implemention, parquet itself will not check the sorting_order is satisfied in user input. So we'd better:

Maybe only enable writing for test?

Or tell the user this is so dangerous and user should set this order if he/she cannot not gurantee ordering.

This is made clear in the docstrings of the Parquet Writer:

arrow/python/pyarrow/parquet/core.py

Lines 885 to 888 in 5db4e8e

sorting_columns : Sequence of SortingColumn, default None

Specify the sort order of the data being written. The writer does not sort

the data nor does it verify that the data is sorted. The sort order is

written to the row group metadata, which can then be used by readers.

judahrand · 2023-10-02T14:01:26Z

A rebase seems to have fixed the failing tests and now it is just AppVeyor failing (which I think is unrelated to these changes). Could someone please review this when they get the chance?

mapleFU

cc @AlenkaF @jorisvandenbossche

judahrand · 2023-10-23T19:26:37Z

cc @AlenkaF @jorisvandenbossche

AlenkaF

One question otherwise LGTM 👍

AlenkaF · 2023-10-24T07:43:00Z

python/pyarrow/_parquet.pyx

+    return out
+
+
+def _sort_keys_to_sorting_columns(sort_keys, null_placement, Schema schema):


It seems this is not used anywhere - do you know, is it meant to be as a possible helper function? Looks like a duplicate of from_sort_order to me.

They do look to do exactly the same thing so almost certainly just accidentially left in when the classmethod was added.

AlenkaF

AppVeyor failure is not connected. Thanks again @judahrand !

@jorisvandenbossche would you like to have a view before we merge?

jorisvandenbossche

Generally looks good! I am bit hesitant about the custom to_sort_order / from_sort_order API ..

python/pyarrow/_parquet.pxd

python/pyarrow/_parquet.pyx

jorisvandenbossche · 2023-10-27T09:01:58Z

python/pyarrow/_parquet.pyx

+    out = dict()
+
+    cdef SchemaDescriptor* parquet_schema = sp_parquet_schema.get()
+
+    for i in range(parquet_schema.num_columns()):
+        name = frombytes(parquet_schema.Column(i).path().get().ToDotString())
+        out[name] = i
+
+    return out


This could also be done with existing ParquetSchema cython class (parquet_schema[idx].path instead of creating the dict + lookup), but I suppose the problem is that we currently don't allow to create a ParquetSchema from scratch from a pyarrow schema, but only from an actual FileMetaData object?

Yes. A from_arrow_schema classmethod could be introduced on ParquetSchema but that feels a bit out of scope of these changes.

mapleFU · 2023-12-10T11:04:00Z

Ping @jorisvandenbossche

mapleFU · 2023-12-13T04:38:26Z

Gentle ping @jorisvandenbossche

mapleFU · 2023-12-18T13:39:17Z

@AlenkaF would you like to move it forward? Or lets wait after January

AlenkaF · 2023-12-18T14:07:54Z

Will wait till tomorrow for any comments otherwise I think this could get merged 👍

AlenkaF · 2023-12-19T11:43:54Z

Just one small comment before we merge: #37665 (comment)

AlenkaF

Thank you! +1

conbench-apache-arrow · 2023-12-20T19:34:55Z

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit cc9e649.

There were 2 benchmark results indicating a performance regression:

Commit Run on ursa-i9-9960x at 2023-12-20 10:21:10Z
- tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-04, scale_factor=1
- tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-04, scale_factor=1

The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them.

### Rationale for this change Picking up where apache#35453 left off. Closes apache#35331 This PR builds on top of apache#37469 ### What changes are included in this PR? ### Are these changes tested? ### Are there any user-facing changes? * Closes: apache#35331 Lead-authored-by: Judah Rand <17158624+judahrand@users.noreply.github.com> Co-authored-by: Will Jones <willjones127@gmail.com> Signed-off-by: AlenkaF <frim.alenka@gmail.com>

github-actions bot added Component: Python Component: Documentation awaiting review Awaiting review labels Sep 11, 2023

judahrand force-pushed the GH-35331-parquet-sorting-python branch from 191309d to 8b9fffe Compare September 11, 2023 17:07

mapleFU mentioned this pull request Sep 11, 2023

GH-37470: [Python][Parquet] Add missing arguments to ParquetFileWriteOptions #37469

Merged

judahrand force-pushed the GH-35331-parquet-sorting-python branch 2 times, most recently from b31ef33 to c6bb3fb Compare September 11, 2023 19:24

judahrand force-pushed the GH-35331-parquet-sorting-python branch from c6bb3fb to 5db4e8e Compare September 20, 2023 10:38

mapleFU reviewed Sep 20, 2023

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 20, 2023

judahrand force-pushed the GH-35331-parquet-sorting-python branch from 5db4e8e to 65a5798 Compare October 2, 2023 11:15

judahrand requested a review from mapleFU October 2, 2023 14:01

mapleFU approved these changes Oct 3, 2023

View reviewed changes

judahrand force-pushed the GH-35331-parquet-sorting-python branch from 65a5798 to c162438 Compare October 23, 2023 19:26

AlenkaF reviewed Oct 24, 2023

View reviewed changes

AlenkaF approved these changes Oct 24, 2023

View reviewed changes

jorisvandenbossche reviewed Oct 27, 2023

View reviewed changes

github-actions bot added awaiting changes Awaiting changes awaiting review Awaiting review awaiting committer review Awaiting committer review and removed awaiting committer review Awaiting committer review awaiting review Awaiting review awaiting changes Awaiting changes labels Oct 27, 2023

judahrand force-pushed the GH-35331-parquet-sorting-python branch from e76aeaa to fcfd65b Compare October 30, 2023 11:56

wjones127 and others added 16 commits December 10, 2023 10:57

feat: expose parquet sort metadata

94b437f

refactor: simplify API

23aa525

docs: add a nice example

fe0c7c0

fix: address nested schemas

f92869b

fix the repr

ad128a6

docs: fix doc formatting

2ed9b1b

docs: update repr in examples

67584e5

remove sort order aggregation

bba1a12

make repr deterministic

3a487e3

fix doctest

2c98b04

pr feedback

d994b42

Add sorting_columns to ParquetFileWriteOptions

dda4fed

Format for reduced git diff

0de1e68

Remove unused helper

280cfee

Move import to where it is needed!

0249bb6

Remove unnecessary exception

7c8849f

judahrand force-pushed the GH-35331-parquet-sorting-python branch from 6b86bcd to 7c8849f Compare December 10, 2023 10:59

Fix rebase error

842107f

Rename methods

44dff9d

AlenkaF approved these changes Dec 20, 2023

View reviewed changes

AlenkaF merged commit cc9e649 into apache:main Dec 20, 2023
14 checks passed

AlenkaF removed the awaiting committer review Awaiting committer review label Dec 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-35331: [Python] Expose Parquet sorting metadata #37665

GH-35331: [Python] Expose Parquet sorting metadata #37665

judahrand commented Sep 11, 2023 •

edited

judahrand commented Sep 20, 2023

mapleFU commented Sep 20, 2023

judahrand commented Sep 20, 2023

mapleFU Sep 20, 2023

judahrand Oct 2, 2023

judahrand commented Oct 2, 2023

mapleFU left a comment

judahrand commented Oct 23, 2023

AlenkaF left a comment

AlenkaF Oct 24, 2023

judahrand Oct 24, 2023

AlenkaF left a comment

jorisvandenbossche left a comment

jorisvandenbossche Oct 27, 2023

judahrand Oct 30, 2023

mapleFU commented Dec 10, 2023

mapleFU commented Dec 13, 2023

mapleFU commented Dec 18, 2023

AlenkaF commented Dec 18, 2023

AlenkaF commented Dec 19, 2023

AlenkaF left a comment

conbench-apache-arrow bot commented Dec 20, 2023

	sorting_columns : Sequence of SortingColumn, default None
	Specify the sort order of the data being written. The writer does not sort
	the data nor does it verify that the data is sorted. The sort order is
	written to the row group metadata, which can then be used by readers.

		return out


		def _sort_keys_to_sorting_columns(sort_keys, null_placement, Schema schema):

GH-35331: [Python] Expose Parquet sorting metadata #37665

GH-35331: [Python] Expose Parquet sorting metadata #37665

Conversation

judahrand commented Sep 11, 2023 • edited

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

judahrand commented Sep 20, 2023

mapleFU commented Sep 20, 2023

judahrand commented Sep 20, 2023

mapleFU Sep 20, 2023

Choose a reason for hiding this comment

judahrand Oct 2, 2023

Choose a reason for hiding this comment

judahrand commented Oct 2, 2023

mapleFU left a comment

Choose a reason for hiding this comment

judahrand commented Oct 23, 2023

AlenkaF left a comment

Choose a reason for hiding this comment

AlenkaF Oct 24, 2023

Choose a reason for hiding this comment

judahrand Oct 24, 2023

Choose a reason for hiding this comment

AlenkaF left a comment

Choose a reason for hiding this comment

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jorisvandenbossche Oct 27, 2023

Choose a reason for hiding this comment

judahrand Oct 30, 2023

Choose a reason for hiding this comment

mapleFU commented Dec 10, 2023

mapleFU commented Dec 13, 2023

mapleFU commented Dec 18, 2023

AlenkaF commented Dec 18, 2023

AlenkaF commented Dec 19, 2023

AlenkaF left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Dec 20, 2023

judahrand commented Sep 11, 2023 •

edited