github-actions[bot] commented on code in PR #65502:
URL: https://github.com/apache/doris/pull/65502#discussion_r3569872229
##########
be/src/format/table/iceberg_reader.cpp:
##########
@@ -261,12 +262,30 @@ Status
IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) {
}
}
- std::string file_col_name = old_name;
- auto it = field_id_to_file_col_name.find(field_id);
- if (it != field_id_to_file_col_name.end()) {
- file_col_name = it->second;
+ const FieldSchema* file_column = nullptr;
+ auto id_it = field_id_to_file_column.find(field_id);
Review Comment:
This still bypasses name-mapping mode for partial-ID files. The normal
Parquet and ORC `*_field_id_with_name_mapping()` helpers switch to
name/name_mapping matching once any top-level file field lacks an Iceberg id,
but both hidden equality-key loops first accept any physical column whose id
equals the delete key id and only consult `table_info_node` if that lookup
misses. For a migrated file where the real key is an id-less alias and another
column still carries the stale key id, the equality delete will compare against
the unrelated column and miss or misapply deletes. Please prefer the
already-resolved `table_info_node` mapping in name-mapping mode, or skip this
id fast path whenever the file is not all-id-mapped, and cover both V1 Parquet
and V1 ORC partial-ID/name-mapping files.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalUtil.java:
##########
@@ -34,9 +34,11 @@
import org.apache.doris.thrift.schema.external.TStructField;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
Review Comment:
This import is unused in the post-PR file, and FE checkstyle runs
`UnusedImports` during Maven `validate` with `failsOnError=true`, so FE
validation will fail before tests run. Please remove it.
##########
fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalUtilTest.java:
##########
@@ -40,6 +40,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
Review Comment:
This import is unused, and FE checkstyle also scans test sources via
`includeTestSourceDirectory=true`. With `UnusedImports` enabled and
`failsOnError=true`, FE validation will fail here. Please remove it.
##########
fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java:
##########
@@ -75,6 +75,8 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
Review Comment:
This import is unused, and FE checkstyle also scans test sources via
`includeTestSourceDirectory=true`. With `UnusedImports` enabled and
`failsOnError=true`, FE validation will fail here. Please remove it.
--
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]
