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 1cd7c5684f6eb9..d04c3b405ed909 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 @@ private IcebergSnapshotCacheValue loadSnapshotProjection( 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 da4513ff8a1d7e..0cf3fbd0e475b3 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.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; @@ -2256,10 +2255,16 @@ public static IcebergSnapshotCacheValue getSnapshotCacheValue( */ static IcebergSnapshotCacheValue newExplicitSnapshotValue( IcebergTableQueryInfo info, Table queryScopedTable, IcebergTableCacheValue generation) { + Optional>> 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()); } @@ -2337,9 +2342,19 @@ private static IcebergSchemaCacheValue buildTableSchemaCacheValue(ExternalTable /** * Extract the Iceberg name mapping while retaining the distinction between an absent property - * and a valid empty mapping. + * and a valid (possibly empty) mapping. + * + *

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>> getNameMapping(Table icebergTable) { + public static Optional>> getNameMapping(Table icebergTable) + throws UserException { String nameMappingJson = icebergTable.properties().get(TableProperties.DEFAULT_NAME_MAPPING); if (nameMappingJson == null || nameMappingJson.isEmpty()) { return Optional.empty(); @@ -2353,13 +2368,14 @@ public static Optional>> getNameMapping(Table icebergT 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> 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 2a45a1b749f794..e40dcc68afc7a8 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 @@ void checkVariantBackendCompatibilityForCurrentScan(Iterable backends) checkVariantBackendCompatibility(projectsVariant, backends); } - private Optional>> extractNameMapping() { + private Optional>> extractNameMapping() throws UserException { Optional 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 @@ private String getDeleteFileContentType(int content) { public void createScanRangeLocations() throws UserException { Schema scanSchema = getQuerySchema(); - Optional>> 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>> nameMapping = + isSystemTable ? Optional.empty() : extractNameMapping(); Set 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 616f1eefa5f3e2..828da91447ed2a 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 void testSnapshotCacheFreezesSharedTableOperations() { } @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 void testRetainedGenerationKeepsProjectionAtomic() { } @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>> mapping = IcebergUtils.getNameMapping(table); Assert.assertTrue(mapping.isPresent()); - Map> 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 8bb684ce1b8f8a..6ebc3a82ff2d57 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 void testExtractNameMappingDistinguishesAbsentAndEmpty() throws Exception } @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,