branch-4.1:[feature](iceberg) Enforce NULL defaults for VARIANT - #67323
Open
hubgeter wants to merge 1 commit into
Open
branch-4.1:[feature](iceberg) Enforce NULL defaults for VARIANT#67323hubgeter wants to merge 1 commit into
hubgeter wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review status: complete. Approval opinion: looks good from a code-review perspective; no blocking issue was found on head d2b6763.
Critical checkpoints:
- Iceberg defaults: optional VARIANT initial and write defaults remain typed SQL NULL, while non-NULL defaults are rejected in FE and defensively in BE. Explicit Variant values, including a Variant payload representing JSON null, remain distinct from SQL NULL.
- Schema and type identity: the FE to TField carrier remains recursive, and the BE correctly preserves the compute-V2 VARIANT representation across the legacy TColumnType boundary for root and nested fields.
- Reader materialization: missing historical fields, required-field failures, nested struct defaults, schema-history lookups, and equality-delete consumers follow the same table-layer default contract. Historical ORC rows remain supported without enabling physical ORC Variant.
- Architecture and lifecycle: the change stays within schema/default and TableReader responsibilities, adds no reverse format-layer dependency, and does not alter split, EOF, cleanup, caching, or profile behavior.
- Performance and observability: the added checks run during schema/default setup, not in per-row or batch hot paths; no counter or attribution path is removed.
- Coverage: focused FE and BE tests cover top-level and nested null/default behavior and malformed metadata. The external regression covers historical Parquet and ORC rows, INSERT, UPDATE, MERGE, DEFAULT, SQL NULL versus Variant JSON null, invalid DDL atomicity, and Spark interoperability. No local builds or tests were run, per the review-runner instruction.
User focus: no additional focus points were provided.
Convergence: the normal FE/integration review, normal BE/materialization review, and separate risk-focused review all returned NO_NEW_VALUABLE_FINDINGS in Round 1. The final changed-file, unresolved-candidate, live-head, and live-inline-thread sweep was clean. No inline comments are needed.
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?
Related PR: #66538
Problem Summary:
Iceberg V3 requires both
initial-defaultandwrite-defaultof a VARIANT field to be NULL. After adding general Iceberg V3 default-value support, Doris still needs explicit VARIANT handling to avoid accepting unsupported non-NULL defaults or reconstructing a compute-V2 VARIANT as the legacy type.This PR:
initial-defaultandwrite-defaultvalues in FE.DEFAULT, SQL NULL versus JSON null, invalid DDL, and Spark interoperability.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)