Repository navigation
Implement PARQUET-2249: Introduce IEEE 754 total order - #9619
Merged
Merged
Conversation
etseidl
added
the
api-change
etseidl
commented
Mar 26, 2026
| // For floating point we need to compare NaN values until we encounter a non-NaN | ||
| // value which then becomes the new min/max. After this, only non-NaN values are | ||
| // evaluated. If all values are NaN, then the min/max NaNs as determined by | ||
| // IEEE 754 total order are returned. |
Contributor
Author
There was a problem hiding this comment.
This has me a bit worried. I need to do some benchmarking to make sure all the complicated NaN logic isn't killing performance.
Contributor
There was a problem hiding this comment.
Agree, this method is on the hot path. I had a look at optimizing it, but could not get the compiler to generate nice auto-vectorized code for nan-handling yet. I think we can try optimizations in a followup, it would be more important to get the semantics correct first and make sure there are tests for edge cases.
If all values are NaN, then the min/max NaNs as determined by
// IEEE 754 total order are returned.
Does the current code correctly distinguish different NaN payloads according to their sign and bit patterns?
(Solved, github was hiding the changes to compare_greater in mod.rs)
Contributor
Author
|
Tests will fail until apache/parquet-testing#104 is merged. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
Shall i mark it draft and add next release tag? |
etseidl
marked this pull request as draft
June 2, 2026 16:31
etseidl
added
the
next-major-release
Contributor
|
👍 |
This was referenced Jun 3, 2026
This was referenced Jun 10, 2026
Contributor
|
🚀 |
etseidl
added a commit
that referenced
this pull request
Aug 4, 2026
# Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. --> - Closes #10103. - Depends on #9619 # Rationale for this change With the addition of new `ColumnOrder` variants, the public function `sort_order_for_type` has become unwieldy. # What changes are included in this PR? Add a deprecation notice to `sort_order_for_type`, and add a new `pub(crate)` function `get_sort_order_for_type` to replace it. The preferred path to obtain a `SortOrder` is now via `ColumnOrder::sort_order`. # Are these changes tested? Covered by existing tests # Are there any user-facing changes? Yes, this deprecates the public `sort_order_for_type`. --------- Co-authored-by: Andrew Lamb <[email protected]>
etseidl
added a commit
that referenced
this pull request
Aug 4, 2026
# Which issue does this PR close? - Closes #10105. - Depends on #10104 (which depends on #9619) # Rationale for this change Spark continues to use INT96 timestamps, despite INT96 being marked as deprecated in 2018. Query engines want valid statistics to allow reliably pruning on INT96 columns. apache/parquet-format#584 adds a new `ColumnOrder` variant which can be used to signal compliance with the only known use of INT96 (4-byte julian day from epoch, 8-byte nanosecond). # What changes are included in this PR? Adds support for the new enum variant, and writes the appropriate value in the `FileMetaData.column_orders` field. This builds on changes introduced in #7687. # Are these changes tested? Yes # Are there any user-facing changes? Yes, this adds a new variant to public enums (`ColumnOrder::INT96_TIMESTAMP_ORDER`, `SortOrder::INT96_TIMESTAMP`). --------- Co-authored-by: Andrew Lamb <[email protected]>
aaron-kumo
added a commit
to NVIDIA/kumo-engine
that referenced
this pull request
Nov 4, 2026
## What Move every arrow-rs crate and parquet in DiskGraph from 59.3.2 to 60.1. Also remove a stale `cargo update` of `time` from `docker/build_diskgraph_wheel`; the lockfile already pins it. ## Why parquet 60 brings zstd 0.14, which clears RUSTSEC-2026-0391, -0398, -0403 and -0410. ## Behavior change parquet-rs 60 always marks float and double columns with IEEE 754 total order (apache/arrow-rs#9619), and pyarrow up to 24 ignores min/max statistics on such columns (apache/arrow#50807). The only effect is that a reader filtering on a float column reads every row group instead of skipping some. Kumo never does that: DiskGraph doesn't read float statistics, and our Python code reads these files whole or filters only on ID columns. Values are unchanged. [Change map](#193 (comment))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
This takes the implementation done by @Xuanwo (#8158) and updates it to the new thrift format and recent changes to the original proposal (apache/parquet-format#514).
What changes are included in this PR?
Adds needed thrift structures as well as NaN counts for pages and column chunks.
Are these changes tested?
Yes, new tests added (more may be needed).
Are there any user-facing changes?
Yes. This adds the
IEEE_754_TOTAL_ORDERvariant to the publicColumnOrderenum, andTOTAL_ORDERto the publicSortOrderenum, which are technically breaking API changes. This also adds anan_countargument to the public functionColumnIndexBuilder::append.Non-breaking changes include adding
nan_countfields to theValueStatisticsandColumnIndexstructs.Behaviorally, this will now order all floating point statistics using IEEE754 total order, and all floating point columns will use the new
IEEE_754_TOTAL_ORDERcolumn order in theFileMetaData.column_orderslist. Newer readers that recognize the new column order may now safely use the statistics for chunk and page pruning. Older readers will not recognize this column order and should continue to ignore the statistics.