[fix](iceberg) Preserve nested field case in created schemas - #67166
Merged
Gabriel39 merged 9 commits intoSep 3, 2026
Conversation
Gabriel39
requested review from
924060929,
CalvinKirs,
englefly,
morrySnow and
starocean999
as code owners
August 26, 2026 10:13
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes for two blocking issues.
- Correctness: CREATE/readback now preserve nested external spelling while runtime lookup and Thrift remain normalized, but Iceberg complex MODIFY regresses when an existing mixed-case field is redeclared with different case. The same root cause affects type-, comment-, and nullability-only changes under STRUCT/ARRAY/MAP.
- CI: the new public Nereids constructor fails the required CheckStyle gate because it lacks Javadoc.
Checkpoint conclusions: the dual-name design is otherwise focused; recursive Iceberg/Paimon CREATE paths, catalog display, runtime lookup, Gson backward fallback, and FE/BE serialization use the intended identity. No separate concurrency, lifecycle, configuration, transaction/partial-write, observability, storage-format, or material performance issue survived review. The added tests cover the converter boundary but not the failing real Iceberg evolution case. CheckStyle is failed; compile and FE UT are still pending, and no local build/test was run because the review bundle prohibits it. No additional user focus was supplied.
The review converged after two fresh rounds: all final full/risk reviewers returned NO_NEW_VALUABLE_FINDINGS after revalidating these two findings and their anchors.
### What problem does this PR solve? Problem Summary: Complex Iceberg MODIFY COLUMN operations treated a case-only spelling difference as a forbidden nested-field rename. Match existing fields case-insensitively, keep the persisted Iceberg field path spelling, reject case-insensitive append collisions, and document the dual-name StructField contract. ### Release note Fix mixed-case nested Iceberg field evolution while preserving external schema spelling.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Automated review completed after two rounds; every Round 2 normal and risk-focused reviewer returned NO_NEW_VALUABLE_FINDINGS. Two distinct P1 correctness gaps remain and are described inline.
Critical checkpoint conclusions:
- Goal and proof: the patch preserves nested spelling on direct structured CREATE/MODIFY and ordinary ASCII Iceberg evolution, but the goal is incomplete for Unicode field identity and Parquet FILE-TVF CTAS.
- Scope and clarity: the production changes are small and focused; the dual-name split is clear once each producer/consumer is traced.
- Concurrency, locking, and lifecycle: no Doris shared mutable state, thread, lock, static initialization, callback, or teardown path is added. Iceberg owns optimistic conflict detection for the single metadata commit.
- Configuration and compatibility: no configuration, storage-format, function-symbol, connector-SPI, or FE-BE protocol change is introduced. Old Gson records safely fall back from missing
originalNameto the historicalname; BE Thrift intentionally retains normalized runtime names. - Parallel paths and conditions: direct parsed CREATE/ADD/MODIFY, connector-imported schemas, plugin-table CTAS, ARRAY/MAP recursion, canonical existing Iceberg paths, appended-field collision checks, and dedicated rename were traced. FILE-TVF CTAS is the missing producer path, while the Unicode comparator uses a different identity from Iceberg and its sibling collision set.
- Error handling and data correctness: unsupported category, narrowing, MAP-key, rename/reorder, nullability, and append-collision cases fail before
commit(). The two findings instead allow successful operations that persist the wrong schema or mutate the wrong nested field. - Tests and results: the added unit/InMemoryCatalog tests correctly cover direct export plus ASCII type, comment, nullability, ARRAY/MAP, and collision behavior. They omit the Unicode comparator boundary and the Parquet FILE-TVF to external Iceberg CTAS path. No local build or test was run because the authoritative review prompt forbids it; live CheckStyle passes, while FE UT/compile/performance were still pending at submission time.
- Observability: existing field/path exceptions are sufficient for this metadata operation; no new metric or log is required.
- Persistence and writes: this is one atomic Iceberg metadata commit rather than a Doris transaction/EditLog change, but atomicity cannot correct the wrong field mapping identified inline.
- Performance: recursive diffing remains linear in complex-type size with a small set per STRUCT; no material performance or memory issue was found.
- Other review points: the Nereids
toSql()spelling discrepancy was dismissed because plugin external CREATE/ALTER use structured objects and never consume or reparse that diagnostic text. No other correctness, compatibility, lifecycle, or coverage issue survived convergence.
User focus: no additional review focus was supplied.
Review status: complete for the supplied head; all candidates were accepted, deduplicated, or dismissed before submission.
Contributor
TPC-H: Total hot run time: 17122 ms |
Contributor
TPC-DS: Total hot run time: 81641 ms |
Contributor
ClickBench: Total hot run time: 14.59 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#67166 Problem Summary: The mixed-case Iceberg schema evolution fixture drops a lowercase nested field and re-adds it as uppercase DROP_AND_ADD. After preserving external nested field spelling, DESC correctly reports the uppercase name, but the regression baseline still expected the old normalized spelling. Update the Parquet and ORC schema expectations and document that runtime lookup continues to use the normalized field name. ### Release note None ### Check List (For Author) - Test: Regression test - Built the regression framework and ran its four unit tests with ./run-regression-test.sh --compile - Compiled test_iceberg_struct_schema_evolution.groovy with the Groovy compiler - Verified both mixed-case DESC baselines retain six columns and preserve DROP_AND_ADD - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Automated review result: changes requested.
Two blocking correctness issues remain on the current head; both already have complete live inline threads, so this review deliberately adds no duplicate inline comments:
- Use Iceberg's ROOT-lowercase name identity.
equalsIgnoreCasecan accept a positional match whose lowercase keys differ, then stage type/comment/nullability updates on the wrong canonical old field path. - Preserve FILE-TVF spelling before constructing the field. FILE-TVF decoding lowercases
PStructFieldnames before the new dual-name carrier sees them, so S3/FILE CTAS to Iceberg still persists lowercase nested names.
Critical checkpoint conclusions:
- Goal and proof: the change correctly separates original external spelling from normalized runtime lookup for direct DDL, ordinary catalog/Nereids/connector round trips, display, and normalized BE thrift. The unit tests, real Iceberg InMemoryCatalog commits, and Parquet/ORC regression expectations prove the covered ASCII paths, but the two blockers above mean the full stated goal is not yet achieved.
- Scope and parallel paths: the implementation is focused and reuses the existing conversion flow. Direct CREATE, ordinary connector CTAS, nested ARRAY/MAP recursion, complex MODIFY, constant-folding constraints, and nested-pruning reachability were traced. FILE-TVF is the missed functional parallel path.
- Concurrency and lifecycle: no new threads, locks, mutable shared state, static initialization, configuration, or dynamic-config behavior are introduced. Complex Iceberg changes remain staged in one
UpdateSchemaand committed atomically. - Compatibility and persistence: the catalog field is an additive Gson member with an old-image fallback to normalized
name; equality/runtime maps and FE-to-BE thrift remain normalized, so no new BE protocol or rolling-BE payload is introduced. No new FE/BE variable requires propagation. - Data correctness and errors: ordinary validation fails loud and canonical old paths prevent case-only renames, but the Unicode identity mismatch can commit changes to the wrong field, which is blocking.
- Tests and results: the changed expected results are consistent with preserving external spelling while retaining lowercase runtime access. Please add the Unicode identity InMemoryCatalog regression and an end-to-end FILE-TVF-to-Iceberg CTAS regression requested in the existing threads. No builds or tests were run by this automated review because the review environment explicitly prohibits them.
- Performance and observability: the extra immutable spelling field and lowercase collision keys have negligible cost; no new logging, metrics, or observability surface is required.
- User focus: no additional user-provided focus was supplied.
Review completion: the authoritative 11-file diff was fully covered. One normal FE/lifecycle review, one normal Iceberg/test review, and a separate risk-focused ingress/identity review all converged with NO_NEW_VALUABLE_FINDINGS; every candidate was independently validated and deduplicated. No additional inline finding is warranted beyond the two live blockers above.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review result: request changes
Two P1 correctness gaps remain in the new case-preservation behavior.
Checkpoint conclusions
- Goal and proof: The patch otherwise preserves nested spelling through direct CREATE, source CTAS, FILE-TVF CTAS, and complex MODIFY, including STRUCT under ARRAY/MAP. The two inline issues show that runtime identity is still JVM-locale-dependent and flat Iceberg MODIFY still fails before the corrected child matcher when the top-level stored spelling differs.
- Scope and parallel paths: All 14 changed paths were reviewed across parser/type conversion, catalog and Nereids representations, connector export, FILE protobuf decoding, flat/nested MODIFY, rendering, runtime lookup/pruning, and the changed tests/results. No additional user focus was supplied.
- Concurrency, lifecycle, and configuration: No new threads, locks, background/static lifecycle, configuration, or FE/BE variable behavior is introduced. Existing connector session/authentication scope and table cleanup remain unchanged.
- Compatibility and persistence:
originalNameis additive Gson metadata with an old-image fallback toname; thrift/protobuf execution shape is unchanged. Iceberg schema changes remain staged in oneUpdateSchemaand committed atomically. Locale-dependent runtime keys still make metadata and lookup behavior inconsistent across FE locales, as noted inline. - Special conditions and errors: ROOT-lowercase child identity, Unicode collision handling, canonical nested paths, and fail-before-commit validation otherwise hold. The flat top-level root and
AFTERreference still need canonical stored spellings. - Tests and result files: The changed unit/regression coverage exercises real Iceberg commits and real Parquet FILE-TVF-to-Iceberg CTAS, and the modified result rows match the new rendering. Missing boundaries are a Turkish-default-locale uppercase
I/dotlessıcase and a case-mismatched mixed-case top-level MODIFY root. Per the review runner instructions, no local build or test command was run. - Observability and performance: These are synchronous DDL/type-conversion paths with existing error context; no new observability requirement was found. The added per-field string state and DDL-time normalization cost are negligible.
All three final convergence reviewers returned no new valuable findings after second rechecks. Live head/base still match the authoritative bundle. CheckStyle is passing; FE UT, COMPILE, and performance are currently pending.
Contributor
TPC-H: Total hot run time: 16895 ms |
Contributor
TPC-DS: Total hot run time: 81927 ms |
Contributor
ClickBench: Total hot run time: 14.76 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#67166 Problem Summary: Struct runtime lookup keys still depended on the JVM default locale, which could collapse distinct field names under locales such as Turkish. Flat Iceberg MODIFY COLUMN also resolved and staged updates with the caller spelling even though Iceberg update paths are case-sensitive. Use Locale.ROOT throughout the struct identity pipeline, resolve top-level Iceberg fields case-insensitively, and stage all updates and positions with persisted names. ### Release note Preserve mixed-case Iceberg field names reliably across locales and allow case-insensitive Doris MODIFY COLUMN references without losing the stored spelling. ### Check List (For Author) - Test: Unit Test - 38 targeted FE core tests - 44 Iceberg catalog-backed column evolution tests - Full FE Checkstyle - Behavior changed: Yes. Iceberg schema changes keep persisted field spelling while Doris lookup remains case-insensitive and locale-independent. - Does this need documentation: No.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review summary
Requesting changes for one P1 correctness gap: Unicode runtime-name canonicalization still misses dotted dereferences whose base is a computed expression. The patch otherwise consistently preserves external struct spelling through Catalog/Nereids conversions, connector requests, FILE TVF ingestion, and Iceberg schema evolution while keeping normalized names on thrift/BE runtime paths.
Key checkpoints
- Goal and parallel paths: explicit
element_atand bare dottedUnboundSlotaccess are canonicalized, but a non-slot base is parsed and analyzed through the separateDereferenceExpressionpath; this prevents the runtime-name contract from being complete. - Scope/minimality: the production changes are focused on the dual-name type contract, conversion boundaries, Iceberg mutation paths, analyzer/pruning alignment, and their tests.
- Compatibility/persistence:
originalNameis additive persisted metadata and the thrift shape remains unchanged. I did not repeat the already-threaded pre-ROOT legacy lookup concern. - Iceberg writes/errors: flat and nested mutations resolve persisted Iceberg identities and remain staged on one
UpdateSchemawith one final commit; no separate wrong-target, partial-commit, or swallowed-error defect survived review. - Concurrency/lifecycle/configuration: no new production thread, lock, resource lifetime, static-initialization, or configuration behavior is introduced.
- FE-to-BE/performance/observability: normalized names continue over the existing wire representation; the inline finding is a selector that fails to adopt that identity before BE lookup. Added schema-width work is confined to analysis/DDL paths, and no new observability surface is needed.
- Tests: the added tests cover conversion, replay, explicit and bare selectors, pruning, Iceberg evolution, and the FILE-TVF-to-Iceberg boundary, but not the computed-base dotted case. Per the review instructions I did not run builds or tests locally; live CheckStyle is green, while skipped or absent heavier jobs are not treated as proof.
No additional user review focus was provided, so I reviewed the full authoritative diff. Existing inline discussions were treated as duplicate fences.
| lastFieldName = fieldName; | ||
| expression = new ElementAt(expression, new StringLiteral(fieldName)); | ||
| // Dereference-created selectors also cross the thrift boundary and must use runtime identity. | ||
| expression = new ElementAt(expression, new StringLiteral(field.getName())); |
Contributor
There was a problem hiding this comment.
[P1] Canonicalize computed dotted dereferences too
This line fixes only the UnboundSlot path handled by bindNestedFields. A computed base such as (CAST(payload AS STRUCT<`Σ`:BIGINT>)).`Σ` is parsed as DereferenceExpression; visitDereferenceExpression resolves the field but returns new ElementAt(expression, dereferenceExpression.child(1)), and that newly created node is not revisited by visitElementAt. The selector therefore remains Σ while the cast/thrift struct field is σ, so BE's bytewise iequal lookup still reports the field missing. This is a separate analyzer branch from the existing direct-selector thread. Please canonicalize the resolved field in visitDereferenceExpression too (or share one helper) and add a computed-base dotted-access test.
Contributor
Author
There was a problem hiding this comment.
Fixed in 458e465. The struct branch in visitDereferenceExpression now canonicalizes the newly constructed ElementAt before returning, because that node is not revisited by visitElementAt. A regression test covers dotted access on a computed CAST base with a Unicode selector.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review result: request changes
Two P1 correctness gaps remain on exact head 6f1193d7f63f92948be69f0c62d6a643ad8be848. Both already have live inline anchors, so this review deliberately adds no duplicate root comments.
-
Canonicalize computed dotted dereferences. The parser-shaped path below resolves the struct field but returns a fresh
ElementAtwith selectorΣ; it is not revisited by the branch that canonicalizes to runtime keyσ, so FE analysis and the normalized thrift/BE lookup disagree.Project[(CAST(payload AS STRUCT<Σ:BIGINT>)).Σ] └─ child producing payload -
Preserve lookup for pre-ROOT serialized structs. I continued that thread with a two-field pre-change
tr-TRcounterexample: old keysıandican coexist, and selectorInow exact-hits the wrongisibling before the compatibility fallback runs.
Key checkpoint conclusions
- Goal and proof: External spelling and ROOT-normalized runtime identity are preserved across the covered Catalog/Nereids/connector/FILE-TVF/Iceberg paths, but the two blockers mean selector correctness and old-image compatibility are incomplete.
- Scope and parallel paths: The implementation is focused. Direct and function
element_at, simple dotted slots, computed-base dereference, CTAS, thrift translation, pruning, flat/nested Iceberg evolution, STRUCT under ARRAY/MAP, and external spelling round trips were traced. The missed computed-base branch is the first blocker. - Concurrency, lifecycle, and configuration: No new shared mutable state, lock, callback/resource lifetime, static initialization, configuration, or dynamic-reload behavior is introduced.
- Compatibility and persistence:
originalNameis additive metadata and current runtime/thrift naming is consistent. The exact-first legacy lookup can nevertheless rebind a persisted pre-ROOT two-field schema, which is a blocking compatibility/data-correctness issue. No new FE/BE variable or wire-shape change was found. - Conditions, errors, transactions, and writes: Iceberg validation and canonical target paths stage mutations in one
UpdateSchemaand one atomic commit; no separate partial-write or wrong-Iceberg-target issue survived review. The remaining failures occur in analyzer/runtime identity and legacy binding. - Tests and results: Added tests cover current-schema conversion, direct/function/simple-slot selectors, single-field legacy replay, pruning, real Iceberg evolution, and FILE-TVF CTAS. They omit a parser-shaped computed-base dotted expression and the two-field legacy collision. No local build or test was run because the review runner prohibits it. Live CheckStyle and lightweight checks pass; FE UT, COMPILE, and performance are pending at submission time.
- Observability and performance: Existing analysis/DDL errors are sufficient once identity is corrected. Added normalization and schema scans are outside scan hot paths; no distinct logging, metrics, memory, or performance issue was found.
- Other review points: Existing review threads were hard duplicate fences. No additional correctness, security, documentation, or user-focus concern survived the full changed-file sweep; no additional user focus was supplied.
Review status: complete for the exact supplied head. One normal FE review, one normal Iceberg/integration review, and a separate risk-focused review all returned NO_NEW_VALUABLE_FINDINGS after applying the existing-thread fences; every candidate was verified, deduplicated, or dismissed before submission.
Contributor
TPC-H: Total hot run time: 16804 ms |
Contributor
TPC-DS: Total hot run time: 80910 ms |
Contributor
ClickBench: Total hot run time: 14.68 s |
Contributor
Compatibility Review: Struct Field Name Lookup RisksThanks for the detailed work on preserving nested field case. I ran targeted compatibility tests simulating locale-dependent metadata persistence and replay. Below are findings from code tracing and Java-level verification. Issue 1 —
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
|
@924060929 Thanks for the compatibility review. I checked all four points against the current PR head:
I also fixed the separate computed-base dereference gap in 458e465 and added regression coverage for both valid issues. |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
There was a problem hiding this comment.
Review result: request changes
Two new P1 correctness gaps remain on exact head 458e4651d95aad2ae1da42b665082ce4791e0462 and are described inline:
- Flat Iceberg
ADD ... AFTERand full reorder still pass caller spellings to case-sensitive Iceberg move APIs, even though the new MODIFY path canonicalizes them. - Bracket syntax retains a
Variablewrapper through STRUCT-selector canonicalization, so a displayed Unicode selector can still reach BE with the external rather than runtime spelling.
The already-threaded pre-ROOT replay concern also remains incomplete for locale mappings that change code-point count: under lt-LT, capital I plus combining grave lowercases to i plus combining dot plus combining grave, which the new equalsIgnoreCase fallback cannot match to the ROOT-lowercased selector. I did not add a duplicate inline comment.
Reduced plan for the new analyzer gap:
Project[payload[@field]]
└─ Scan[payload STRUCT<σ:INT>] (@field = "Σ")
Key checkpoint conclusions
- Goal and proof: The patch preserves external STRUCT spelling while retaining stable runtime identity on the covered Catalog, Nereids, FILE-TVF, connector, pruning, and Iceberg MODIFY paths. The goal is incomplete at the two new inline paths and the existing legacy-replay thread. Current tests do not prove those boundaries.
- Scope and clarity: The dual-name and canonical-path changes are focused. All 28 changed paths and the relevant parser, analyzer, conversion, replay, pruning, translation, BE lookup, connector, Iceberg mutation, and regression consumers were traced.
- Concurrency: No new threads, locks, or shared mutable state are introduced. Iceberg schema mutation retains its existing optimistic commit-conflict boundary.
- Lifecycle: No new callback, cache-handle, static-initialization, teardown, or non-intuitive ownership lifecycle is added.
- Configuration: No configuration item or dynamic-reload behavior is added.
- Compatibility:
originalNameis additive Gson metadata and the FE-to-BE thrift shape remains unchanged. Current-generation ROOT identities and supported conversion paths are consistent, but the existing pre-ROOT replay fallback still misses the length-expanding locale case described above. - Parallel paths: Direct/function literals, integer selectors, prepared placeholders, ordinary/computed dereferences, flat/nested MODIFY, STRUCT under ARRAY/MAP, and canonical error-upgrade paths are covered. The missed parallels are bracket-variable selectors and the flat ADD/reorder positioning paths reported inline.
- Conditions and errors: Invalid Iceberg mutations fail before the single commit, and no swallowed status or misleading new error path survived review. The accepted DDL paths deterministically reject valid case-insensitive references; the accepted selector path passes FE analysis and fails at BE lookup.
- Test coverage: Unit, InMemoryCatalog, protobuf, and external regression tests provide real conversion, metadata, pruning, and execution oracles. Missing regressions are mixed-case ADD ... AFTER/reorder, variable-backed bracket execution plus SQL-cache accounting, and the duplicate-fenced Lithuanian replay case.
- Test results: The changed expected results are consistent with the intended spelling/runtime split. No local build or test was run because the authoritative review bundle prohibits it. At submission time CheckStyle and the applicable lightweight checks pass; COMPILE, FE UT, performance, and code-review are pending.
- Observability: Existing analyzer and Iceberg exceptions provide adequate context once identity is corrected; no new log, metric, or tracing surface is needed.
- Persistence and transactions: Gson replay and Iceberg schema metadata are the applicable persistence paths; no Doris EditLog/transaction path is changed. Each Iceberg evolution entry point stages one
UpdateSchemaand commits once. - Data writes: Only Iceberg metadata is mutated, atomically at commit. No partial schema publication, FE/BE crash leak, or wrong-field write survived beyond the rejected-name paths described above.
- FE/BE variables and protocol: No new cross-process variable or wire field is added. The bracket issue concerns analyzer phase ordering for an existing SQL variable and the unchanged runtime name sent to BE; a repair must preserve SQL-cache variable accounting.
- Memory and BE nullable handling: These Java metadata/analyzer changes add no BE buffer ownership, allocator, nullable-column, or const-nullable path.
- Performance: New work is limited to planner/DDL-time field or path scans. No scan-hot-path, material allocation, or complexity regression was found.
- Other: No other distinct correctness issue survived validation and live-thread duplicate fencing. No additional user-provided review focus was supplied.
Review completion: both Round 2 normal full-review agents and both dedicated risk agents returned NO_NEW_VALUABLE_FINDINGS; every candidate was accepted, duplicate-fenced, or dismissed with evidence. The live head/base and current GitHub patches were revalidated immediately before submission.
| updateSchema.makeColumnOptional(currentName); | ||
| } | ||
| applyPosition(updateSchema, position, column.getName()); | ||
| IcebergNestedColumnEvolution.applyTopLevelPosition( |
Contributor
There was a problem hiding this comment.
[P1] Canonicalize the other top-level positioning paths too. This fixes MODIFY by resolving both the target and AFTER sibling to persisted Iceberg spellings, but ADD still routes through applyPosition, which passes position.getAfterColumn() verbatim, and reorderColumns still passes every caller-provided name directly to the case-sensitive moveFirst/moveAfter APIs. On a table with stored fields Id/Label, ADD ... AFTER id and a reorder using label/id therefore still fail even though Doris resolves external columns case-insensitively. Please reuse schema-backed canonicalization for ADD and resolve the complete reorder list before staging it, with mixed-case catalog-backed tests.
| private ElementAt canonicalizeStructSelector(ElementAt elementAt) { | ||
| Expression left = elementAt.left(); | ||
| Expression right = elementAt.right(); | ||
| if (left.getDataType() instanceof StructType && right instanceof StringLikeLiteral) { |
Contributor
There was a problem hiding this comment.
[P1] Unwrap bracket-expression variables before canonicalizing. The parser builds payload[@field] directly as ElementAt, and ElementAt legality/type resolution uses getArgument(1), so a user variable whose value is displayed Unicode Σ is accepted. This helper checks the raw right child, sees Variable, and skips the rewrite; VariableToLiteral later exposes Σ, but only integer selectors are normalized afterward. BE then receives Σ against the thrift runtime name σ and fails lookup. Please canonicalize the wrapper-aware argument while preserving variable-use/SQL-cache accounting, and add a variable-backed bracket execution test. Function syntax is already unwrapped by constructUnboundFunctionArguments.
Contributor
FE UT Coverage ReportIncrement line coverage |
924060929
approved these changes
Sep 3, 2026
Contributor
TPC-H: Total hot run time: 17027 ms |
Contributor
TPC-DS: Total hot run time: 82056 ms |
Contributor
ClickBench: Total hot run time: 14.71 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
approved
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
3 participants
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
Problem Summary:
Doris used the lowercase runtime name of nested STRUCT fields when building connector schemas. Creating an Iceberg table through Doris therefore changed persisted mixed-case field names such as
CaseSensitivetocasesensitive, causing schema compatibility checks from engines such as Trino and Spark to fail.The same identity mismatch also affected schema evolution, query execution, and pruning:
MODIFY COLUMNcould interpret a case-only spelling difference as a nested rename;MODIFY COLUMNand itsAFTERreference used caller spelling even though Iceberg update paths are case-sensitive;MODIFY COLUMNerror upgrading resolved only the leaf name and could inspect an unrelated same-named top-level column;DESCRIBE, such asΣorẞ, could fail at execution because FE passed that external spelling to BE while the thrift struct descriptor contained the normalized runtime name (σorß);equalsIgnoreCase, which could map distinct ROOT-normalized siblings such asiandıto the wrong source field.This change keeps two explicit field identities: a locale-independent lowercase key for Doris runtime lookup and the original spelling for connector/external metadata. Connector create-table paths, including FILE TVF CTAS, use the original spelling. Iceberg schema evolution resolves existing fields case-insensitively and then stages type, comment, nullability, and position changes with the persisted canonical path. Only an explicit rename operation changes field spelling. Nested modify error upgrading resolves the complete
ConnectorColumnPathand remains best-effort so it cannot replace the original build error for a missing target.After Nereids successfully resolves a STRUCT selector, it replaces the external spelling with the resolved field's normalized runtime name for thrift/BE execution. This canonicalization is applied to directly analyzed
ElementAtnodes,ElementAtproduced while binding SQL function syntax fromUnboundFunction, andElementAtcreated for dotted access on a computed base such asCAST(... AS STRUCT<...>).field. The latter two paths construct and return a new node without revisitingvisitElementAt. Consequently,element_at(struct, 'field')and dotted dereference both work whileDESCRIBEand connector metadata continue to expose the original external spelling. Cast-aware pruning compares exact ROOT-normalized keys so that distinct siblings remain distinct.Metadata and rolling-upgrade compatibility
StructField.name,StructType.fields, and the currentfieldMaplookup index can be present in FE image metadata. Before this change, an FE running with a locale such astr-TRcould therefore persistIas the runtime keyı. A new FE usingLocale.ROOTproduces the lookup keyi, so exact ROOT lookup alone cannot read that pre-ROOT image.For fields replayed from metadata that predates
originalName, this PR records a runtime-only legacy marker through Catalog-to-Nereids conversion. Current fields use the exact ROOT lookup key. Legacy fields first accept an exact persisted runtime spelling; broader case matching is used only when it identifies a single legacy field. If multiple legacy runtime names match, lookup rejects the ambiguous selector instead of silently returning the wrong sibling. Newly created metadata always hasoriginalName, so valid ROOT-distinct names such asiandıare not merged by the compatibility path.This covers the supported rolling-upgrade direction where upgraded Followers/Observers replay metadata written by an older Master, followed by upgrading the Master. It does not make an old FE understand metadata first written by a new FE. It also cannot reconstruct original spelling that an old FE already discarded; it only preserves unambiguous lookup compatibility for the persisted runtime name. Rebuilding
fieldMapduring deserialization would not recover the old FE locale or discarded spelling because Doris replays these objects through Gson and that information was never persisted.Fix boundary
The fix is limited to nested field identity preservation, Iceberg schema evolution, STRUCT selector canonicalization in FE, unambiguous legacy pre-ROOT struct lookup, and consistent cast-pruning identity. Doris runtime lookup remains case-insensitive for current metadata. BE continues to receive and compare normalized thrift names; this PR does not add Unicode case folding to BE. Current metadata continues to persist the existing
fieldMap; rebuilding or removing that lookup index is a separate metadata-format change and is intentionally outside this PR. Removed code paths such asStructElementandIcebergScanNodeare not reintroduced; their current replacements already route through the fixed lookup or preserve partition-column case.Release note
Preserve mixed-case nested field names when creating and evolving Iceberg schemas, allow displayed Unicode nested field names to be queried, keep nested pruning correct for locale-sensitive Unicode names, and retain safe lookup compatibility with pre-ROOT FE metadata.
Check List (For Author)
Test
Added coverage for mixed-case nested fields in create-table and FILE TVF CTAS paths, STRUCT/ARRAY/MAP evolution, locale-independent runtime lookup and pruning, case-insensitive flat Iceberg
MODIFY COLUMN/AFTERresolution, full-path nested modify error handling, pre-ROOT Turkish metadata replay across Catalog and Nereids, ambiguous legacy-field rejection, cast pruning with ROOT-distinct sibling names, and execution of exact displayed Unicode names (Σandẞ). TheExpressionAnalyzercoverage constructs anUnboundFunctionto exercise the same SQL function-binding path aselement_at(...), and covers directElementAt, ordinary dotted dereference, and computed-base dotted dereference.Latest local validation passed 23 Iceberg connector column-evolution tests, all 8
ExpressionAnalyzertests, and all 5ColumnGsonSerializationTesttests. Full FE Checkstyle passed all 74 modules. The external Spark/Iceberg regression requires the CI test environment and was added for CI execution.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)