github-actions[bot] commented on code in PR #65784:
URL: https://github.com/apache/doris/pull/65784#discussion_r3610533880
##########
be/src/format/table/table_schema_change_helper.cpp:
##########
@@ -660,37 +774,44 @@ Status
TableSchemaChangeHelper::BuildTableInfoUtil::by_parquet_field_id_with_nam
auto struct_node =
std::make_shared<TableSchemaChangeHelper::StructNode>();
std::map<int32_t, size_t> file_column_id_idx_map;
- bool all_have_field_id = true;
+ // Apply the same any-ID rule recursively so nested mixed-ID structs
retain ID matches.
+ bool has_field_id = false;
Review Comment:
[P1] Keep Parquet's whole-file ID mode through nested structs
This recomputes the mapping mode from only the matched struct's direct
children. For table `id#1, s#10<a#2 names=[legacy_a]>` and Parquet file `id#1,
s#10<legacy_a(no id)>`, the root correctly enters ID mode and matches `s#10`,
but this recursion sees no child IDs and alias-matches `a` to `legacy_a`.
Iceberg's existential `hasIds` decision is whole-file, so once any file field
has an ID, that ID-less primitive cannot satisfy `a#2`; V2 keeps global
`BY_FIELD_ID` and materializes the child as missing/default. Please compute V1
Parquet's mode once for the whole file and thread it through recursion,
retaining only the existing complex-wrapper exception for shared descendant
IDs. This is distinct from the wrapper threads because the parent already
matched by ID and the primitive child must not be rebound by name.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -270,25 +273,27 @@ protected void doInitialize() throws UserException {
/**
* Extract name mapping from Iceberg table properties.
- * Returns a map from field ID to list of mapped names.
+ * Returns a present map, possibly empty, only when the property parsed
successfully.
*/
- private Map<Integer, List<String>> extractNameMapping() {
+ private Optional<Map<Integer, List<String>>> extractNameMapping() {
Map<Integer, List<String>> result = new HashMap<>();
+ String nameMappingJson =
icebergTable.properties().get(TableProperties.DEFAULT_NAME_MAPPING);
Review Comment:
[P1] Pin the name mapping to the statement metadata
The target Columns below are resolved through the statement's
`IcebergMvccSnapshot`, but this property comes from the `Table` loaded later by
`doInitialize()`. If the property is absent when a statement binds, then a
property-only commit installs valid `[]` and invalidates Doris's cache before
scan initialization, the schema ID stays unchanged while this reads the
refreshed property. This PR then sends authoritative-empty aliases for the
pinned Columns, so both scanners reject an ID-less file column whose current
name should still match for that statement and materialize the initial
default/NULL instead. The snapshot payload retains no table properties, so
please pin the parsed mapping, including absent versus present-empty state, to
the same metadata generation as the statement schema and cover this refresh
race. This is separate from r3610384732, which pinned only the binary-default
markers.
--
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]
