PARQUET-2249: Write IEEE 754 total order by default for floating-point columns - #3699
Open
Jiayi-Wang-db wants to merge 1 commit into
Open
PARQUET-2249: Write IEEE 754 total order by default for floating-point columns#3699Jiayi-Wang-db wants to merge 1 commit into
Jiayi-Wang-db wants to merge 1 commit into
Conversation
wgtmac
reviewed
Jul 31, 2026
| * the type-defined fallback. | ||
| */ | ||
| private boolean defaultsToIeee754TotalOrder(PrimitiveTypeName primitive) { | ||
| if (primitive != PrimitiveTypeName.FLOAT && primitive != PrimitiveTypeName.DOUBLE) { |
Member
There was a problem hiding this comment.
Is this correct? FLOAT16 has FLBA primitive type which will be rejected here. Should we pass LogicalTypeAnnotation explicitly instead of calling getLogicalTypeAnnotation() so these logics are in a single place?
Contributor
Author
There was a problem hiding this comment.
Thanks for catching this. Refactored into a single static defaultColumnOrder(primitive, originalType, logicalTypeAnnotation) that recognizes FLOAT16 .
| @Test | ||
| public void testSpecBuilderForFloat() { | ||
| PrimitiveType type = Types.required(FLOAT).named("test_float"); | ||
| // NaN/+-0 read semantics are specific to type-defined order (float now defaults to IEEE 754). |
Member
There was a problem hiding this comment.
There are so many similar verbose comments. Should we just remove them? I believe it is clear when test cases have set explicit column orders.
Jiayi-Wang-db
force-pushed
the
ieee754-total-order-default
branch
from
July 31, 2026 18:00
9388a3e to
38f5429
Compare
…t columns Follow-up to apache#3393, which added IEEE_754_TOTAL_ORDER support but kept TYPE_DEFINED_ORDER as the default for FLOAT, DOUBLE and FLOAT16 columns. Keeping the type-defined order as the default is a latent backward-compat hazard: the writer now computes finite min/max over the non-NaN subset and records nan_count, but an old reader that predates nan_count ignores it, accepts the finite bounds, and can incorrectly prune row groups that contain NaN. Readers instead ignore statistics written under an unknown sort order, so writing IEEE 754 total order by default is the safer behavior. See the discussion on apache#3393. This makes FLOAT, DOUBLE and FLOAT16 columns built without an explicit column order default to IEEE_754_TOTAL_ORDER (mirroring how apache#3610 defaults INT96 to INT96_TIMESTAMP_ORDER). Columns with a logical annotation that does not accept IEEE 754 total order (e.g. an unknown annotation) fall back to type-defined order so they remain constructible. The default-order selection is unified in PrimitiveType.defaultColumnOrder so construction and text serialization agree. To stay backward compatible on read, a footer that carries no column_orders list predates IEEE_754_TOTAL_ORDER, so floating-point columns read from such a footer are given type-defined order rather than inheriting the new construction-time default; their legacy statistics are thus not reinterpreted under IEEE 754 total order. The text schema representation now carries a non-default column order (columnorder(...) after the type/annotation) and MessageTypeParser parses it, so a column order set explicitly survives toString()/parse round-trips such as the one GroupWriteSupport performs. Columns left at their default emit no token, keeping existing schema strings unchanged. Tests that exercise the legacy type-defined NaN / +-0 semantics set TYPE_DEFINED_ORDER explicitly, and new tests cover the default serialization, the column-order-less read path, and the text round-trip. Co-authored-by: Isaac
Jiayi-Wang-db
force-pushed
the
ieee754-total-order-default
branch
from
July 31, 2026 20:15
38f5429 to
c8a7179
Compare
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
2 participants
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.
Rationale for this change
Under the new writer behavior, a floating-point column containing NaN gets finite min/max computed over the non-NaN values, together with a
nan_count. A reader that predatesnan_countignores it, accepts the finite bounds, and can incorrectly prune row groups that actually contain NaN. e.g. [1.0, NaN] is written as min = max = 1.0, so an old reader drops the row group for greater(x, 1.0) even though NaN matches.By contrast, readers ignore statistics written under an unknown sort order. Writing IEEE_754_TOTAL_ORDER by default is therefore the safer, forward-compatible behavior: older readers simply decline to use the stats they can't interpret rather than misinterpreting them.
What changes are included in this PR?
FLOAT, DOUBLE and FLOAT16 columns built without an explicit column order now default to
IEEE_754_TOTAL_ORDERAre these changes tested?
yes
Are there any user-facing changes?
Yes. Floating-point columns are now written with IEEE_754_TOTAL_ORDER by default instead of TYPE_DEFINED_ORDER.