fix: Iceberg parquet dereference pushdown for nested types - #28512
Merged
Merged
Conversation
was broken for TIMESTAMP_WITH_TIME_ZONE, TIME and UUID since we relied on toHiveType to read them. Hive does not support these data types, so parsing them was broken
Contributor
Reviewer's GuideFix Iceberg Parquet nested dereference pushdown by resolving subfield types directly from Presto row types, preserving TIMESTAMP WITH TIME ZONE, TIME, and UUID instead of relying on lossy or unsupported Hive representations. Regression tests verify pushdown plans, exact synthesized types, and returned values. Sequence diagram for Iceberg nested dereference type resolutionsequenceDiagram
participant Optimizer as IcebergParquetDereferencePushDown
participant Handle as IcebergColumnHandle
participant RowType as PrestoRowType
participant Synthesized as SynthesizedColumnHandle
Optimizer->>Handle: getType()
Handle-->>Optimizer: base RowType
Optimizer->>RowType: getFields()
RowType-->>Optimizer: matching nested field type
Optimizer->>Synthesized: getSynthesizedIcebergColumnHandle(subfieldColumnName, pushdownColumnType, subfield)
Synthesized-->>Optimizer: nested column handle preserving TIMESTAMP_WITH_TIME_ZONE, TIME, or UUID
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
aaneja
marked this pull request as ready for review
September 18, 2026 14:28
aaneja
requested review from
a team,
ZacBlanco,
hantangwangd and
imjalpreet
as code owners
September 18, 2026 14:28
prestodb-ci
requested review from
a team and
bibith4
and removed request for
a team
September 18, 2026 14:28
Contributor
|
Thanks for the release note! Looks great. |
aditi-pandit
left a comment
Contributor
There was a problem hiding this comment.
Thanks @aaneja. Looks good minus the comment on the tests.
…ZONE subfields Add `WHERE x.b = TIMESTAMP ... UTC` and `WHERE x.c = TIME ...` variants to testParquetDereferencePushDownWithTypesNotRepresentableInHive. Both assert the filter lands on the synthesized subfield column as a single value domain. The expected domain is keyed by the synthesized column handle, so TupleDomain equality also pins the handle's Presto type. That makes the TIME case a regression guard for the Hive round trip, which silently produced a BIGINT handle instead of throwing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
was broken for TIMESTAMP_WITH_TIME_ZONE, TIME and UUID since we relied on
toHiveTypeto read them. Hive does not support these data types, so parsing them was brokenDescription
Fixes #28510
Motivation and Context
Impact
Test Plan
Contributor checklist
Summary by Sourcery
Fix Iceberg Parquet nested dereference pushdown for types that cannot be reliably represented by Hive.
Bug Fixes:
Enhancements:
Tests: