github-actions[bot] commented on code in PR #65610:
URL: https://github.com/apache/doris/pull/65610#discussion_r3580922888
##########
be/src/format/table/iceberg_reader.cpp:
##########
@@ -365,32 +363,14 @@ ColumnIdResult IcebergParquetReader::_create_column_ids(
};
for (const auto* slot : tuple_descriptor->slots()) {
- const FieldSchema* field_schema = nullptr;
- if (table_info_node != nullptr) {
- if (table_info_node->children_column_exists(slot->col_name())) {
- // Use the physical child selected by the schema-mapping pass.
This keeps partial-id
- // files in BY_NAME mode from binding a projected column
through an unrelated stale
- // field id.
- const auto& file_column_name =
-
table_info_node->children_file_column_name(slot->col_name());
- for (int i = 0; i < field_desc->size(); ++i) {
- const auto* candidate = field_desc->get_column(i);
- if (candidate != nullptr && candidate->name ==
file_column_name) {
- field_schema = candidate;
- break;
- }
- }
- DORIS_CHECK(field_schema != nullptr);
- }
- } else {
- auto it =
iceberg_id_to_field_schema_map.find(slot->col_unique_id());
- if (it != iceberg_id_to_field_schema_map.end()) {
- field_schema = it->second;
- }
- }
- if (field_schema == nullptr) {
+ // Match projected slots to file columns by Iceberg field id.
Synthesized/metadata slots
+ // (e.g. the TopN global row-id or the $row_id column) carry a
sentinel col_unique_id that
Review Comment:
This now makes the column-id pruning decision independently from the
schema-mapping decision above. For Iceberg files where any Parquet field is
missing `field_id`, `by_parquet_field_id_with_name_mapping()` intentionally
switches that level to name/name-mapping and `table_info_node` may map a
projected nested column to physical `legacy_struct`. With this lookup,
`_column_ids` can still come from another physical field that happens to carry
the table field id, or skip the id-less physical field entirely.
`_init_read_columns()` will then ask the row-group reader to read the
name-mapped `legacy_struct`, but `ParquetColumnReader::create()` prunes
struct/map children using the non-empty `_column_ids` set and installs
`SkipReadingReader` for the real children, so nested values can come back as
defaults/nulls. Please derive these ids from the same physical child selected
by `table_info_node` in the BY_NAME/name-mapping case, or leave the set
unrestrictive when file ids are not trustworth
y.
##########
be/src/format/table/iceberg_reader.cpp:
##########
@@ -692,31 +670,14 @@ ColumnIdResult IcebergOrcReader::_create_column_ids(
};
for (const auto* slot : tuple_descriptor->slots()) {
- const orc::Type* orc_field = nullptr;
- if (table_info_node != nullptr) {
- if (table_info_node->children_column_exists(slot->col_name())) {
- // Select the physical child resolved by the shared
schema-mapping pass. Hidden
- // equality keys and projected columns must obey the same
BY_NAME decision for
- // partial-id ORC files.
- const auto& file_column_name =
-
table_info_node->children_file_column_name(slot->col_name());
- for (uint64_t i = 0; i < orc_type->getSubtypeCount(); ++i) {
- if (orc_type->getFieldName(i) == file_column_name) {
- orc_field = orc_type->getSubtype(i);
- break;
- }
- }
- DORIS_CHECK(orc_field != nullptr);
- }
- } else {
- auto it = iceberg_id_to_orc_type_map.find(slot->col_unique_id());
- if (it != iceberg_id_to_orc_type_map.end()) {
- orc_field = it->second;
- }
- }
Review Comment:
This has the same partial-id/name-mapping split as the Parquet path, but ORC
fails even for a scalar projected column. `by_orc_field_id_with_name_mapping()`
falls back to name/name-mapping when any child lacks the `iceberg.id`
attribute, so `_read_file_cols` can correctly contain the mapped physical name.
The new id-only set can still be non-empty from another projected column or a
stale id on an unrelated ORC field, which makes `_init_orc_row_reader()` call
`includeTypes(_column_ids)` instead of `include(_read_file_cols)`. The selected
ORC schema can then omit the required mapped field and the read path will hit
`Wrong read column` or read the stale selected type. When the mapping pass is
in BY_NAME mode, please keep ORC projection name-based or compute type ids from
the exact physical fields stored in `table_info_node`.
--
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]
