github-actions[bot] commented on code in PR #65610:
URL: https://github.com/apache/doris/pull/65610#discussion_r3580922893
##########
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`.
##########
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;
- }
- }
- if (orc_field == 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
+ // is not a real field id, so they miss the map and are skipped.
+ auto it = iceberg_id_to_orc_type_map.find(slot->col_unique_id());
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]
