Add support for level histograms added in PARQUET-2261 to `ParquetMetaData` #6105

etseidl · 2024-07-23T20:33:19Z

Which issue does this PR close?

Closes #5022.

Rationale for this change

Final piece of the new SizeStatistics. Supercedes #5486.

What changes are included in this PR?

Adds repetition and definition level histograms to ParquetMetaData and to the PageIndex struct (representation of the thrift ColumnIndex).

Are there any user-facing changes?

Yes.

) * bump `tonic` to 0.12 and `prost` to 0.13 for `arrow-flight` Signed-off-by: Bugen Zhao <i@bugenzhao.com> * fix example tests Signed-off-by: Bugen Zhao <i@bugenzhao.com> --------- Signed-off-by: Bugen Zhao <i@bugenzhao.com>

…ally copies data (apache#6043) * deprecate auto copy, ask explicit reference * update comments * make cargo doc happy

* improve dispaly for interval. * update test in pretty, and fix display problem. * tmp * fix tests in arrow-cast. * fix tests in pretty. * fix style.

* update to latest thrift (as of 11 Jul 2024) from parquet-format * pass None for optional size statistics * escape HTML tags * don't need to escape brackets in arrays

… end (#…" (apache#5933) This reverts commit 22e0b44.

This reverts commit 756b1fb.

* Update pyo3 requirement from 0.21.1 to 0.22.1 Updates the requirements on [pyo3](https://github.com/pyo3/pyo3) to permit the latest version. - [Release notes](https://github.com/pyo3/pyo3/releases) - [Changelog](https://github.com/PyO3/pyo3/blob/main/CHANGELOG.md) - [Commits](PyO3/pyo3@v0.21.1...v0.22.1) --- updated-dependencies: - dependency-name: pyo3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> * refactor: remove deprecated `FromPyArrow::from_pyarrow` "GIL Refs" are being phased out. * chore: update `pyo3` in integration tests --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* update to latest thrift (as of 11 Jul 2024) from parquet-format * pass None for optional size statistics * escape HTML tags * don't need to escape brackets in arrays * add support for unencoded_byte_array_data_bytes * add comments * change sig of ColumnMetrics::update_variable_length_bytes() * rename ParquetOffsetIndex to OffsetSizeIndex * rename some functions * suggestion from review Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * add Default trait to ColumnMetrics as suggested in review * rename OffsetSizeIndex to OffsetIndexMetaData --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

Updates the requirements on [pyo3](https://github.com/pyo3/pyo3) to permit the latest version. - [Release notes](https://github.com/pyo3/pyo3/releases) - [Changelog](https://github.com/PyO3/pyo3/blob/v0.22.2/CHANGELOG.md) - [Commits](PyO3/pyo3@v0.21.1...v0.22.2) --- updated-dependencies: - dependency-name: pyo3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…MetaData` (apache#6095) * deprecate read_page_locations * add to_thrift() to OffsetIndexMetaData

…ograms

alamb

Thank you @etseidl -- I think this look really nice

The only thing I could think that we might want to explore is using a rust struct rather than a Option<Vec<i64> so it could be easier to use / document. I'll make a PR to explore what that looks like

alamb · 2024-07-25T21:02:53Z

The clippy failure is likely due to the new rust version which was released today. I fixed it on main. Also since we have made a 52.2.0 release candidate I think we can move 53.0.0 development off of 53.0.0-dev to main. I will create a PR to do so

etseidl · 2024-07-25T21:09:23Z

The only thing I could think that we might want to explore is using a rust struct rather than a Option<Vec<i64> so it could be easier to use / document. I'll make a PR to explore what that looks like

❤️

Yes, while I was reviewing my changes I was thinking the same thing. A histogram class that supports updates, unions, and concatenation.

Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>

…ograms

alamb · 2024-07-25T21:55:07Z

Update for anyone following along: #6126 is the merge of the dev branch to main

etseidl · 2024-07-25T21:57:11Z

Merge went ok...once #6126 merges I'll rebase this on master.

alamb · 2024-07-26T10:15:10Z

I merged the 53 dev branch ~and that seems to have closed this PR` -- any chance you can retarget main?

Update: I restored the branch

alamb · 2024-07-26T12:04:40Z

Hi @etseidl -- could you please retarget this PR at the main branch so I can merge it in?

I also made etseidl#1 for your consideration (that make the level histogram a struct). We could also merge this PR as is and consider that as a follow on.

Thank you for bearing with me here.

etseidl · 2024-07-26T15:33:56Z

Hi @etseidl -- could you please retarget this PR at the main branch so I can merge it in?

done

I also made etseidl#1 for your consideration (that make the level histogram a struct). We could also merge this PR as is and consider that as a follow on.

@alamb I too created a LevelHistogram struct last night...yours is much more elegant 😄. ~~I'm game to merge your PR into my branch before merging this into master. Or you can submit yours as a follow on PR. Whichever is easier for you.~~ Nevermind, your changes were too nice to resist, so I merged them in.

Thank you for bearing with me here.

Thank you for all the help and for the free Rust schooling 😄

Implement LevelHistograms as a struct

etseidl · 2024-07-26T17:23:08Z

parquet/src/file/metadata/mod.rs

+        if let Some(ref rep_lvl_hist) = repetition_level_histogram {
+            let hist = self.repetition_level_histograms.get_or_insert(Vec::new());
+            hist.reserve(rep_lvl_hist.len());
+            hist.extend(rep_lvl_hist.values());


@alamb should we also replace the vectors in ColumnIndexBuilder with LevelHistogram? We could add an append() method to LevelHistogram to reduce some code duplication here.

Same question for PageIndex.

I went back and forth with this -- I think I was thinking this was building the thrift structure so we should use the thrift representation (Vec<i64>) but I may have gotten that wrong as I find the naming quite confusing

How about we explore doing it as a follow on PR?

alamb · 2024-07-26T17:35:07Z

Let's do this!

alamb · 2024-07-26T17:37:27Z

Thank you for all the help and for the free Rust schooling 😄

That is half the fun of working on these projects -- I see a lot of cool things while reviewing / working on PRs

alamb · 2024-07-26T17:37:51Z

Thanks again @etseidl -- I merged this to main and let's keep working to improve things there.

BugenZhao and others added 17 commits July 16, 2024 18:05

Remove impl<T: AsRef<[u8]>> From<T> for Buffer that easily accident…

8f76248

…ally copies data (apache#6043) * deprecate auto copy, ask explicit reference * update comments * make cargo doc happy

Make display of interval types more pretty (apache#6006)

bb5f12b

* improve dispaly for interval. * update test in pretty, and fix display problem. * tmp * fix tests in arrow-cast. * fix tests in pretty. * fix style.

Update snafu (apache#5930)

756b1fb

Update Parquet thrift generated structures (apache#6045)

fe04e09

* update to latest thrift (as of 11 Jul 2024) from parquet-format * pass None for optional size statistics * escape HTML tags * don't need to escape brackets in arrays

Revert "Revert "Write Bloom filters between row groups instead of the…

2e7f7ef

… end (#…" (apache#5933) This reverts commit 22e0b44.

Revert "Update snafu (apache#5930)" (apache#6069)

effccc1

This reverts commit 756b1fb.

remove repeated codes to make the codes more concise. (apache#6080)

05e681d

deprecate read_page_locations

41b8f66

add level histograms to metadata

c4c0a4d

add to_thrift() to OffsetIndexMetaData

1507c46

Merge branch 'deprecate_read_page_locations' into size_stats_histograms

23cc77e

Deprecate read_page_locations() and simplify offset index in `Parquet…

3bc9987

…MetaData` (apache#6095) * deprecate read_page_locations * add to_thrift() to OffsetIndexMetaData

Merge remote-tracking branch 'origin/53.0.0-dev' into size_stats_hist…

8369436

…ograms

github-actions bot added the parquet Changes to the parquet crate label Jul 23, 2024

etseidl added 2 commits July 24, 2024 20:40

move valid test into ColumnIndexBuilder::append_histograms

bf4dadb

move update_histogram() inside ColumnMetrics

4002464

alamb approved these changes Jul 25, 2024

View reviewed changes

alamb and others added 3 commits July 25, 2024 17:12

Merge remote-tracking branch 'apache/master' into 53.0.0-dev

095130f

Update parquet/src/column/writer/mod.rs

a6353d1

Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>

Merge remote-tracking branch 'origin/53.0.0-dev' into size_stats_hist…

f892613

…ograms

alamb deleted the branch apache:master July 26, 2024 10:11

alamb closed this Jul 26, 2024

alamb reopened this Jul 26, 2024

alamb changed the base branch from 53.0.0-dev to master July 26, 2024 11:16

alamb changed the base branch from master to 53.0.0-dev July 26, 2024 11:17

Implement LevelHistograms as a struct

978264d

alamb mentioned this pull request Jul 26, 2024

Implement LevelHistograms as a struct etseidl/arrow-rs#1

Merged

etseidl changed the base branch from 53.0.0-dev to master July 26, 2024 15:15

Merge remote-tracking branch 'origin/master' into size_stats_histograms

642c7d4

etseidl and others added 3 commits July 26, 2024 09:39

Merge pull request #1 from alamb/alamb/level_histogram_structs

9582e0d

Implement LevelHistograms as a struct

formatting

70af5dd

fix error in docs

e774bbe

etseidl commented Jul 26, 2024

View reviewed changes

alamb merged commit b06ffce into apache:master Jul 26, 2024
16 checks passed

etseidl deleted the size_stats_histograms branch July 26, 2024 17:49

This was referenced Jul 26, 2024

Add size statistics to ParquetMetaData introduced in PARQUET-2261 #5486

Closed

Use LevelHistogram throughout Parquet metadata #6134

Closed

alamb mentioned this pull request Aug 28, 2024

Invalid ColumnIndex written in parquet #6310

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for level histograms added in PARQUET-2261 to `ParquetMetaData` #6105

Add support for level histograms added in PARQUET-2261 to `ParquetMetaData` #6105

etseidl commented Jul 23, 2024

alamb left a comment

alamb commented Jul 25, 2024

etseidl commented Jul 25, 2024

alamb commented Jul 25, 2024

etseidl commented Jul 25, 2024

alamb commented Jul 26, 2024 •

edited

Loading

alamb commented Jul 26, 2024

etseidl commented Jul 26, 2024 •

edited

Loading

etseidl Jul 26, 2024

alamb Jul 26, 2024

alamb commented Jul 26, 2024

alamb commented Jul 26, 2024

alamb commented Jul 26, 2024

Add support for level histograms added in PARQUET-2261 to ParquetMetaData #6105

Add support for level histograms added in PARQUET-2261 to ParquetMetaData #6105

Conversation

etseidl commented Jul 23, 2024

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

alamb left a comment

Choose a reason for hiding this comment

alamb commented Jul 25, 2024

etseidl commented Jul 25, 2024

alamb commented Jul 25, 2024

etseidl commented Jul 25, 2024

alamb commented Jul 26, 2024 • edited Loading

alamb commented Jul 26, 2024

etseidl commented Jul 26, 2024 • edited Loading

etseidl Jul 26, 2024

Choose a reason for hiding this comment

alamb Jul 26, 2024

Choose a reason for hiding this comment

alamb commented Jul 26, 2024

alamb commented Jul 26, 2024

alamb commented Jul 26, 2024

Add support for level histograms added in PARQUET-2261 to `ParquetMetaData` #6105

Add support for level histograms added in PARQUET-2261 to `ParquetMetaData` #6105

alamb commented Jul 26, 2024 •

edited

Loading

etseidl commented Jul 26, 2024 •

edited

Loading