Repository navigation
[fix](iceberg) Decode Parquet timestamptz columns as instants by the Iceberg table type - #69845
Conversation
…Iceberg table type ### What problem does this PR solve? Issue Number: None Related PR: apache#69711 Problem Summary: An Iceberg timestamptz column stored as Parquet INT96, the encoding Spark and Hive use for timestamps and which add_files, migrate and snapshot attach unchanged, read shifted by the session time zone's offset in any non-UTC session. INT96 has no instant annotation, FE sends Parquet timestamp-semantics version 1 for every file scan, so FileScannerV2 decoded INT96 as a UTC wall clock and the projection cast it to TIMESTAMPTZ in the session time zone: in Asia/Shanghai 2024-01-01T00:00:00Z read as 2024-01-01 00:00:00+08:00, eight hours early, while Iceberg reads 2024-01-01 08:00:00+08:00. Equality deletes on such a key missed their rows, and in a catalog with enable.mapping.timestamp_tz=false a tombstone at a DST fall-back time deleted both rows that share the local time. FE now sets timestamp_is_adjusted_to_utc on every timestamptz field of the Iceberg schema dictionary, as the Paimon connector does; timestamp fields stay unmarked. IcebergTableReader copies the marker onto the Parquet file column the mapper selects for each field (by field id, the table's name mapping or the positional fallback ids) and rewrites its type with apply_file_timestamp_semantics, moved from PaimonReader to schema_history_util. Hidden equality-delete keys and equality-delete files carry the marker too, so data keys and tombstones of a timestamptz field are both instants. Columns without a marked leaf keep their file types, and plans from older FEs carry no marker and read as before. find_current_schema and build_schema_identity_from_external_field move to schema_history_util. ### Release note Fix Iceberg timestamptz columns stored as Parquet INT96 (written by Spark or Hive and attached with add_files, migrate or snapshot) reading shifted by the session time zone's offset in non-UTC sessions, and equality deletes on such keys missing their rows. ### Check List (For Author) - Test: Unit Test (5 new BE cases in iceberg_reader_test.cpp; FE IcebergSchemaUtilsTest) and Manual test (local cluster reading INT96 and Iceberg-written INT64 files of one table) - Behavior changed: Yes (Parquet columns of Iceberg timestamptz fields decode as instants by the table type) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
TPC-H: Total hot run time: 28539 ms |
TPC-DS: Total hot run time: 151609 ms |
ClickBench: Total hot run time: 24.49 s |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review converged in round 1. The change correctly carries Iceberg timestamptz markers through the ordinary field-id, name-mapping, positional, equality-delete, Parquet conversion, and Paimon compatibility paths, but it has one verified P1: recursive ID-less STRUCT wrappers are supported by the mapper yet the new marker walk stops before the marked leaf, shifting nested INT96 instants and hidden equality-delete keys in non-UTC sessions. No existing inline review threads were present. The review is static-only as required; no builds or tests were run. Concurrency, persistence, protocol, and configuration checkpoints are unaffected. The added tests cover direct and nested field-id paths, equality deletes, DST folds, and FE marker emission, but do not cover the nested ID-less-wrapper case described in the inline finding. No additional user focus was supplied.
Review converged after 1 round(s).
| return key || value; | ||
| } | ||
| case TYPE_STRUCT: { | ||
| bool annotated = false; |
There was a problem hiding this comment.
[P1] Propagate Iceberg timestamp markers through nested ID-less wrappers
TableColumnMapper explicitly enables allow_idless_complex_wrapper_projection recursively for Iceberg Parquet, so it can resolve a selected leaf through an ID-less intermediate STRUCT. The new timestamp walk uses find_matched_file_column(..., false) for every STRUCT child, stops at that wrapper, and leaves the leaf's timestamp_is_adjusted_to_utc unset. With an outer-ID/inner-ID-less/leaf-ID file (the shape covered by ParquetRecursivelyRetainsIdlessWrapperForSelectedLeafId) and an INT96 timestamptz leaf, the scan still maps the leaf but decodes it as a session-local DATETIMEV2, shifting the instant; a hidden equality-delete key through the same path is affected too. Pass the same allow-wrapper flag through the recursive annotation or reuse the mapper's recursive resolver, and add a nested INT96 timestamptz regression test.
There was a problem hiding this comment.
Fixed in f2b97b1. annotate_matched_timestamp_semantics() now takes allow_idless_complex_wrapper_projection and passes it down to every level, so the STRUCT branch resolves a child through an ID-less wrapper the way TableColumnMapper does below the root. BY_NAME mapping is unchanged. The equality-delete paths reuse the same walk, so nested keys under a wrapper get the marker too.
The new IcebergV2ReaderTest.IcebergInt96TimestampTzUnderIdlessWrapperReadsInstantWithTableMarker writes outer (field id 10) holding an ID-less inner struct whose INT96 ts (field id 30) carries the marker. It reads the file in Asia/Shanghai, and applies an adjusted-UTC equality delete on ts, once with outer projected and once with the key hidden. Passing false again in the STRUCT branch makes it fail: the values read eight hours early (2023-12-31 16:00:00.123456+00:00), and the tombstone leaves its row (ids {1, 2} instead of {2}).
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
…rappers at any depth ### What problem does this PR solve? Issue Number: None Related PR: apache#69845 Problem Summary: `TableColumnMapper` resolves an Iceberg Parquet field id through an ID-less complex wrapper at any depth when `allow_idless_complex_wrapper_projection` is on: Iceberg's Parquet projection keeps such a wrapper for a selected nested id (`ParquetRecursivelyRetainsIdlessWrapperForSelectedLeafId`). The timestamp marker walk added earlier in this PR (`annotate_matched_timestamp_semantics`) allowed the wrapper only at the root and passed `false` for every STRUCT child. In a file whose outer struct has a field id, the struct inside it has none, and the leaf has one again, an INT96 timestamptz leaf was still mapped but never marked. It decoded as a UTC wall clock, and the projection converted that in the session time zone, so in Asia/Shanghai it read eight hours early. An equality key through the same path compared that wall clock with the adjusted-UTC tombstone an Iceberg writer stores, and the tombstone missed its row. The walk now passes the mapper's flag down to every level, so it follows the same path as the mapper. BY_NAME mapping is unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - New `IcebergV2ReaderTest.IcebergInt96TimestampTzUnderIdlessWrapperReadsInstantWithTableMarker`: reads such a file in Asia/Shanghai, and applies an equality delete on the nested key with the key projected and hidden - `IcebergV2ReaderTest`: 122 tests pass - Behavior changed: No (within this PR's fix) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed review round 1 in f2b97b1.
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Request changes: one new P1 was verified at head f2b97b1. When an equality key is absent beneath an existing nullable struct, its ancestor read bypasses the new timestamp-marker projection. An otherwise valid older Parquet file with an INT96 timestamptz sibling then fails with a file-block/reader type mismatch. The inline finding describes the trigger and fix direction.
All 11 changed files and their affected FE schema, mapper, data/delete reader, physical conversion, pruning and materialization paths were reviewed statically. Three reviewers completed a second round with NO_NEW_VALUABLE_FINDINGS; all candidates and risk foci have explicit dispositions. Static review converged after 2 rounds. This does not mean the reported bug is fixed.
Existing finding 4414291784 is fixed at this head: the ID-less-wrapper option now propagates through every recursive annotation level, with an added nested INT96 test covering projected and hidden present keys. It is neither reposted nor retained as a current blocking comment. The new finding concerns the separate missing-key ancestor path and requires no ID-less wrapper. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and correctness: the FE marker reaches ordinary matched Parquet leaves, and inspected tests assert instants and delete survivors. The missing-key ancestor composition remains incorrect as reported.
- Scope and reuse: changes are focused; current-schema/identity extraction is shared, and the Paimon timestamp helper move preserves behavior.
- Concurrency and locks: annotation and request overrides operate on per-file/per-reader copies; no new threads, shared mutable cached metadata, locks or lock-order dependencies were found.
- Lifecycle and memory safety: schema/request values own their child trees; native readers copy cached physical metadata before type synchronization. No new dangling reference, ownership cycle or static-initialization dependency was found.
- Configuration: none added. Both existing timestamp-mapping flag values and markerless requests were inspected.
- Compatibility: the existing optional marker is reused; old-FE requests without it retain prior file types. No persisted format or public connector SPI change; mixed-version execution and binary ABI were not tested.
- Thrift/protobuf: no definitions changed, so the cross-branch field-ID audit is not applicable.
- Parallel paths: normal, projected/hidden present equality keys, tombstones, nested mapping and Paimon were traced. The omitted missing-key path is the P1. ORC wall-clock DST ambiguity with legacy mapping disabled and position-delete-system-table INT96 row semantics are preexisting limitations.
- Conditions and invariants: field-ID/name matching reuses mapper resolvers, wrapper recursion agrees, and unmarked/TIMESTAMP_NS leaves preserve physical types. Known malformed containers return errors.
- Test coverage: six new BE cases and the FE recursive-marker case were inspected, including matching modes, old markers, nested wrappers, mapping flags and DST folds. The missing nested key plus marked sibling combination is absent and should be added.
- Expected results: timestamp strings and survivor IDs are consistent with the fixture semantics; no regression output files or source-.cpp test includes changed.
- Observability: existing column/type errors and profile ownership remain intact; no new I/O or asynchronous mechanism requires separate counters.
- Persistence, transactions and failover: no edit-log, publication, visible-version, delete-bitmap or transaction-state change.
- Data writes and atomicity: production writes are unchanged. The reported read-side violation fails explicitly rather than silently continuing.
- FE-BE propagation: normal, pinned, lazy and retained equality schemas recurse through the same FE builder. The incomplete-key ancestor projection is the verified missing transport step.
- Performance: added work is bounded per-split schema traversal/type construction, with no new per-row work or extra file I/O identified. No benchmark claim is made.
- Further checks: late-filter layout guards, implicit MAP-key reads, defaults/NULL ancestors, multiple/empty deletes, projection merging and cached-schema ownership were rechecked. No further new defect was substantiated.
Compatibility checks used Iceberg's field-ID projection and nullable-ancestor equality-delete rules in the Iceberg specification.
Validation limits: this was static inspection only; no builds, tests, runtime checks, benchmarks or binary audits were executed. Added fixtures are Arrow-generated, and this review did not validate real external-writer artifacts, FE-to-BE integration, or an exhaustive marker-aware null/container, pruning-enabled/disabled, mixed-format and split-reuse matrix. These are execution-validation limits; no source-review candidate remains unresolved.
Review converged after 2 round(s).
| // predicate columns here and must keep the marker its mapping carried. | ||
| auto projection = format::LocalColumnIndex::top_level(field_column_id); | ||
| format::attach_timestamp_semantics(root->timestamp_is_adjusted_to_utc, | ||
| root->children, &projection); |
There was a problem hiding this comment.
[P1] Preserve markers on missing-key ancestor projections
Please apply the same timestamp override in _build_missing_equality_delete_key_expr, which still adds its physical ancestor through the markerless _append_file_scan_column at line 2040. For an older Parquet file containing nullable payload {ts: INT96 timestamptz}, a newer schema adding payload.k INT DEFAULT 7, and a nonempty equality delete on k, SELECT id reads payload to preserve ancestor NULLs. The new annotation makes its file-block type STRUCT<TIMESTAMPTZ>, but this request carries no descendant marker, so Parquet still builds STRUCT<DATETIMEV2> and check_block_column_type fails the query. Reuse the annotated projection for this incomplete-key branch too, and extend the missing-nested-key test with a timestamp sibling hidden from the SELECT.
There was a problem hiding this comment.
Fixed in 4aa737f. The projection that the hidden-key path built (top_level plus attach_timestamp_semantics from the annotated file column) is now equality_delete_root_projection(), and _build_missing_equality_delete_key_expr uses it to add the missing key's ancestor as a predicate column, the same way the hidden key and the delete file's keys are added. The failure was wider than SELECT id. When the query projects payload (or SELECT *), the mapper's non-predicate projection of payload does carry the marker, but adding the root as a predicate column for the ancestor read replaced it (FileScanRequestBuilder::_add_column drops the non-predicate entry), so those queries hit the same check_block_column_type error. The key does not need a default either: the ancestor is read whatever the default is. The new IcebergV2ReaderTest.IcebergMissingNestedEqualityKeyAncestorKeepsInt96TimestampTzMarker writes an older file with a nullable payload {existing INT, ts INT96 timestamptz} and deletes on the later-added payload.k (default 7) in Asia/Shanghai. With payload hidden or projected, the only survivor is id 2, the row under the NULL parent. With a tombstone that matches nothing, all three rows survive and the projected ts reads as the instant. A plan without the marker reads as before. Reverting only the ancestor read to _append_file_scan_column makes the new test fail at its first marked case with that error (Parquet column payload ... is read as Nullable(Struct(existing:Nullable(INT), ts:Nullable(DateTimeV2(6)))) but its file block column is Nullable(Struct(existing:Nullable(INT), ts:Nullable(TimeStampTz(6))))), after the unmarked case passes.
TPC-H: Total hot run time: 28313 ms |
TPC-DS: Total hot run time: 151164 ms |
ClickBench: Total hot run time: 24.35 s |
FE UT Coverage ReportIncrement line coverage `` 🎉 |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
… a missing equality key ### What problem does this PR solve? Issue Number: None Related PR: apache#69845 Problem Summary: When an Iceberg equality-delete key is a nested field that an older data file does not store, but the file does store the key's parent struct, `_build_missing_equality_delete_key_expr` reads the whole parent so that the key stays NULL under a NULL parent. It added that read through `_append_file_scan_column`, a bare top-level projection without timestamp markers. Earlier in this PR, `annotate_file_schema` started rewriting each marked timestamptz leaf of the Parquet file schema to TIMESTAMPTZ. The file block is built from that schema, but the Parquet reader takes the timestamp semantics only from the projection. Reproduction: an older Parquet file holds a nullable `payload {existing INT, ts INT96 timestamptz}` (Spark's encoding), the table later added `payload.k`, and a non-empty equality delete is keyed on `payload.k`. Scans of that file failed in `check_block_column_type`: Parquet read `payload` as STRUCT<existing, DATETIMEV2(6)> into a STRUCT<existing, TIMESTAMPTZ(6)> file block. Projecting `payload` did not help: adding the root as a predicate column replaced the projection its mapping had marked. Before this PR the file schema kept the physical types, so the same read succeeded. The fix moves the annotated projection the hidden-key path already built (`top_level` plus `attach_timestamp_semantics` from the annotated file column) into `equality_delete_root_projection()`. The missing key's ancestor, the hidden key and the delete file's keys all use it now. The ancestor is still added as a predicate column through `FileScanRequestBuilder`, so a root already in the request is merged or moved as before. A plan without markers builds the same request as before. ### Release note None ### Check List (For Author) - Test: Unit Test (to be run) - New `IcebergV2ReaderTest.IcebergMissingNestedEqualityKeyAncestorKeepsInt96TimestampTzMarker`: in Asia/Shanghai, with the key's parent holding an INT96 timestamptz sibling, checks the surviving rows with the parent hidden and projected, that the projected sibling reads as an instant, and a plan without the marker - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed review round 2 in 4aa737f.
|
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
TPC-H: Total hot run time: 28739 ms |
TPC-DS: Total hot run time: 150837 ms |
ClickBench: Total hot run time: 24.25 s |
FE UT Coverage ReportIncrement line coverage `` 🎉 |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
/review |
There was a problem hiding this comment.
No new actionable findings were verified at head 4aa737f. The complete 11-file aggregate diff and affected FE schema, mapper, data/delete reader, physical conversion, pruning and materialization paths were reviewed statically. The review converged in the preserved first round: the complete reviewer and both risk reviewers finished with NO_NEW_VALUABLE_FINDINGS, all candidates have evidence-backed dispositions, and no source-review question remains unresolved. The interrupted schema assignment was recovered within that same round.
Both existing P1 findings are fixed at this head. 4414291784 is addressed by forwarding the ID-less-wrapper option through every recursive annotation level. 4419065184 is addressed by using the shared marker-bearing root projection for missing-key ancestor reads as well as present keys and tombstones. The added tests inspect hidden/projected keys and nullable ancestors. Neither issue is reposted or retained in existing_blocking_comment_ids. No additional user review focus was supplied.
The inspected interaction is: FE recursive TField marker -> scan schema dictionary -> fresh per-reader file schema -> Iceberg annotation -> mapper/equality-root projection -> Parquet semantic schema and decoder -> file block/delete comparison -> table materialization.
Critical checkpoint conclusions:
- Goal and correctness: the marker supplies instant semantics for INT96 timestamptz data and equality keys. Inspected assertions cover UTC instant strings and delete survivors; the release note describes this behavior. No additional introduced correctness defect was verified.
- Scope and reuse: changes are focused. Current-schema/identity extraction and timestamp type rewriting reuse shared helpers; the moved Paimon implementation preserves behavior apart from generalized error wording.
- Concurrency and locks: annotation occurs on fresh reader-owned schema during open. No new thread, shared cached-schema mutation, lock, critical section or lock-order dependency was introduced.
- Lifecycle and memory safety: schema definitions own their child trees; current-schema pointers retain the existing scan-parameter lifetime, with nonempty/null guards at callers. No new dangling reference, ownership cycle or cross-translation-unit static initializer was found.
- Configuration: no setting was added. Current catalog planning always enables physical instant mapping; raw legacy false properties do not produce marked/false scan parameters. Older markerless requests retain prior behavior.
- Compatibility: the existing optional marker is reused, with no wire definition, persisted file format or connector SPI change. Unmarked plans and unmarked/TIMESTAMP_NS leaves preserve their file types; ORC retains its separate timestamp-instant path. Runtime upgrade and binary compatibility were not tested.
- Thrift/protobuf IDs: no definitions changed; cross-branch field-ID auditing is not applicable.
- Parallel paths: ordinary data, hidden/projected equality keys, missing-key ancestors and tombstone roots were traced. Paimon retains equivalent rewriting; native metadata is copied before reader-local type synchronization. No new omitted supported path was verified.
- Conditions and invariants: annotation reuses mapper field-ID/name resolvers and the recursive wrapper flag. Positional IDs precede annotation. Comments explain INT96 ambiguity, nullable ancestors and old-plan/NS handling; malformed container errors propagate.
- Test coverage: all seven added BE cases and the FE recursive-marker case were inspected, including identity modes, nested STRUCT/LIST/MAP annotation, wrappers, precision/nullability, old plans, both synthetic mapping flags, hidden/projected/missing keys and DST folds. Existing Paimon tests reach the extracted helper. None were executed here.
- Expected results: inspected UTC timestamps and survivor IDs agree with fixture semantics. No regression output file changed, and no new test includes a source .cpp file.
- Observability: work remains inside existing reader-open/schema timing and column/type diagnostics. No new asynchronous operation, I/O mechanism or per-row logging requires additional metrics.
- Persistence, transactions and failover: no EditLog, snapshot selection, visible-version, publish, delete-bitmap or transaction-state change was introduced.
- Data writes and atomicity: production changes affect reads; new fixture writers are test-only. No new production write/commit or crash-cleanup path was introduced.
- FE-BE propagation: ordinary, pinned, lazy and historical equality-key dictionaries recurse through the same builder. Root and descendant markers reach data and delete scan requests, including replacement of projected roots by predicate roots.
- Performance: added traversal/type rebuilding is bounded by schema size at reader open or delete-cache load. No new per-row schema work, redundant file I/O or unbounded retained allocation was identified; no performance result is claimed.
- Further checks: predicate localization, footer/page statistics, dictionary/Bloom boundaries, aggregates, projection merging, NULL/default ancestors and split-local ownership were rechecked. The proposed mixed-format false-flag issue is prevented by the current FE producer invariant. A partial complex-MAP-key example already violates unchanged TableReader child-count invariants before this PR, so its earlier marker mismatch does not establish a new regression.
Complex map-key compatibility was checked against the Iceberg nested-type specification, which permits nested key types; the preexisting materialization limitation above is not claimed fixed.
Validation limits: static source inspection only. No builds, tests, runtime checks, benchmarks or binary audits were executed. Arrow-generated fixtures and the author's reported runs do not independently validate actual Spark/Hive artifacts, FE-to-BE integration, mixed-version/format execution, or an exhaustive pruning-enabled/disabled, split-reuse and performance matrix in this review. These are execution-validation limits; no introduced-bug candidate or assigned source scope remains unresolved.
Review converged after 1 round(s).
924060929
left a comment
924060929
left a comment
There was a problem hiding this comment.
Iceberg timestamptz fields represent instants, while Parquet INT96 has no timezone annotation. This change carries the table semantics from the shared FE schema builder through scan parameters into BE file-schema annotation and marker-bearing projections for data and equality-delete reads.
Static review completed for head 4aa737f against base 2a4191c, focusing on FE and the necessary FE/BE contract. No actionable introduced P0/P1/P2 issue was found. Both previous findings, 4414291784 and 4419065184, are fixed in the current code and have targeted test coverage. APPROVE.
No builds or tests were run during this review. The supplied current-head context reports successful P0 Regression and a failed FE coverage check. Required-check rules were not supplied; this code approval does not establish merge readiness.
Automated FE-focused static review (GPT-6.1 Sol, medium). No build or tests were run.
What problem does this PR solve?
Issue Number: None
Related PR: #69711
Problem Summary:
In short. An Iceberg
timestamptzcolumn stored as Parquet INT96, the encoding Spark and Hive use for timestamps, reads shifted by the session time zone's offset in any non-UTC session: in Asia/Shanghai, 2024-01-01T00:00:00Z reads as2024-01-01 00:00:00+08:00, eight hours early, while Iceberg itself reads2024-01-01 08:00:00+08:00. Equality deletes on such a key miss their rows for the same reason. This PR has the FE mark eachtimestamptzfield of the scan's schema dictionary, and FileScannerV2 then decodes the matching Parquet columns of data files and equality-delete files as instants, which is how Iceberg reads them. It came up in the review of #69711.Background.
timestamptype (an instant) as INT96 by default, andadd_files,migrateandsnapshotattach such files to Iceberg tables unchanged, astimestamptzcolumns.timestamptzfield reads INT96, and INT64 with or without the UTC annotation, as an instant.parquet_timestamp_semantics_version = 1for every file scan. Parquet scans therefore always use FileScannerV2, and INT96 decodes as a UTC wall clock (DATETIMEV2) unless the table format supplies the semantics. Paimon has the same ambiguity (TIMESTAMP and TIMESTAMP_LTZ share INT96) and solves it withtimestamp_is_adjusted_to_utcon each field of its schema dictionary, whichPaimonReaderapplies to the file schema. The Iceberg FE never set that field.The problem, and what it cost. For a
timestamptzcolumn, the projection casts the INT96 wall clock toTIMESTAMPTZin the session time zone, so the instant moves by the session offset. Reproduced on master with an INT96 file attached likeadd_filesand an Iceberg-written INT64 file holding the same instants, both in one table:2024-01-01 08:00:00+08:002024-01-01 08:00:00+08:002024-01-01 00:00:00+08:00, 8 hours early2024-01-01 08:00:00+08:00The result is the same with field ids in the file, with only the table's name mapping, whatever the catalog's
enable.mapping.timestamp_tzsays (master always mapstimestamptzto TIMESTAMPTZ), and withenable_file_scanner_v2on or off. The same shift breaks equality deletes, which compare the projected values:timestamptzkey, non-UTC sessionWrong values and missed or extra deletes come back without any error.
How this PR fixes it.
IcebergSchemaUtils.buildFieldsetstimestamp_is_adjusted_to_utc = trueon everytimestamptzfield, at every nesting level; the dictionary also carries the hidden equality-delete keys.timestampfields stay unmarked: marking themfalsewould make BE read the Parquet statistics of an adjusted-UTC INT64 file as UTC wall clocks while its rows still decode as session wall clocks, so row-group pruning could drop matching rows.IcebergTableReader::annotate_file_schemacopies the marker onto the Parquet file column thatTableColumnMapperwill select for each field, using the mapper's own matchers (find_column_by_field_id/find_column_by_name) after the positional fallback ids are stamped, so field ids, the table's name mapping and positional ids all work. Like the mapper, it accepts an ID-less wrapper at every depth whenallow_idless_complex_wrapper_projectionis on. It then rewrites the column type with the helperPaimonReaderalready used, moved toschema_history_utilasapply_file_timestamp_semantics. The mapping, the scan request andParquetReaderall seeTIMESTAMPTZ, and INT96 decodes as an instant. Only aDATETIMEV2orTIMESTAMPTZleaf takes the marker; aTIMESTAMP_NSleaf keeps its nanoseconds. Columns without a marked leaf keep the types the file reader reported, and a plan from an older FE carries no marker and reads as before.ColumnMapping: a hidden key column, the parent struct that is read for the NULLs of a nested key the data file lacks, and the key columns of an equality-delete file. Their projections now carry the markers of the annotated file column (equality_delete_root_projection, built onattach_timestamp_semantics, which the mapper now exports). Without them Parquet would decode an INT96 leaf asDATETIMEV2under aTIMESTAMPTZfile schema, and the read would fail its type check. That holds even when the query projects the column, because reading it as a predicate column replaces the projection its mapping marked. Equality-delete files get the same annotation, so tombstones and data keys of atimestamptzfield are both instants. TheDATETIMEV2/TIMESTAMPTZcomparison rule from [fix](be) Compare Iceberg equality-delete keys in the wider of the delete and data file types #69711 stays for plans from older FEs.find_current_schemaandbuild_schema_identity_from_external_field(now also carrying the marker) move toschema_history_util;TableReader's four copies of the current-schema lookup use the helper.Results.
IcebergSchemaUtilsTest#timestamptzFieldsCarryInstantMarkerAtEveryLevel: top-level, struct, list and maptimestamptzfields are marked;timestampandtimestamp_nsfields are not, under both values of the mapping flag.iceberg_reader_test.cpp, all failing without the BE change: the file-schema annotation (field ids at every level,TIMESTAMP_NSuntouched, ORC untouched); INT96timestamptzin Asia/Shanghai with field ids, name mapping and positional ids, with and without the marker; INT96 inside a struct and a list; an INT96 equality key against an Iceberg tombstone, hidden or projected; and the DST fall-back above. Leaving the delete files unannotated fails the DST case (the second tombstone deletes the first row); projecting a hidden key or a tombstone without its marker fails both equality-delete cases with a read error; marking aTIMESTAMP_NSleaf fails the annotation case. With neither data nor delete files annotated (the reader before this PR), the INT96 key deletes nothing and the DST case deletes both rows. The existing case for an older FE's plan (no marker) still passes.timestamptz. It checks the survivors with the parent hidden and projected, that the projected timestamp reads as an instant in Asia/Shanghai, and a plan without the marker. It fails with the reader's type check when the parent is read without the marker.add_files(one table with field ids in the file, one relying onschema.name-mapping.default) and an Iceberg-written INT64 file with the same instants, read through three catalogs (enable.mapping.timestamp_tzunset,true,false), withenable_file_scanner_v2on and off, in UTC and Asia/Shanghai: 24 combinations. Before this PR the INT96 rows were 8 hours early in every Asia/Shanghai combination (8 of the 16 measured); with it the INT96 rows equal the INT64 rows in all 24, and the INT64 rows and the UTC results are unchanged.branch-4.1 and branch-4.2 have the same code.
Classes, and how they call each other.
IcebergSchemaUtils(FE): builds the schema dictionary; now markstimestamptzfields.schema_history_util:find_current_schema,build_schema_identity_from_external_field, andapply_file_timestamp_semantics(moved fromPaimonReader).IcebergTableReader:annotate_file_schema→_apply_positional_id_fallback(the existing logic) and_apply_table_timestamp_semantics;_append_equality_delete_predicates(hidden keys),_build_missing_equality_delete_key_expr(the parent of a missing nested key) and_load_equality_delete_file(tombstones) project their columns with the markers throughequality_delete_root_projection.column_mapper:attach_timestamp_semanticsis exported.ORC files are not changed: ORC's file type already separates
TIMESTAMPfromTIMESTAMP_INSTANT.Release note
Fix Iceberg
timestamptzcolumns stored as Parquet INT96 (written by Spark or Hive and attached withadd_files,migrateorsnapshot) reading shifted by the session time zone's offset in non-UTC sessions, and equality deletes on such keys missing their rows.Check List (For Author)
run-be-ut.sh --run --filter='IcebergV2ReaderTest.*:Paimon*ReaderTest.*:Hudi*Test.*:TableReaderTest.*:ColumnMapper*:FlussUnionLakeReaderTest.*:EqualityDeletePredicateTest.*:*PositionDelete*': 532 of 532 pass on macOS arm64 (ASAN), including the 5 new cases. After the first review round,IcebergV2ReaderTestpasses 122 of 122, including the new nested ID-less wrapper case. It fails when the walk accepts the wrapper only at the root. After the second review round, the whole filter passes 534 of 534 andIcebergV2ReaderTest123 of 123, including the new missing nested key case. It fails when the missing key's parent is read without the marker. They ran on a local base; the commit was then applied to master2a4191c1dc9without conflicts, and the changed sources and their test files were syntax-checked against that tree with the UT build flags.mvn -pl :fe-connector-iceberg -am package -Dtest=IcebergSchemaUtilsTest: 35 of 35 pass, checkstyle passes.timestamptzfields decode as instants by the table type, as Iceberg reads them: INT96 no longer shifts by the session offset, an INT64 timestamp without the UTC annotation under atimestamptzfield reads its value as UTC, and equality deletes ontimestamptzkeys compare instants. Plans from an older FE are unchanged.Check List (For Reviewer who merge this PR)