Gabriel39 commented on code in PR #65784:
URL: https://github.com/apache/doris/pull/65784#discussion_r3610091107
##########
be/src/exec/scan/access_path_parser.cpp:
##########
@@ -87,6 +87,9 @@ void inherit_schema_metadata(format::ColumnDefinition* column,
return;
}
column->name_mapping = schema_column->name_mapping;
+ // The presence bit is part of the mapping contract: an explicit empty
mapping must remain
+ // authoritative after access-path pruning instead of enabling
current-name fallback.
+ column->has_name_mapping = schema_column->has_name_mapping;
Review Comment:
Addressed in adfe227951e. Access-path pruning now preserves initial-default
payload and Base64 state recursively; both reader generations decode and
materialize missing nested defaults. Added binary default metadata and result
coverage.
##########
be/src/format_v2/table/iceberg_reader.h:
##########
@@ -77,24 +77,21 @@ class IcebergTableReader : public format::TableReader {
private:
struct EqualityDeleteFilter;
- bool _has_field_id(const std::vector<format::ColumnDefinition>& schema)
const {
+ bool _has_any_field_id(const std::vector<format::ColumnDefinition>&
schema) const {
for (const auto& field : schema) {
- // TopN lazy materialization asks the file reader to synthesize
GLOBAL_ROWID in the
- // first-phase scan. That virtual column is not an Iceberg data
field and therefore has
- // no Iceberg field id. Do not let it downgrade schema-evolution
reads to BY_NAME,
- // otherwise old data files whose physical names predate a rename
(for example,
- // table column `new_new_id` stored as file column `id`) are
materialized as defaults.
+ // Iceberg's hasIds contract is existential. Ignore synthesized
columns and keep ID
+ // projection as soon as any real field (including a nested field)
carries an ID.
if (field.column_type != format::ColumnType::DATA_COLUMN) {
continue;
}
- if (!field.has_identifier_field_id()) {
- return false;
+ if (field.has_identifier_field_id()) {
Review Comment:
Addressed in adfe227951e. The stale-ID fixtures now follow Iceberg's
existential hasIds contract: the physical field carrying the requested ID is
authoritative. Updated both V1 formats and V2 expectations; all three former CI
failures pass locally.
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -488,6 +489,7 @@ Status TableReader::annotate_projected_column(const
TFileScanSlotInfo& slot_info
context->schema_column =
build_schema_column_from_external_field(*schema_field, column->type);
column->identifier = context->schema_column->identifier;
column->name_mapping = context->schema_column->name_mapping;
+ column->has_name_mapping = context->schema_column->has_name_mapping;
Review Comment:
Addressed by 54ab787b88a and covered by the AccessPathParser nested
projection tests. The mapping presence bit is preserved together with aliases,
including explicit empty authoritative mappings.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalUtil.java:
##########
@@ -178,9 +178,11 @@ private static TField getExternalSchema(Type columnType,
Column dorisColumn,
root.setInitialDefaultValue(dorisColumn.getDefaultValue());
}
- if (nameMapping != null &&
nameMapping.containsKey(dorisColumn.getUniqueId())) {
- // for iceberg set name mapping.
- root.setNameMapping(new
ArrayList<>(nameMapping.get(dorisColumn.getUniqueId())));
+ if (nameMapping != null && !nameMapping.isEmpty()) {
Review Comment:
Addressed by 54ab787b88a. FE preserves mapping presence independently of
list contents, so an explicitly empty mapping remains distinguishable from
absent mapping metadata.
--
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]
