This is an automated email from the ASF dual-hosted git repository.
Gabriel39 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new b73b9e41f0a [fix](iceberg) Reject invalid name mapping instead of
reading NULL (#68004)
b73b9e41f0a is described below
commit b73b9e41f0a655baa2c4d01adfae8f97cac28c41
Author: daidai <[email protected]>
AuthorDate: Thu Sep 17 09:36:37 2026 +0800
[fix](iceberg) Reject invalid name mapping instead of reading NULL (#68004)
### 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
- [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
---
.../connector/iceberg/IcebergSchemaUtils.java | 28 ++++++++++++-------
.../connector/iceberg/IcebergSchemaUtilsTest.java | 31 +++++++++++++++-------
2 files changed, 41 insertions(+), 18 deletions(-)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaUtils.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaUtils.java
index 364d2d7e443..e156b63181b 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaUtils.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaUtils.java
@@ -17,6 +17,7 @@
package org.apache.doris.connector.iceberg;
+import org.apache.doris.connector.spi.DorisConnectorException;
import org.apache.doris.thrift.TColumnType;
import org.apache.doris.thrift.TFileScanRangeParams;
import org.apache.doris.thrift.TPrimitiveType;
@@ -28,13 +29,13 @@ import org.apache.doris.thrift.schema.external.TNestedField;
import org.apache.doris.thrift.schema.external.TSchema;
import org.apache.doris.thrift.schema.external.TStructField;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.iceberg.Schema;
import org.apache.iceberg.SingleValueParser;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableProperties;
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;
@@ -214,8 +215,15 @@ public final class IcebergSchemaUtils {
* name-mapping property, and a present (possibly empty) map when it does
— the distinction #65784 relies on
* to make a table-level mapping AUTHORITATIVE (an unmapped field then
materializes its default/NULL instead
* of silently matching a physical column by its current name; see {@link
#buildField}). Port of legacy
- * {@code IcebergScanNode.extractNameMapping} + {@code
IcebergUtils.getNameMapping} (#65784). A malformed
- * property fails soft to a current-name mapping instead of becoming
indistinguishable from no property.
+ * {@code IcebergScanNode.extractNameMapping} + {@code
IcebergUtils.getNameMapping} (#65784).
+ *
+ * <p>A property that is present but cannot be parsed is a metadata fault,
not an absent mapping: Iceberg
+ * readers refuse such tables (Spark parses {@code
schema.name-mapping.default} while constructing the file
+ * reader), so fail instead of degrading to current-schema aliases. Those
aliases cannot resolve the old
+ * physical columns of ID-less files after a rename — the scan would
silently return NULL, or bind the wrong
+ * physical column once a name has been reused.
+ *
+ * @throws DorisConnectorException if the property is present but cannot
be parsed as a name mapping
*/
static Optional<Map<Integer, List<String>>> extractNameMapping(Table
table) {
String nameMappingJson =
table.properties().get(TableProperties.DEFAULT_NAME_MAPPING);
@@ -231,12 +239,14 @@ public final class IcebergSchemaUtils {
collectNameMappings(mapping.asMappedFields(), result);
return Optional.of(result);
} catch (Exception e) {
- // Preserve legacy current-name readability for ID-less files when
a malformed table property
- // cannot provide authoritative aliases; Optional.empty() now
means the property is truly absent.
- LOG.warn("Failed to parse name mapping from Iceberg table
properties", e);
- Map<Integer, List<String>> fallback = new HashMap<>();
-
collectNameMappings(MappingUtil.create(table.schema()).asMappedFields(),
fallback);
- return Optional.of(fallback);
+ LOG.warn("Failed to parse name mapping of table {}", table.name(),
e);
+ throw new DorisConnectorException(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, table.name(),
+ ExceptionUtils.getRootCauseMessage(e)), e);
}
}
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergSchemaUtilsTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergSchemaUtilsTest.java
index aaa597580e1..bc184d50d90 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergSchemaUtilsTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergSchemaUtilsTest.java
@@ -17,6 +17,7 @@
package org.apache.doris.connector.iceberg;
+import org.apache.doris.connector.spi.DorisConnectorException;
import org.apache.doris.thrift.TFileScanRangeParams;
import org.apache.doris.thrift.TPrimitiveType;
import org.apache.doris.thrift.schema.external.TField;
@@ -715,21 +716,33 @@ public class IcebergSchemaUtilsTest {
}
@Test
- public void malformedNameMappingKeepsIdlessCurrentNameFallback() {
- // A malformed name-mapping property must not break the scan or look
identical to a genuinely absent
- // mapping. Required and optional fields both need current-name
aliases for ID-less legacy files.
+ public void malformedNameMappingFailsInsteadOfFallingBackToCurrentNames() {
+ // Iceberg refuses to read a table whose name mapping cannot be parsed
(Spark's BaseReader parses the
+ // property while constructing the file reader), so the connector must
surface the metadata fault.
+ // Rewriting the property into current-schema aliases would silently
return NULL for the renamed
+ // columns of ID-less files instead of reporting it.
Table table = createTable("t1", SCHEMA,
Collections.singletonMap(TableProperties.DEFAULT_NAME_MAPPING,
"{not valid json"));
- Map<Integer, List<String>> fallback =
IcebergSchemaUtils.extractNameMapping(table).orElseThrow();
- Assertions.assertEquals(Collections.singletonList("id"),
fallback.get(1));
- Assertions.assertEquals(Collections.singletonList("name"),
fallback.get(2));
+ DorisConnectorException exception =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> IcebergSchemaUtils.extractNameMapping(table));
+
Assertions.assertTrue(exception.getMessage().contains(TableProperties.DEFAULT_NAME_MAPPING));
+ Assertions.assertTrue(exception.getMessage().contains("t1"));
+ }
+
+ @Test
+ public void validEmptyNameMappingStaysAuthoritative() {
+ // An explicitly empty mapping is NOT the same as an absent property:
it stays authoritative, so an
+ // ID-less file's columns resolve to their defaults/NULLs instead of
matching by current name.
+ Table table = createTable("t1", SCHEMA,
+ Collections.singletonMap(TableProperties.DEFAULT_NAME_MAPPING,
"[]"));
+
+ Map<Integer, List<String>> mapping =
IcebergSchemaUtils.extractNameMapping(table).orElseThrow();
+ Assertions.assertTrue(mapping.isEmpty());
Map<String, TField> fields = topFields(dict(table, "id", "name"));
Assertions.assertTrue(fields.get("id").isNameMappingIsAuthoritative());
- Assertions.assertEquals(Collections.singletonList("id"),
fields.get("id").getNameMapping());
-
Assertions.assertTrue(fields.get("name").isNameMappingIsAuthoritative());
- Assertions.assertEquals(Collections.singletonList("name"),
fields.get("name").getNameMapping());
+ Assertions.assertTrue(fields.get("id").getNameMapping().isEmpty());
}
// --- round-trip through the prop transport (what the generic node does)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
