Repository navigation
[fix](iceberg) Preserve timestamp precision and semantics in metadata scans - #68985
Merged
Merged
Conversation
… scans ### What problem does this PR solve? Related PR: apache#68276 Problem Summary: Iceberg files metadata partition timestamps lose microseconds through millisecond conversion, and timestamp-without-zone values incorrectly follow the session timezone. Decode signed epoch microseconds with floor division and propagate Iceberg types through nested metadata values while retaining projected row ordering. ### Release note Preserve microsecond precision and timestamp-without-zone wall-clock values in Iceberg metadata table output. ### Check List (For Author) - Test: Focused Java unit tests (2 passed); external system table projection and position deletes suites passed; files timestamp partition regression generated and verified against golden output (1 suite passed). - Behavior changed: Yes; correct timestamp precision and timestamp-without-zone timezone handling. - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
4 tasks done
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28729 ms |
Contributor
TPC-DS: Total hot run time: 151929 ms |
Contributor
ClickBench: Total hot run time: 24.71 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
### What problem does this PR solve? Related PR: apache#68985 Problem Summary: External regression build 1067800 failed test_iceberg_write_stats2 because its readable_metrics golden output still expected timestamp-without-zone bounds shifted by eight hours. Regenerate the Parquet and ORC expectations using the corrected metadata scanner. Only the timestamp lower and upper bounds change. ### Release note None ### Check List (For Author) - Test: Regression test; test_iceberg_write_stats2 passed normal golden comparison (1 suite, 0 failures, 0 skips), using the deployed PR scanner plugin and personal Iceberg profile. - Behavior changed: No; align test expectations with the timestamp semantics fixed in this PR. - Does this need documentation: No
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29222 ms |
Contributor
TPC-DS: Total hot run time: 151947 ms |
Contributor
ClickBench: Total hot run time: 24.51 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Collaborator
|
/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 of all six changed files at 791e9298078c9ef95d7ffe2d459d418ffd5403ae found no substantiated new defect. No existing inline finding or additional user focus was supplied; a live inline-comment read also found none.
The reviewed path is SQL metadata-table projection -> ordered Iceberg task -> JNI positional row access plus logical type lookup -> nested ColumnValue wrappers -> DATETIME/TIMESTAMPTZ vector packing. Signed floor division/modulus preserves negative and positive microsecond fractions. NTZ reconstruction preserves wall-clock fields; instant timestamps retain UTC storage and session-zone presentation.
| Required checkpoint | Conclusion |
|---|---|
| Goal and proof | The implementation addresses precision loss and NTZ zone shifts. Two new helper unit tests and the identity-partition regression cover the intended values; this review did not execute them. |
| Scope and clarity | Two production Java classes, matching unit/regression tests and timestamp goldens; shared timestamp arithmetic avoids duplication. |
| Concurrency and locks | No new thread, lock or shared mutable state; the final Type array is scanner-local. |
| Lifecycle and static initialization | No new resource ownership or static initializer. Existing iterator cleanup and vector-table release-on-exception remain in use. |
| Configuration and dynamic changes | No configuration added; existing session zone is passed through. The tested mapping property is a legacy option at this head, so its two settings correctly produce identical results. |
| API, storage and rolling compatibility | Old constructors remain; no plugin SPI, serialized task shape or storage-format change. Existing Iceberg serialization compatibility is untouched. |
| Thrift/protobuf ids | No changed definitions; cross-branch field-id audit is not applicable. |
| Parallel paths | Both BE JNI reader generations share this scanner. Regular Iceberg data and native position-delete readers use separate conversions and are outside the stated fix. |
| Special conditions and errors | TimestampType/shouldAdjustToUTC selects wall-clock versus instant semantics; null-Type overloads preserve legacy behavior. Missing schema fields fail explicitly, and vector appends check nulls before getters. |
| Test coverage and negative cases | Signed pre-epoch/submillisecond values, UTC/Shanghai, NTZ/TZ, reversed struct indexes, arrays, map keys/values and a null partition are covered by checked-in cases. Full partition and selected child expressions are exercised. |
| All changed expected results | All eight new golden blocks (56 rows) match independent static arithmetic; both changed write-stat bounds match their unchanged little-endian raw microsecond bounds and inserted DATETIME literals. Result sorting makes random file names irrelevant. |
| Observability | Pure decoding adds no new distributed operation; existing field-name exceptions, JNI error propagation and query-profile timing are adequate. |
| Transactions, persistence and failover | No transaction, EditLog, version-visibility, snapshot-selection or delete-bitmap change. |
| Writes, atomicity and crashes | Production changes only decode metadata reads; existing exception cleanup remains intact. Test fixture writes use the existing Spark helper. |
| FE-BE variable passing | No new wire variable; Type metadata comes from the existing serialized task. |
| Performance and memory | Schema/name lookup is cached and performed at scanner construction, not per row; arithmetic and child-type lookup are bounded. No material new buffers, remote IO or tracked native ownership. Iceberg schema cache. |
| Other issues and user concerns | All changed-file scope was inspected, with separate implementation, test and risk reviewers. No extra user focus, verified candidate or duplicate blocking finding. |
Risk analysis checked both full-schema StaticDataTask and projected ManifestReadTask layouts, nested ordinal/type pairing, readable-metrics bounds, name casing and evolved partition schemas. FE keeps full nested metadata fields and disables physical nested pruning for Iceberg metadata readers; the child regression tests expression projection. The SDK converts metadata-family millisecond times to microseconds before row decoding. Static task projection, snapshot timestamp rows.
Validation limits: static analysis only, as required. No build, test, cluster run or source modification was performed; checked-in goldens and author-reported runs do not independently prove execution or generation provenance. Added tests do not directly exercise every metadata-table family, physically pruned later children (disabled by current metadata-reader contract), DST/historical-offset zones, empty multi-level containers or invalid field/zone diagnostics. These are validation limits with no concrete incorrect supported behavior identified; they are not unresolved findings.
Convergence: true, after 1 review round. Implementation, test and dedicated risk reviewers all returned NO_NEW_VALUABLE_FINDINGS; main independently verified their evidence, completed the full-scope final sweep and closed both initial risks. No unresolved candidate or changed-file coverage gap remains. New inline findings: 0. Confirmed existing blocking findings: 0.
Review converged after 1 round(s).
Gabriel39
approved these changes
Oct 15, 2026
Gabriel39
added a commit
that referenced
this pull request
Oct 19, 2026
…69171) ## Problem and behavior `branch-4.2` is missing several external type mapping fixes already merged into master. This adapted backport aligns these paths while retaining the branch's existing VARBINARY and TIMESTAMPTZ support: | Path | Result | Upstream | | --- | --- | --- | | Parquet/ORC file TVFs | Unannotated binary fields infer VARBINARY with the legacy option absent, false, or true, including nested fields | #68786 | | Oracle JDBC | BINARY_FLOAT maps to FLOAT; BINARY_DOUBLE maps to DOUBLE | #68200 | | SQL Server JDBC | Resolve user-defined aliases by JDBC type code; strip IDENTITY decoration only when the base type agrees with that code | #67916 | | Iceberg metadata scanner | Preserve signed epoch microseconds and timestamp-without-zone wall-clock values through nested structs, arrays, and maps | #68985 | Logical UUID annotations retain their existing file-format semantics instead of being overridden by the raw-binary default. Parquet schema discovery retains canonical STRING values and ORC retains native UUID values. Modern and legacy Parquet ENUM annotations remain STRING. Full external native UUID alignment and nanosecond timestamp support in #69163 remain separate. Golden results in this PR target its own branch-4.2 build and do not assume those reader changes have landed. The Iceberg scanner keeps this branch's full-schema source indexes rather than copying master's projected-row indexing. The master JDBC scanner architecture remains outside this backport. Oracle connections enable binary IEEE floating-point bindings so valid BINARY_DOUBLE values outside NUMBER’s range can be written without overflow. Binary TVF columns display hexadecimal values. Direct CTAS into an internal VARBINARY column is rejected, consistently with master. The CTAS partitioning/distribution suite explicitly decodes its UTF-8 binary fixture into STRING; the dedicated binary suite checks rejection of direct VARBINARY CTAS and byte-preserving HEX materialization. Paimon raw-file TVF expectations now preserve binary bytes even without the obsolete mapping option. Variant shredding assertions likewise require binary metadata and residual values while preserving all physical-layout and logical round-trip checks. ## Validation - Reproduced the Oracle failure against a real Oracle database: the old executor throws `IllegalArgumentException: Overflow` from `setDouble(1e200)`; the fixed executor preserves large/small doubles, floats, and NULLs through INSERT/readback. - Added shared-schema coverage for modern/legacy ENUM annotations and UUID precedence, plus an ENUM-only CTAS regression with both legacy option values. - Updated scalar/nested UUID regression coverage so a raw-binary default cannot override logical UUID annotations. - Built FE and the full ASAN BE from this PR worktree. FE Checkstyle and clang-format 16 passed. - Five Oracle Java tests passed against a real Oracle database, including the optional INSERT/readback integration test; the new Doris JDBC regression block also passed. - Two focused production-library-linked Parquet schema tests passed; the new logical-annotation test failed against the previous implementation as expected. All four modified BE test translation units passed compilation checks. - All seven UUID regression suites and the complete HDFS Parquet group 6 suite passed actual generation followed by independent comparison runs, with zero skipped suites. Updated golden blocks were taken from those executions. The previous coverage build stopped while downloading Maven (`wget` exit 4), before project compilation. This is an infrastructure failure and is retriggered with the updated source. Muted unrelated compaction/cache/S3 timeout cases are not changed here.
This was referenced Nov 12, 2026
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.
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.partitiontimestamp of1234567microseconds is truncated to1970-01-01 00:00:01.234000rather than1970-01-01 00:00:01.234567. Switching from UTC to Asia/Shanghai incorrectly shifts timestamp-without-zone values by eight hours.This PR:
$files.partitionregression 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_deletesserialization 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
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.IcebergSysTableColumnValueTest: 2 tests passed; the FE/Java-extension reactor completed successfully.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:
Does this need documentation?
Check List (For Reviewer who merge this PR)