MR: Pass partition constants to the format model read builder - #18037
Open
thswlsqls wants to merge 1 commit into
Open
MR: Pass partition constants to the format model read builder#18037thswlsqls wants to merge 1 commit into
thswlsqls wants to merge 1 commit into
Conversation
IcebergRecordReader#openTask no longer passed the identity partition constants to the reader after the Avro, ORC and Parquet branches were merged into the single FormatModel ReadBuilder API in a2802c4 (apache#15333). Data files that do not physically store the identity partition columns -- the ones produced by add_files/migrate through TableMigrationUtil#listPartition -- are read back with null in those columns instead of the value recorded in the manifest. Build the constants map with PartitionUtil#constantsMap and pass it to ReadBuilder#idToConstant, matching data/GenericReader#openFile, which kept that call in the same change. Generated-by: Claude Code
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
1 participant
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.
Closes #18033
Summary
IcebergRecordReader.openTask()builds the reader withoutidToConstant(...), so identity partition values that live only in the manifest never reach the reader and come back asnull.ReadBuilder; all three used to passconstantsMap(task, IdentityPartitionConverters::convertConstant).data/GenericReader.openFile(), touched by the same PR, kept that call, and so do Flink'sRowDataFileScanTaskReaderand Spark'sBaseRowReader.TableMigrationUtil.listPartition(...)takes partition values from Hive metadata rather than from the file, so tables imported withadd_files/migratehit this.Testing done
TestIcebergInputFormats#identityPartitionValuesMissingFromDataFile, which writes a data file whose schema omits the identity partition columns and asserts the read-back values come from the manifest. It fails before the fix on all 6 parameterized variants (IcebergInputFormat/MapredIcebergInputFormatx Avro/ORC/Parquet) and passes after../gradlew :iceberg-mr:check— 164 tests, 2 skipped, 0 failures.:iceberg-mris not a REVAPI module, so no API compatibility check applies.AI Disclosure
IcebergInputFormat.IcebergRecordReader.openTask()stopped passing to the format modelReadBuilderin a2802c4, mirroring the siblingdata/GenericReader.openFile(), and add a regression test inTestIcebergInputFormats.