github-actions[bot] commented on code in PR #69547:
URL: https://github.com/apache/doris/pull/69547#discussion_r4365360556
##########
be/src/format/table/iceberg_reader.cpp:
##########
@@ -570,6 +668,44 @@ Status
IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) {
}
return nullptr;
};
+ std::unordered_map<int32_t, const SlotDescriptor*> projected_slots;
+ for (const auto* slot : ctx->tuple_descriptor->slots()) {
+ projected_slots.emplace(slot->col_unique_id(), slot);
+ }
+ // Finds a nested key inside its root struct when the query projects that
root, so the key is
+ // taken from the projected column instead of reading the root a second
time.
+ const auto find_projected_root = [&](int32_t field_id, const DataTypePtr&
key_type,
+ std::string* root_name,
+ std::vector<size_t>* child_indexes)
-> Status {
+ const auto key_path = _find_schema_field_path(field_id);
+ if (key_path.empty()) {
+ return Status::OK();
+ }
+ DORIS_CHECK(key_path.front()->__isset.id);
+ const auto slot = projected_slots.find(key_path.front()->id);
+ if (slot == projected_slots.end()) {
+ return Status::OK();
+ }
+ std::vector<const schema::external::TField*> root_path;
+ if (!_find_schema_field_path_in_root(_current_schema_root(),
key_path.front()->id,
+ &root_path)) {
+ return Status::OK();
+ }
+ DORIS_CHECK(root_path.size() == 1);
+ DataTypePtr leaf_type;
+ bool found = false;
+ RETURN_IF_ERROR(find_projected_equality_key_path(*root_path.front(),
key_path,
+ slot->second->type(),
child_indexes,
+ &leaf_type, &found));
+ // The projected leaf has the table's current type. A key written
before a type promotion
+ // (INT promoted to BIGINT) keeps its own type, so it still reads the
physical root.
+ if (found &&
remove_nullable(leaf_type)->equals(*remove_nullable(key_type))) {
Review Comment:
[P1] Verify physical binding before reusing a projected key
For a legacy FE plan without `iceberg_scan_semantics_version`, the projected
struct and the hidden key can resolve different physical fields. For example,
let the file contain ID-tagged roots `id#1` and `payload#2`, with children
`old_key#3 INT` and an unrelated ID-less sibling; the current projected type is
`payload<new_key#3 INT>`, with no name-mapping aliases. Hidden-key lookup finds
field 3 by ID, but `BuildTableInfoUtil::by_parquet_field_id_with_name_mapping`
switches the projected struct's children to name matching when that sibling
lacks an ID (`table_schema_change_helper.cpp:895-921`), so `new_key` is filled
with NULL. This logical-path/type check still selects `payload`, and the reuse
branch skips the physical carrier. An equality delete on field 3 = 20 then
compares NULL and silently retains the deleted row. Older FE source actually
emits nested schema metadata without the marker, so this is reachable during a
BE-first rolling upgrade. Please verify that the projected sc
hema-mapping path resolves the same physical leaf as `file_path`, or fall back
to the carrier, and cover this legacy mixed-ID/renamed-key case. The new tests
all set semantics v2 and miss this branch.
--
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]
