github-actions[bot] commented on code in PR #65675:
URL: https://github.com/apache/doris/pull/65675#discussion_r3597966126
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java:
##########
@@ -70,16 +80,16 @@ public StructField conversion() {
}
public StructField withDataType(DataType dataType) {
- return new StructField(name, dataType, nullable, comment);
+ return new StructField(originalName, dataType, nullable, comment);
}
public StructField withDataTypeAndNullable(DataType dataType, boolean
nullable) {
- return new StructField(name, dataType, nullable, comment);
+ return new StructField(originalName, dataType, nullable, comment);
}
public org.apache.doris.catalog.StructField toCatalogDataType() {
return new org.apache.doris.catalog.StructField(
- name, dataType.toCatalogDataType(), comment, nullable);
+ originalName, dataType.toCatalogDataType(), comment, nullable);
Review Comment:
This still loses the preserved spelling on the normal Nereids query path.
For an Iceberg field `CamelName`, `DataType.fromCatalogType` records it as this
field's `originalName`, but this method calls the public catalog constructor,
which lowercases `name`. `PlanTranslatorContext` puts that round-tripped type
in the slot descriptor, and BE builds the struct serializer's JSON keys from
those descriptor names, so `SELECT s` emits `camelname` while the
catalog/DESCRIBE path reports `CamelName`. Preserve the catalog field name
across this round trip (without changing internal-table normalization) and
cover the default-Nereids descriptor/result path.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -270,20 +270,25 @@ protected void doInitialize() throws UserException {
* Returns a map from field ID to list of mapped names.
*/
private Map<Integer, List<String>> extractNameMapping() {
+ String nameMappingJson =
icebergTable.properties().get(TableProperties.DEFAULT_NAME_MAPPING);
+ if (nameMappingJson == null || nameMappingJson.isEmpty()) {
+ // Table does not define a name mapping: return null so callers
can distinguish
+ // "table uses name mapping" (missing entries must resolve to
NULL) from "no name mapping
+ // at all" (legacy physical-name matching is allowed). A non-null
result may still be empty.
+ return null;
+ }
Map<Integer, List<String>> result = new HashMap<>();
try {
- String nameMappingJson =
icebergTable.properties().get(TableProperties.DEFAULT_NAME_MAPPING);
- if (nameMappingJson != null && !nameMappingJson.isEmpty()) {
- NameMapping mapping =
NameMappingParser.fromJson(nameMappingJson);
- if (mapping != null) {
- // Extract mappings from NameMapping
- // NameMapping contains field mappings, we need to convert
them to our format
- extractMappingsFromNameMapping(mapping.asMappedFields(),
result);
- }
+ NameMapping mapping = NameMappingParser.fromJson(nameMappingJson);
+ if (mapping != null) {
+ // Extract mappings from NameMapping
+ // NameMapping contains field mappings, we need to convert
them to our format
+ extractMappingsFromNameMapping(mapping.asMappedFields(),
result);
}
} catch (Exception e) {
- // If name mapping parsing fails, continue without it
+ // If name mapping parsing fails, fall back to legacy behavior to
avoid all-NULL reads.
LOG.warn("Failed to parse name mapping from Iceberg table
properties", e);
+ return null;
Review Comment:
A present mapping that cannot be parsed must not be treated as if the table
had no mapping. Returning `null` makes `ExternalUtil` omit the mapping-present
signal, so an id-less file after a rename can silently bind by its current
physical name (or materialize NULL for the historical name) instead of using
the declared history. That produces successful but incorrect reads from
corrupted/misconfigured metadata, contrary to the error-means-failure contract.
Propagate a user-visible scan-planning error with table/property context and
add an invalid-mapping test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
