Skip to content

[fix](iceberg) Reject invalid name mapping instead of reading NULL - #68004

Merged
Gabriel39 merged 1 commit into
apache:masterfrom
hubgeter:fix/iceberg-invalid-name-mapping-master
Sep 17, 2026
Merged

Gabriel39 merged 1 commit into
apache:masterfrom
hubgeter:fix/iceberg-invalid-name-mapping-master

Conversation

@hubgeter

@hubgeter hubgeter commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

When the Iceberg table property schema.name-mapping.default is present but malformed, IcebergSchemaUtils.extractNameMapping caught the parse failure and rebuilt a "current schema" name mapping, which is authoritative. Data files that do not carry Iceberg field ids could then only be resolved by their current column names. After a column rename the old physical column is unmatched, V2 semantics materializes NULL for it, and the query succeeds with historical values lost. If the old name was later reused by another column, the fallback could even bind the wrong physical column.

Iceberg itself does not degrade like this: Spark's BaseReader parses schema.name-mapping.default while constructing the reader and fails the query for a malformed value (IllegalArgumentException / UncheckedIOException), regardless of whether the data files carry field ids.

This PR removes the current-schema fallback and reports the metadata fault instead. The connector now throws a DorisConnectorException that names the table, the property, the root cause and the remediation, so the failure is visible instead of silently returning NULL. Metadata (system) table scans are unaffected, because the schema-evolution carrier is only built for base-table scans.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • 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
  • Behavior changed:

    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…pache#68004)

### What problem does this PR solve?

Problem Summary:
A table property `schema.name-mapping.default` that is present but malformed was silently
degraded by the Iceberg connector: `IcebergSchemaUtils.extractNameMapping` caught the parse
failure and rebuilt a "current schema" mapping, which is authoritative. Data files without
Iceberg field ids could then only be resolved by their current column names, so after a
column rename the old physical column is unmatched and V2 semantics materializes NULL for
it - the query succeeds but historical values are lost. Once a name was reused, the fallback
could even bind the wrong physical column.

Iceberg readers do not behave that way: Spark's `BaseReader` parses
`schema.name-mapping.default` while constructing the reader and fails the query for a
malformed value (`IllegalArgumentException` / `UncheckedIOException`), regardless of whether
the data files carry field ids.

This PR removes the current-schema fallback and reports the metadata fault instead: the
connector now throws a `DorisConnectorException` naming the table, the property, the root
cause and the remediation. Metadata (system) table scans are unaffected because the
schema-evolution carrier is only built for base-table scans.

### Behavior changes

Before this change:

- A malformed `schema.name-mapping.default` was rewritten into current-schema aliases, so
  ID-less files were read by current column name (renamed columns returned NULL).
- No user visible error was raised; only a warning was logged.

After this change:

- A malformed `schema.name-mapping.default` fails the query with an explicit metadata error.

### Check List (For Author)

- Test
    - [x] Unit Test
- Behavior changed:
    - [x] Yes.
- Does this need documentation?
    - [x] No.
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hubgeter
hubgeter force-pushed the fix/iceberg-invalid-name-mapping-master branch from f2d22f3 to d10b548 Compare September 15, 2026 07:07
@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes: I found two P1 correctness issues in the name-mapping contract; details are inline.

Review checkpoints

  • Goal and tests: non-empty malformed mappings now fail through the common base-table preflight with a useful connector error, but the persistable empty-string boundary still bypasses that failure. The valid-empty utility test proves the transported dictionary, not the required mixed-version backend fence.
  • Compatibility and parallel paths: ordinary, equality-delete, time-travel, and Top-N base scans share the preflight. Iceberg metadata tables do not expose the base mapping, so system-table isolation remains intact. The unresolved compatibility issue is the new-FE/old-BE disagreement for projected authoritative no-fallback mappings.
  • Error handling and observability: once invoked, DorisConnectorException propagates across the plugin classloader boundary and includes the table, property, root cause, and remediation. The dependency/package path is established.
  • Scope and other risks: the change is localized. I found no new concurrency, lifecycle, configuration, persistence, transaction/data-write, storage-format, or material performance issue.
  • Release note: the PR marks Behavior changed: Yes, but ### Release note is None. Please replace it with a concise user-facing note explaining that malformed schema.name-mapping.default values now fail queries and how operators should remediate them.
  • Evidence: this was a static review only; no builds or tests were run under the review contract. At submission time, CheckStyle and the lightweight checks pass, while COMPILE, FE UT, and performance are still pending. Three bounded review rounds converged without a new finding in the final round. No additional user review focus was supplied.

@hubgeter
hubgeter marked this pull request as draft September 15, 2026 10:17
@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/112) 🎉
Increment coverage report
Complete coverage report

@hubgeter
hubgeter marked this pull request as ready for review September 16, 2026 09:56
@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16948 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit d10b54877c8494102760808361a9c58e54e18e3d, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17562	3038	3001	3001
q2	2129	244	219	219
q3	10211	958	512	512
q4	4664	250	224	224
q5	7659	579	388	388
q6	138	116	95	95
q7	529	487	386	386
q8	9235	871	835	835
q9	3501	2450	2444	2444
q10	6514	858	748	748
q11	401	201	181	181
q12	616	273	199	199
q13	18115	1552	1177	1177
q14	159	153	141	141
q15	q16	442	396	375	375
q17	1356	881	798	798
q18	3157	2355	2298	2298
q19	1136	905	762	762
q20	374	289	207	207
q21	5311	1733	1795	1733
q22	328	269	225	225
Total cold run time: 93537 ms
Total hot run time: 16948 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3397	3328	3313	3313
q2	508	407	379	379
q3	2293	2323	2273	2273
q4	1223	1192	900	900
q5	2247	2185	2161	2161
q6	166	120	94	94
q7	1117	935	904	904
q8	1614	1407	1414	1407
q9	3219	3246	3204	3204
q10	1916	1872	1668	1668
q11	360	281	256	256
q12	460	439	346	346
q13	1499	1559	1180	1180
q14	181	178	166	166
q15	q16	402	402	361	361
q17	3760	3458	3371	3371
q18	4967	4590	5060	4590
q19	955	846	874	846
q20	1040	1028	847	847
q21	3916	3248	3286	3248
q22	403	351	340	340
Total cold run time: 35643 ms
Total hot run time: 31854 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 82857 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit d10b54877c8494102760808361a9c58e54e18e3d, data reload: false

query5	4257	420	335	335
query6	382	136	131	131
query7	4932	433	226	226
query8	285	120	121	120
query9	8680	2932	2929	2929
query10	370	224	184	184
query11	5365	1089	915	915
query12	117	71	69	69
query13	1196	459	324	324
query14	6034	2256	2142	2142
query14_1	2020	2030	2027	2027
query15	179	125	109	109
query16	933	387	400	387
query17	775	436	357	357
query18	2330	308	227	227
query19	164	138	103	103
query20	70	74	71	71
query21	199	102	85	85
query22	5560	5471	5405	5405
query23	6875	6402	6208	6208
query23_1	6169	6134	6218	6134
query24	7279	1103	776	776
query24_1	765	775	800	775
query25	401	280	233	233
query26	1225	217	127	127
query27	2803	429	244	244
query28	4685	1527	1464	1464
query29	919	429	336	336
query30	245	163	129	129
query31	809	407	335	335
query32	128	77	85	77
query33	444	209	181	181
query34	988	819	490	490
query35	405	414	358	358
query36	573	594	525	525
query37	127	83	72	72
query38	1015	865	839	839
query39	506	485	487	485
query39_1	472	494	470	470
query40	201	96	81	81
query41	62	57	57	57
query42	76	70	72	70
query43	242	245	214	214
query44	1001	540	535	535
query45	112	106	105	105
query46	801	871	537	537
query47	764	781	721	721
query48	308	298	240	240
query49	550	244	203	203
query50	740	272	201	201
query51	8443	8074	8001	8001
query52	66	66	59	59
query53	185	203	192	192
query54	211	164	148	148
query55	72	59	57	57
query56	195	160	156	156
query57	712	673	643	643
query58	204	166	164	164
query59	1253	1270	1131	1131
query60	231	180	187	180
query61	113	139	113	113
query62	342	218	171	171
query63	169	143	138	138
query64	2823	671	578	578
query65	1710	1638	1714	1638
query66	1783	252	195	195
query67	9913	9956	10044	9956
query68	2774	1088	690	690
query69	345	249	183	183
query70	674	606	625	606
query71	245	176	160	160
query72	2247	1687	1495	1495
query73	635	587	322	322
query74	1570	1237	1137	1137
query75	1186	1120	973	973
query76	2279	701	505	505
query77	251	255	204	204
query78	4087	3790	3260	3260
query79	1173	787	555	555
query80	458	305	262	262
query81	419	164	132	132
query82	595	122	98	98
query83	298	211	192	192
query84	286	111	106	106
query85	705	324	274	274
query86	307	178	179	178
query87	1045	995	907	907
query88	2753	2097	2104	2097
query89	287	193	178	178
query90	1912	132	135	132
query91	129	116	94	94
query92	77	70	68	68
query93	1132	1057	694	694
query94	452	258	211	211
query95	529	250	230	230
query96	833	582	292	292
query97	1121	1105	1054	1054
query98	139	135	129	129
query99	421	343	308	308
Total cold run time: 174768 ms
Total hot run time: 82857 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.88 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit d10b54877c8494102760808361a9c58e54e18e3d, data reload: false

query1	0.01	0.01	0.00
query2	0.09	0.05	0.04
query3	0.26	0.11	0.10
query4	1.60	0.10	0.10
query5	0.17	0.15	0.16
query6	1.26	0.72	0.67
query7	0.04	0.01	0.01
query8	0.05	0.03	0.03
query9	0.29	0.22	0.22
query10	0.36	0.36	0.35
query11	0.17	0.12	0.12
query12	0.15	0.12	0.11
query13	0.31	0.32	0.31
query14	0.46	0.46	0.47
query15	0.37	0.35	0.35
query16	0.22	0.22	0.23
query17	0.66	0.71	0.75
query18	0.19	0.16	0.17
query19	1.18	1.17	1.28
query20	0.02	0.01	0.01
query21	15.49	0.16	0.11
query22	5.10	0.05	0.04
query23	16.12	0.25	0.10
query24	2.97	0.32	0.25
query25	0.12	0.04	0.04
query26	0.78	0.16	0.11
query27	0.03	0.03	0.03
query28	3.64	0.58	0.28
query29	12.42	3.19	2.57
query30	0.26	0.11	0.12
query31	2.76	0.39	0.17
query32	3.50	0.33	0.24
query33	1.49	1.58	1.45
query34	15.37	2.26	1.82
query35	1.79	1.76	1.76
query36	0.46	0.28	0.29
query37	0.06	0.04	0.04
query38	0.04	0.04	0.03
query39	0.03	0.02	0.02
query40	0.12	0.08	0.07
query41	0.09	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.58 s
Total hot run time: 14.88 s

@Gabriel39
Gabriel39 merged commit b73b9e4 into apache:master Sep 17, 2026
50 checks passed
yiguolei pushed a commit that referenced this pull request Sep 20, 2026
…ding NULL (#68004) (#68003)

### What problem does this PR solve?

Problem Summary:
bp #68004

When the Iceberg table property `schema.name-mapping.default` is present
but malformed, the FE caught the parse failure and rebuilt a "current
schema" name mapping, which is authoritative. Data files that do not
carry Iceberg field ids could then only be resolved by their current
column names. After a column rename the old physical column is
unmatched, V2 semantics materializes NULL for it, and the query succeeds
with historical values lost. If the old name was later reused by another
column, the fallback could even bind the wrong physical column.

Iceberg itself does not degrade like this: Spark's `BaseReader` parses
`schema.name-mapping.default` while constructing the reader and fails
the query for a malformed value (`IllegalArgumentException` /
`UncheckedIOException`), regardless of whether the data files carry
field ids.

This PR removes the current-schema fallback and reports the metadata
fault instead. `IcebergUtils.getNameMapping` now throws a
`UserException` that names the table, the property, the root cause and
the remediation, so the failure is visible instead of silently returning
NULL. Metadata (system) table scans are excluded, because they never
resolve physical data columns by name.

### Release note

None

### Check List (For Author)

- Test
    - [ ] Regression test
    - [x] Unit Test
    - [ ] 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

- Behavior changed:
    - [x] Yes.

- Does this need documentation?
    - [ ] No.
    - [ ] Yes.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants