Repository navigation
Conversation
### What problem does this PR solve?
Issue Number: N/A (internal: DORIS-29054, DORIS-29055)
Related PR: N/A
Problem Summary:
Iceberg partition timestamps are epoch microseconds. Splitting negative values
with truncating division can produce an invalid nanosecond value during scan
planning. Iceberg `$files` metadata conversion also went through milliseconds,
which lost microsecond precision and applied the session time zone to
timestamp-without-zone values.
Use floor-based microsecond decomposition for partition timestamp conversion
and metadata-table conversion. Timestamp-without-zone now preserves its UTC
wall-clock value; timestamp-with-zone remains converted from its instant in
UTC.
### Release note
Fix Iceberg timestamp partition reads around the Unix epoch and preserve
microsecond precision in metadata tables.
### Check List (For Author)
- Test
- [x] Unit Test
- `IcebergUtilsTest#testPartitionTimestampSerializationPreservesNegativeMicros`
- `IcebergSysTableColumnValueTest`
- [x] Manual test
- Iceberg external `test_external_catalog_iceberg_partition` passed
with the personal Iceberg profile.
- `$files` external coverage requires the community pipeline because
the persistent personal BE uses a newer JNI plugin ABI than 4.1.
- Behavior changed:
- [x] Yes. Negative epoch-microsecond partition timestamps no longer fail;
metadata timestamps retain microseconds and timestamp-without-zone is not
shifted by the session time zone.
- Does this need documentation?
- [x] No.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review result: request changes for one P2 correctness issue.
Critical checkpoints:
- Negative microsecond arithmetic: the floor-division/modulus changes correctly preserve six-digit precision on both sides of the epoch in the FE serializer and Java metadata scanner.
- Timestamp semantics and nesting: timestamp-without-zone, timestamp-with-zone mapped to DATETIMEV2, and timestamp-with-zone mapped to TIMESTAMPTZ otherwise reach the intended getter/storage representation; list/map/struct type propagation and null handling remain aligned.
- Blocking gap: mapping-enabled timestamp-with-zone identity partitions can still be silently materialized as NULL in
$position_deletes.partitionafter the new negative conversion succeeds. See the inline comment. - Compatibility, lifecycle, and performance: no additional correctness, compatibility, lifecycle, configuration, or material performance issue was substantiated.
- Tests and validation: static review only; no build or test was run in this review environment. The new unit/golden coverage exercises the arithmetic and scanner paths but omits the failing mapping-enabled negative with-zone position-delete case. At submission time, compile, FE/BE unit, external, and P0 regression checks are passing;
check_coverage_feis failing and the automated review checks are still pending. - User focus: no additional focus was provided.
Review convergence completed in two rounds. All Round 2 normal and risk-focused reviews returned NO_NEW_VALUABLE_FINDINGS, every candidate was adjudicated, and the final changed-file sweep found no unresolved point beyond the inline issue. Reviewed head: afcfbb74cde8ca12d93b6b38541e2f7d96dea724.
Collaborator
Author
|
run buildall |
Collaborator
Author
|
run cloud_p0 |
4 of 16 tasks
Collaborator
Author
|
Superseded by #68985, which targets master and limits the fix to Iceberg metadata-scanner timestamp precision and timestamp-without-zone semantics. The current branch-4.1 already uses floor-based negative-microsecond decomposition and offset-bearing timestamptz partition serialization, so the old FE/native changes are not carried into the new PR. The remaining $files metadata-scanner issue on branch-4.1 is not claimed fixed by this closure; a branch-4.1 backport should follow the master fix after merge. Closing this conflicting branch-4.1 PR in favor of the master-first workflow; keeping the branch for reference. |
Gabriel39
pushed a commit
that referenced
this pull request
Oct 15, 2026
… scans (#68985) ### What problem does this PR solve? Issue Number: N/A Related PR: #68276 (supersedes its metadata-scanner fix; this PR targets master) Problem Summary: Iceberg metadata table timestamps are encoded as epoch microseconds. The JNI scanner converts them through milliseconds, losing fractional precision. It also applies the session time zone to timestamp-without-zone partition values, changing their wall-clock value. For example, a `$files.partition` timestamp of `1234567` microseconds is truncated to `1970-01-01 00:00:01.234000` rather than `1970-01-01 00:00:01.234567`. Switching from UTC to Asia/Shanghai incorrectly shifts timestamp-without-zone values by eight hours. This PR: - Decodes signed epoch microseconds with floor division/modulus, preserving pre-epoch fractions. - Propagates Iceberg types through struct/list/map values to distinguish wall-clock timestamps from instants. - Resolves requested field types by name while continuing to read projected rows by position. - Adds unit tests and a `$files.partition` regression covering `-1000001`, `-1`, `1`, `999`, `1000`, `1234567`, and NULL, UTC/Asia/Shanghai, both timestamp mapping settings, and a projected partition child. Scope is the Java Iceberg metadata scanner. This PR does not modify regular data-table timestamp reads or the FE/native `$position_deletes` serialization path. There are no storage-format or configuration changes. ### Release note Fix microsecond precision loss and incorrect session-time-zone shifts for timestamp-without-zone values in Iceberg metadata tables. ### Check List (For Author) - Test - [x] Regression test - `test_iceberg_files_timestamp_partition`: generated the golden output, checked its timestamp values, and reran normal comparison successfully (1 suite, 0 failures/skips). - `test_iceberg_system_table_projection`: passed. - `test_iceberg_position_deletes_sys_table`: passed. - [x] Unit Test - `IcebergSysTableColumnValueTest`: 2 tests passed; the FE/Java-extension reactor completed successfully. - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason Local External tests used an isolated Iceberg fixture stack and the newly packaged metadata-scanner plugin deployed to the personal Doris BE. The full FE/BE deployment build was interrupted during C++ submodule downloads; FE/BE executables were not rebuilt for these runs. These are targeted local results, not full community External/P0/coverage results. - Behavior changed: - [ ] No. - [x] Yes. Metadata timestamps retain microseconds; timestamp-without-zone values no longer shift with the session time zone. Timestamp-with-zone values preserve their instant semantics. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
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.
fix Preserve timestamp partition micros
What problem does this PR solve?
Issue Number: N/A (internal: DORIS-29054, DORIS-29055)
Related PR: N/A
Problem Summary:
Iceberg partition timestamps are epoch microseconds. Splitting negative values with truncating division can produce an invalid nanosecond value during scan planning. Iceberg
$filesmetadata conversion also went through milliseconds, which lost microsecond precision and applied the session time zone to timestamp-without-zone values.Use floor-based microsecond decomposition for partition timestamp conversion and metadata-table conversion. Timestamp-without-zone now preserves its UTC wall-clock value; timestamp-with-zone remains converted from its instant in UTC.
Release note
Fix Iceberg timestamp partition reads around the Unix epoch and preserve microsecond precision in metadata tables.
Check List (For Author)
Test
IcebergUtilsTest#testPartitionTimestampSerializationPreservesNegativeMicrosIcebergSysTableColumnValueTesttest_external_catalog_iceberg_partitionpassed with the personal Iceberg profile.external_table_p0/iceberg/write/test_iceberg_write_stats2passed after rebuilding FE and BE from this branch.Behavior changed:
Does this need documentation?