This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new aeb7e1a0107 branch-4.1: [fix](iceberg) Reject invalid name mapping
instead of reading NULL (#68004) (#68003)
aeb7e1a0107 is described below
commit aeb7e1a0107647db0fbb0139492f82f296d862ba
Author: daidai <[email protected]>
AuthorDate: Sun Sep 20 10:17:00 2026 +0800
branch-4.1: [fix](iceberg) Reject invalid name mapping instead of reading
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
---
.../iceberg/IcebergExternalMetaCache.java | 4 +++
.../doris/datasource/iceberg/IcebergUtils.java | 38 +++++++++++++++-------
.../datasource/iceberg/source/IcebergScanNode.java | 8 +++--
.../doris/datasource/iceberg/IcebergUtilsTest.java | 28 +++++++++++-----
.../iceberg/source/IcebergScanNodeTest.java | 25 +++++++++++++-
5 files changed, 80 insertions(+), 23 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java
index 1cd7c5684f6..d04c3b405ed 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java
@@ -18,6 +18,7 @@
package org.apache.doris.datasource.iceberg;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.UserException;
import org.apache.doris.common.security.authentication.ExecutionAuthenticator;
import org.apache.doris.datasource.CacheException;
import org.apache.doris.datasource.CatalogIf;
@@ -568,6 +569,9 @@ public class IcebergExternalMetaCache extends
AbstractExternalMetaCache {
retainedTable);
} catch (AnalysisException e) {
throw new RuntimeException(ExceptionUtils.getRootCauseMessage(e),
e);
+ } catch (UserException e) {
+ // AnalysisException subclasses UserException, so the more
specific type comes first.
+ throw new RuntimeException(e.getMessage(), e);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
index 76b731b410b..3c8d600eb86 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java
@@ -117,7 +117,6 @@ import org.apache.iceberg.hive.HiveCatalog;
import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.mapping.MappedField;
import org.apache.iceberg.mapping.MappedFields;
-import org.apache.iceberg.mapping.MappingUtil;
import org.apache.iceberg.mapping.NameMapping;
import org.apache.iceberg.mapping.NameMappingParser;
import org.apache.iceberg.transforms.Transforms;
@@ -2266,10 +2265,16 @@ public class IcebergUtils {
*/
static IcebergSnapshotCacheValue newExplicitSnapshotValue(
IcebergTableQueryInfo info, Table queryScopedTable,
IcebergTableCacheValue generation) {
+ Optional<Map<Integer, List<String>>> nameMapping;
+ try {
+ nameMapping = getNameMapping(queryScopedTable);
+ } catch (UserException e) {
+ throw new RuntimeException(e.getMessage(), e);
+ }
return new IcebergSnapshotCacheValue(
IcebergPartitionInfo.empty(),
new IcebergSnapshot(info.getSnapshotId(), info.getSchemaId()),
- getNameMapping(queryScopedTable), queryScopedTable)
+ nameMapping, queryScopedTable)
.bindCapturedAuthenticator(generation.getAuthenticator());
}
@@ -2347,9 +2352,19 @@ public class IcebergUtils {
/**
* Extract the Iceberg name mapping while retaining the distinction
between an absent property
- * and a valid empty mapping.
+ * and a valid (possibly empty) mapping.
+ *
+ * <p>A property that is present but cannot be parsed is a metadata fault
rather than an absent
+ * mapping. Iceberg readers refuse such tables outright (Spark's {@code
BaseReader} parses
+ * {@code schema.name-mapping.default} while constructing the file
reader), so Doris reports the
+ * fault instead of silently degrading to the current column names.
Degrading hides renamed
+ * columns behind NULLs when reading data files without field ids, and can
even return wrong
+ * values once a column name has been reused.
+ *
+ * @throws UserException if the property is present but cannot be parsed
as a name mapping
*/
- public static Optional<Map<Integer, List<String>>> getNameMapping(Table
icebergTable) {
+ public static Optional<Map<Integer, List<String>>> getNameMapping(Table
icebergTable)
+ throws UserException {
String nameMappingJson =
icebergTable.properties().get(TableProperties.DEFAULT_NAME_MAPPING);
if (nameMappingJson == null || nameMappingJson.isEmpty()) {
return Optional.empty();
@@ -2363,13 +2378,14 @@ public class IcebergUtils {
extractMappingsFromNameMapping(mapping.asMappedFields(), result);
return Optional.of(result);
} catch (Exception e) {
- // Keep ID-less files readable by current names when a malformed
property cannot provide
- // authoritative aliases; Optional.empty() must remain reserved
for an absent property.
- LOG.warn("Failed to parse name mapping from Iceberg table
properties", e);
- Map<Integer, List<String>> fallback = new HashMap<>();
- extractMappingsFromNameMapping(
-
MappingUtil.create(icebergTable.schema()).asMappedFields(), fallback);
- return Optional.of(fallback);
+ LOG.warn("Failed to parse name mapping of table {}",
icebergTable.name(), e);
+ throw new UserException(String.format(
+ "Invalid table property '%s' of Iceberg table %s: %s. "
+ + "The value must be an Iceberg name mapping JSON
array; please fix or drop "
+ + "the property (for example with ALTER TABLE ...
UNSET TBLPROPERTIES in "
+ + "Spark) and refresh the table.",
+ TableProperties.DEFAULT_NAME_MAPPING, icebergTable.name(),
+ ExceptionUtils.getRootCauseMessage(e)), e);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
index 3188e7a73e6..10b119eda81 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java
@@ -339,7 +339,7 @@ public class IcebergScanNode extends FileQueryScanNode {
checkVariantBackendCompatibility(projectsVariant, backends);
}
- private Optional<Map<Integer, List<String>>> extractNameMapping() {
+ private Optional<Map<Integer, List<String>>> extractNameMapping() throws
UserException {
Optional<MvccSnapshot> snapshot = getPinnedRelationSnapshot();
if (snapshot.isPresent() && snapshot.get() instanceof
IcebergMvccSnapshot) {
// The mapping must come from the same metadata generation as the
pinned schema; a
@@ -625,7 +625,11 @@ public class IcebergScanNode extends FileQueryScanNode {
public void createScanRangeLocations() throws UserException {
Schema scanSchema = getQuerySchema();
- Optional<Map<Integer, List<String>>> nameMapping =
extractNameMapping();
+ // Metadata (system) table scans never resolve physical data columns
by name, so a malformed
+ // name-mapping property must not fail them. Data scans are validated
here instead: a
+ // malformed schema.name-mapping.default is a metadata fault that
Iceberg would reject too.
+ Optional<Map<Integer, List<String>>> nameMapping =
+ isSystemTable ? Optional.empty() : extractNameMapping();
Set<Integer> equalityDeleteFieldIds = Collections.emptySet();
if (!isSystemTable) {
ConnectContext context =
Preconditions.checkNotNull(ConnectContext.get(),
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java
index 616f1eefa5f..828da91447e 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java
@@ -140,7 +140,7 @@ public class IcebergUtilsTest {
}
@Test
- public void testRetainedGenerationKeepsProjectionAtomic() {
+ public void testRetainedGenerationKeepsProjectionAtomic() throws Exception
{
Schema originalSchema = new Schema(
Types.NestedField.required(1, "id", Types.IntegerType.get()));
Schema evolvedSchema = new Schema(
@@ -174,20 +174,30 @@ public class IcebergUtilsTest {
}
@Test
- public void testMalformedNameMappingFallsBackToCurrentSchemaNames() {
- Schema schema = new Schema(
- Types.NestedField.required(1, "id", Types.IntegerType.get()),
- Types.NestedField.optional(2, "name", Types.StringType.get()));
+ public void
testMalformedNameMappingFailsInsteadOfFallingBackToCurrentSchemaNames() {
Table table = Mockito.mock(Table.class);
+ Mockito.when(table.name()).thenReturn("db.tbl");
Mockito.when(table.properties()).thenReturn(Collections.singletonMap(
TableProperties.DEFAULT_NAME_MAPPING, "{not valid json"));
- Mockito.when(table.schema()).thenReturn(schema);
+
+ // Iceberg (and therefore Spark) refuses to read a table whose name
mapping cannot be
+ // parsed; silently rewriting the property into current-schema aliases
would turn renamed
+ // columns of ID-less files into NULLs instead of reporting the
metadata fault.
+ UserException exception = Assert.assertThrows(UserException.class,
+ () -> IcebergUtils.getNameMapping(table));
+
Assert.assertTrue(exception.getMessage().contains(TableProperties.DEFAULT_NAME_MAPPING));
+ Assert.assertTrue(exception.getMessage().contains("db.tbl"));
+ }
+
+ @Test
+ public void testEmptyNameMappingStillParsesAsAuthoritativeMapping() throws
Exception {
+ Table table = Mockito.mock(Table.class);
+ Mockito.when(table.properties()).thenReturn(
+ Collections.singletonMap(TableProperties.DEFAULT_NAME_MAPPING,
"[]"));
Optional<Map<Integer, List<String>>> mapping =
IcebergUtils.getNameMapping(table);
Assert.assertTrue(mapping.isPresent());
- Map<Integer, List<String>> fallback = mapping.get();
- Assert.assertEquals(Collections.singletonList("id"), fallback.get(1));
- Assert.assertEquals(Collections.singletonList("name"),
fallback.get(2));
+ Assert.assertTrue(mapping.get().isEmpty());
}
@Test
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java
index 50279f24fab..036c63569ba 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java
@@ -495,7 +495,30 @@ public class IcebergScanNodeTest {
}
@Test
- public void testSnapshotCacheIgnoresIdlessNameMappingWrapper() {
+ public void testExtractNameMappingRejectsMalformedProperty() throws
Exception {
+ TestIcebergScanNode node = new TestIcebergScanNode(new
SessionVariable());
+ Table table = Mockito.mock(Table.class);
+ Mockito.when(table.name()).thenReturn("db.tbl");
+ setIcebergTable(node, table);
+ IcebergSource source = Mockito.mock(IcebergSource.class);
+
Mockito.when(source.getTargetTable()).thenReturn(Mockito.mock(IcebergExternalTable.class));
+ setIcebergSource(node, source);
+
+ Mockito.when(table.properties()).thenReturn(Collections.singletonMap(
+ TableProperties.DEFAULT_NAME_MAPPING, "{not valid json"));
+
+ // A malformed name mapping is a metadata fault that Iceberg refuses
to read, so the scan
+ // must surface it instead of degrading to current-schema aliases
(which silently returns
+ // NULL for the columns of ID-less files that were renamed).
+ InvocationTargetException thrown =
Assert.assertThrows(InvocationTargetException.class,
+ () -> extractNameMapping(node));
+ Assert.assertTrue(thrown.getCause() instanceof UserException);
+ Assert.assertTrue(thrown.getCause().getMessage()
+ .contains(TableProperties.DEFAULT_NAME_MAPPING));
+ }
+
+ @Test
+ public void testSnapshotCacheIgnoresIdlessNameMappingWrapper() throws
Exception {
Table table = Mockito.mock(Table.class);
Mockito.when(table.properties()).thenReturn(Collections.singletonMap(
TableProperties.DEFAULT_NAME_MAPPING,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
