Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix decimals min max statistics #1621

Merged
merged 13 commits into from
Apr 29, 2022
Merged

Conversation

atefsawaed
Copy link
Contributor

Which issue does this PR close?

Closes #1532.

Rationale for this change

What changes are included in this PR?

Added a suitable comparator for Decimals that are built with byte arrays.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the parquet Changes to the parquet crate label Apr 26, 2022
@HaoYang670
Copy link
Contributor

Hi @viirya, could you please help to trigger the CI processes? Thank you!

parquet/src/column/writer.rs Show resolved Hide resolved
parquet/src/column/writer.rs Outdated Show resolved Hide resolved
parquet/src/column/writer.rs Outdated Show resolved Hide resolved
parquet/src/column/writer.rs Outdated Show resolved Hide resolved
Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

parquet/src/column/writer.rs Outdated Show resolved Hide resolved
parquet/src/column/writer.rs Outdated Show resolved Hide resolved
Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with two questions.

@viirya
Copy link
Member

viirya commented Apr 29, 2022

I will merge this after CI passes.

@alamb
Copy link
Contributor

alamb commented Apr 29, 2022

I will hold off on making the 13.0.0 release candidate to include this fix

@alamb
Copy link
Contributor

alamb commented Apr 29, 2022

Since the MIRI test takes a while to run (and it will run again on the branch prior to release) I am going to merge this PR in even before it is complete so I can make the release candidate. 🚀

@alamb
Copy link
Contributor

alamb commented Apr 29, 2022

Thanks @viirya and @atefsawaed

@alamb alamb merged commit d8d6499 into apache:master Apr 29, 2022
@viirya
Copy link
Member

viirya commented Apr 29, 2022

Thanks @alamb @atefsawaed @sunchao @HaoYang670

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect min/max statistics for decimals with byte-array notation
7 participants