Skip to content

GH-3835: Preserve legacy float column order by default - #3842

Closed
efegokdemir wants to merge 1 commit into
apache:masterfrom
efegokdemir:codex/3835-legacy-float-order
Closed

efegokdemir wants to merge 1 commit into
apache:masterfrom
efegokdemir:codex/3835-legacy-float-order

Conversation

@efegokdemir

Copy link
Copy Markdown

Rationale for this change

Parquet Java 1.18 changed the default column order for FLOAT, DOUBLE, and FLOAT16 to IEEE 754 total order. Older readers such as Hive 3.1.2 do not handle the new order consistently, which can make files written with the default schema unreadable. Retaining the previous default restores interoperability.

What changes are included in this PR?

Restore TYPE_DEFINED_ORDER as the default for floating-point columns while keeping IEEE 754 total order available through explicit schema configuration. INT96 and INTERVAL continue to default to UNDEFINED. The schema builder documentation and metadata conversion regression coverage reflect these semantics.

Are these changes tested?

  • Regression test failed before the implementation change: testFloatingPointColumnsDefaultToTypeDefinedOrder observed IEEE_754_TOTAL_ORDER in the generated footer.
  • After the change, TestParquetMetadataConverter passed (77 tests).
  • ./mvnw -pl parquet-column -am -Dthrift.version=0.25.0 test passed (704 tests in parquet-column; all reactor modules passed).
  • TestIeee754TotalOrderE2E passed (11 tests), including explicit IEEE 754 opt-in behavior.
  • ./mvnw -pl parquet-column,parquet-hadoop -Dthrift.version=0.25.0 spotless:check passed.

The local environment provides Thrift 0.25.0 while this checkout expects 0.24.0, so the test commands used -Dthrift.version=0.25.0 to satisfy the version check.

Are there any user-facing changes?

Yes. Schemas that leave floating-point column order unspecified now write the legacy TYPE_ORDER metadata. Applications that need IEEE 754 total ordering can continue to select it explicitly.

Generated with assistance from OpenAI Codex.

Keep IEEE 754 total order available as an explicit column order while preserving the type-defined default for older readers.

Generated-by: OpenAI Codex
@wgtmac

wgtmac commented Oct 8, 2026

Copy link
Copy Markdown
Member

Should we directly fix this in Hive? (Is it possible?) We shouldn't enable the old column order by default as per the discussion in #3699. WDYT? @gszadovszky @Fokko @divjotarora @Jiayi-Wang-db

@gszadovszky

Copy link
Copy Markdown
Contributor

The PR's description already says that Parquet-java 1.18 has moved to defaulting to IEEE 754 order for floating point numbers. It means, there are files out written with it already. Changing the default behavior in a later Parquet-java release would not change that. (Not talking about whether one would not use the defaults.) Hive should be fixed instead.

@Jiayi-Wang-db

Jiayi-Wang-db commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

No, I don't think changing the default is correct, can we fix Hive to ignore stats if the reader doesn't understand the column order? This is the contract listed in the parquet thrift definition: https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift#L1099-L1100

@efegokdemir

Copy link
Copy Markdown
Author

Closing this PR based on the recent maintainer feedback favoring Hive reader-side handling. Reverting the writer default would not address existing files already written with IEEE 754 column order, and I will not move this PR into a separate Hive implementation.

@efegokdemir efegokdemir closed this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants