github-actions[bot] commented on code in PR #67690:
URL: https://github.com/apache/doris/pull/67690#discussion_r3964554968
##########
be/src/format_v2/table/iceberg_reader.h:
##########
@@ -69,6 +69,12 @@ class IcebergTableReader : public format::TableReader {
if (!_data_reader.file_schema.empty() && has_field_ids) {
return format::TableColumnMappingMode::BY_FIELD_ID;
}
+ if (!_data_reader.file_schema.empty() &&
supports_iceberg_scan_semantics_v2(_scan_params) &&
Review Comment:
[P1] Fence this result-changing rule during rolling upgrades
This backport changes fully ID-less files from `BY_NAME` to `BY_FIELD_ID`
under the already-existing semantics-v2 marker. A smooth-upgrade source BE
still binds current physical names, while an upgraded BE returns
NULL/default/error for the same optional projection. The FE's current
compatibility gates do not cover an ordinary optional scan with no mapping,
defaults, deletes, required-field risk, or alias collision, so splits from one
query can observe different results. Please introduce a new capability/version
or conservatively fence source BEs for this case.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java:
##########
@@ -2333,8 +2334,13 @@ public static Optional<Map<Integer, List<String>>>
getNameMapping(Table icebergT
extractMappingsFromNameMapping(mapping.asMappedFields(), result);
return Optional.of(result);
} catch (Exception e) {
Review Comment:
[P1] Route an empty mapping property through the malformed fallback
A present empty string returns `Optional.empty()` before this catch, even
though Iceberg accepts non-null string property values and its name-mapping
parser requires JSON array input. Under the new V2 branch, that absent carrier
selects `BY_FIELD_ID` for ID-less files, so a table that previously fell back
to current names now produces NULL/default/error instead of taking the
current-schema fallback added here. Please reserve `Optional.empty()` for a
missing property and handle `""` like the other malformed JSON values; the new
test only covers a nonempty invalid string.
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -2682,6 +2778,17 @@ Status TableColumnMapper::localize_filters(const
std::vector<TableFilter>& table
FileScanRequestBuilder builder(file_request);
RETURN_IF_ERROR(builder.add_non_predicate_column(std::move(demoted_projection)));
}
+ // Predicate demotion can widen a nested projection after mappings were
localized. Reapply the
+ // final shape so TableReader interprets the same child ordinals that
FileReader returns.
+ for (auto& mapping : _mappings) {
+ if (mapping.file_local_id.has_value() &&
+
file_request->local_positions.contains(LocalColumnId(*mapping.file_local_id))) {
Review Comment:
[P1] Preserve the active row-position index across rejected refreshes
This compaction also runs while rebuilding a fixed-layout request. With an
active nested predicate at slot 0, deferred output at 1, and Iceberg row
position at 2, refresh seeds only `{root:0,rowpos:2}`, recreates the deferred
slot at 3, and compacts row position to 1. `customize_file_scan_request()` then
commits `_row_position_block_position=1`; `same_physical_scan_layout()` rejects
the new map, but that member mutation survives while the active template still
has the deferred complex value at slot 1 and row position at 2. The next row-id
materialization casts slot 1 as `ColumnInt64`. This is newly failing even for a
one-row-group file spanning multiple batches: the base never activates its
incompatible pending request before EOF, while this mutation breaks the next
active-layout batch. Please keep fixed layouts unrenumbered during refresh or
commit reader-owned positions only after compatibility acceptance.
--
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]
