From 58188bd36a7ee0e3df9400da76eb5277453e2960 Mon Sep 17 00:00:00 2001 From: daidai Date: Wed, 15 Jul 2026 00:13:33 +0800 Subject: [PATCH 1/4] [fix](iceberg) Match column ids by field id to skip synthesized slots _create_column_ids resolved every projected slot to a file column by name through the schema-mapping StructNode. Synthesized/metadata slots such as the TopN global row-id and the $row_id column are never serialized into the schema tree, so they are not registered as struct children. Asking the node about them made StructNode::children_column_exists() hit its DCHECK(children.contains(name)) and abort (and throw std::out_of_range from .at() in release builds), crashing Parquet/ORC reader init on a TopN projection over an Iceberg table. Restore matching by Iceberg field id (col_unique_id): synthesized slots carry a sentinel col_unique_id that is not a real field id, so they miss the field-id map and are skipped, exactly as before. Drop the now-unused table_info_node parameter from both the Parquet and ORC overloads. --- be/src/format/table/iceberg_reader.cpp | 73 +++++++------------------- be/src/format/table/iceberg_reader.h | 10 ++-- 2 files changed, 22 insertions(+), 61 deletions(-) diff --git a/be/src/format/table/iceberg_reader.cpp b/be/src/format/table/iceberg_reader.cpp index 38bb395120df24..50cf36dc31a090 100644 --- a/be/src/format/table/iceberg_reader.cpp +++ b/be/src/format/table/iceberg_reader.cpp @@ -222,7 +222,7 @@ Status IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) { // Create column IDs from field descriptor auto column_id_result = - _create_column_ids(field_desc, ctx->tuple_descriptor, ctx->table_info_node); + _create_column_ids(field_desc, ctx->tuple_descriptor); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); @@ -337,9 +337,8 @@ Status IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) { // ============================================================================ // IcebergParquetReader: _create_column_ids // ============================================================================ -ColumnIdResult IcebergParquetReader::_create_column_ids( - const FieldDescriptor* field_desc, const TupleDescriptor* tuple_descriptor, - const std::shared_ptr& table_info_node) { +ColumnIdResult IcebergParquetReader::_create_column_ids(const FieldDescriptor* field_desc, + const TupleDescriptor* tuple_descriptor) { auto* mutable_field_desc = const_cast(field_desc); mutable_field_desc->assign_ids(); @@ -365,32 +364,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 + // is not a real field id, so they miss the map and are skipped. + auto it = iceberg_id_to_field_schema_map.find(slot->col_unique_id()); + if (it == iceberg_id_to_field_schema_map.end()) { continue; } + const auto* field_schema = it->second; if ((slot->col_type() != TYPE_STRUCT && slot->col_type() != TYPE_ARRAY && slot->col_type() != TYPE_MAP)) { @@ -555,7 +536,7 @@ Status IcebergOrcReader::on_before_init_reader(ReaderInitContext* ctx) { // Create column IDs from ORC type auto column_id_result = - _create_column_ids(orc_type_ptr, ctx->tuple_descriptor, ctx->table_info_node); + _create_column_ids(orc_type_ptr, ctx->tuple_descriptor); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); @@ -662,9 +643,8 @@ Status IcebergOrcReader::on_before_init_reader(ReaderInitContext* ctx) { // ============================================================================ // IcebergOrcReader: _create_column_ids // ============================================================================ -ColumnIdResult IcebergOrcReader::_create_column_ids( - const orc::Type* orc_type, const TupleDescriptor* tuple_descriptor, - const std::shared_ptr& table_info_node) { +ColumnIdResult IcebergOrcReader::_create_column_ids(const orc::Type* orc_type, + const TupleDescriptor* tuple_descriptor) { std::unordered_map iceberg_id_to_orc_type_map; for (uint64_t i = 0; i < orc_type->getSubtypeCount(); ++i) { const auto* orc_sub_type = orc_type->getSubtype(i); @@ -692,31 +672,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()); + if (it == iceberg_id_to_orc_type_map.end()) { continue; } + const orc::Type* orc_field = it->second; if ((slot->col_type() != TYPE_STRUCT && slot->col_type() != TYPE_ARRAY && slot->col_type() != TYPE_MAP)) { diff --git a/be/src/format/table/iceberg_reader.h b/be/src/format/table/iceberg_reader.h index 9a237848b522f7..205e46a3a1bf7b 100644 --- a/be/src/format/table/iceberg_reader.h +++ b/be/src/format/table/iceberg_reader.h @@ -105,9 +105,8 @@ class IcebergParquetReader final : public IcebergReaderMixin { this->get_state(), this->_meta_cache); } - static ColumnIdResult _create_column_ids( - const FieldDescriptor* field_desc, const TupleDescriptor* tuple_descriptor, - const std::shared_ptr& table_info_node = nullptr); + static ColumnIdResult _create_column_ids(const FieldDescriptor* field_desc, + const TupleDescriptor* tuple_descriptor); private: Status _read_position_delete_file(const TFileRangeDesc* delete_range, @@ -152,9 +151,8 @@ class IcebergOrcReader final : public IcebergReaderMixin { this->get_io_ctx(), this->_meta_cache); } - static ColumnIdResult _create_column_ids( - const orc::Type* orc_type, const TupleDescriptor* tuple_descriptor, - const std::shared_ptr& table_info_node = nullptr); + static ColumnIdResult _create_column_ids(const orc::Type* orc_type, + const TupleDescriptor* tuple_descriptor); static const std::string ICEBERG_ORC_ATTRIBUTE; From c59f2fe030996f996eb0b9145ce84f7e65cf2ec3 Mon Sep 17 00:00:00 2001 From: daidai Date: Wed, 15 Jul 2026 00:19:59 +0800 Subject: [PATCH 2/4] fix format --- be/src/format/table/iceberg_reader.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/be/src/format/table/iceberg_reader.cpp b/be/src/format/table/iceberg_reader.cpp index 50cf36dc31a090..0d432041db9b49 100644 --- a/be/src/format/table/iceberg_reader.cpp +++ b/be/src/format/table/iceberg_reader.cpp @@ -221,8 +221,7 @@ Status IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) { _all_required_col_names = ctx->column_names; // Create column IDs from field descriptor - auto column_id_result = - _create_column_ids(field_desc, ctx->tuple_descriptor); + auto column_id_result = _create_column_ids(field_desc, ctx->tuple_descriptor); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); @@ -535,8 +534,7 @@ Status IcebergOrcReader::on_before_init_reader(ReaderInitContext* ctx) { _all_required_col_names = ctx->column_names; // Create column IDs from ORC type - auto column_id_result = - _create_column_ids(orc_type_ptr, ctx->tuple_descriptor); + auto column_id_result = _create_column_ids(orc_type_ptr, ctx->tuple_descriptor); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); From 4602bafd2ac401484dd93b7e70f7956044ccffc0 Mon Sep 17 00:00:00 2001 From: daidai Date: Wed, 15 Jul 2026 15:31:09 +0800 Subject: [PATCH 3/4] [fix](iceberg) Guard synthesized slots in _create_column_ids _create_column_ids resolves each projected slot to a file column by name through the schema-mapping StructNode, keeping the column-id set consistent with the BY_ID / BY_NAME (name-mapping) decision. Synthesized/metadata slots such as the TopN global row-id and the $row_id column are never serialized into the schema tree, so they are absent from the node. Calling StructNode::children_column_exists() on such an unregistered name hits DCHECK(children.contains(name)) and aborts in debug/ASAN builds (and throws std::out_of_range from .at() in release builds), crashing Parquet/ORC reader init on a TopN projection over an Iceberg table. Guard the lookup with StructNode::get_children().contains() before querying the child, matching the existing pattern used for equality-delete expand columns. Synthesized slots are skipped instead of aborting; the name-based resolution and its partial-id/name-mapping correctness are unchanged, and the shared children_column_exists() helper is left untouched. Add BE unit tests (Parquet and ORC) that project a synthesized global row-id slot absent from the schema-mapping node; they abort without the guard and pass with it. Make enable_file_scanner_v2 a fuzzy session variable so external regression runs randomly exercise both the V1 and V2 file scanners, the paths this fix spans. --- be/src/format/table/iceberg_reader.cpp | 99 +++++++++++++++---- be/src/format/table/iceberg_reader.h | 10 +- .../iceberg_reader_create_column_ids_test.cpp | 72 ++++++++++++++ .../org/apache/doris/qe/SessionVariable.java | 5 +- 4 files changed, 163 insertions(+), 23 deletions(-) diff --git a/be/src/format/table/iceberg_reader.cpp b/be/src/format/table/iceberg_reader.cpp index 0d432041db9b49..d4040d7ec4ae62 100644 --- a/be/src/format/table/iceberg_reader.cpp +++ b/be/src/format/table/iceberg_reader.cpp @@ -221,7 +221,8 @@ Status IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) { _all_required_col_names = ctx->column_names; // Create column IDs from field descriptor - auto column_id_result = _create_column_ids(field_desc, ctx->tuple_descriptor); + auto column_id_result = + _create_column_ids(field_desc, ctx->tuple_descriptor, ctx->table_info_node); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); @@ -336,8 +337,9 @@ Status IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) { // ============================================================================ // IcebergParquetReader: _create_column_ids // ============================================================================ -ColumnIdResult IcebergParquetReader::_create_column_ids(const FieldDescriptor* field_desc, - const TupleDescriptor* tuple_descriptor) { +ColumnIdResult IcebergParquetReader::_create_column_ids( + const FieldDescriptor* field_desc, const TupleDescriptor* tuple_descriptor, + const std::shared_ptr& table_info_node) { auto* mutable_field_desc = const_cast(field_desc); mutable_field_desc->assign_ids(); @@ -362,15 +364,45 @@ ColumnIdResult IcebergParquetReader::_create_column_ids(const FieldDescriptor* f IcebergParquetNestedColumnUtils::extract_nested_column_ids); }; + // The Iceberg schema-mapping root is a StructNode whose registered children are the real + // table columns. When present, resolve each column by name through it so the column-id set + // stays consistent with the schema-mapping decision (BY_ID or BY_NAME/name-mapping); + // otherwise fall back to matching by Iceberg field id. + const auto* struct_node = + dynamic_cast(table_info_node.get()); + for (const auto* slot : tuple_descriptor->slots()) { - // 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_field_schema_map.find(slot->col_unique_id()); - if (it == iceberg_id_to_field_schema_map.end()) { + const FieldSchema* field_schema = nullptr; + if (struct_node != nullptr) { + // Synthesized/metadata slots (e.g. the TopN global row-id or the $row_id column) are + // never registered as children, so check membership before querying: calling + // children_column_exists() on an unregistered name DCHECK-aborts in debug builds and + // throws std::out_of_range from .at() in release builds. + if (struct_node->get_children().contains(slot->col_name()) && + struct_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 = + struct_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) { continue; } - const auto* field_schema = it->second; if ((slot->col_type() != TYPE_STRUCT && slot->col_type() != TYPE_ARRAY && slot->col_type() != TYPE_MAP)) { @@ -534,7 +566,8 @@ Status IcebergOrcReader::on_before_init_reader(ReaderInitContext* ctx) { _all_required_col_names = ctx->column_names; // Create column IDs from ORC type - auto column_id_result = _create_column_ids(orc_type_ptr, ctx->tuple_descriptor); + auto column_id_result = + _create_column_ids(orc_type_ptr, ctx->tuple_descriptor, ctx->table_info_node); ctx->column_ids = std::move(column_id_result.column_ids); ctx->filter_column_ids = std::move(column_id_result.filter_column_ids); @@ -641,8 +674,9 @@ Status IcebergOrcReader::on_before_init_reader(ReaderInitContext* ctx) { // ============================================================================ // IcebergOrcReader: _create_column_ids // ============================================================================ -ColumnIdResult IcebergOrcReader::_create_column_ids(const orc::Type* orc_type, - const TupleDescriptor* tuple_descriptor) { +ColumnIdResult IcebergOrcReader::_create_column_ids( + const orc::Type* orc_type, const TupleDescriptor* tuple_descriptor, + const std::shared_ptr& table_info_node) { std::unordered_map iceberg_id_to_orc_type_map; for (uint64_t i = 0; i < orc_type->getSubtypeCount(); ++i) { const auto* orc_sub_type = orc_type->getSubtype(i); @@ -669,15 +703,44 @@ ColumnIdResult IcebergOrcReader::_create_column_ids(const orc::Type* orc_type, IcebergOrcNestedColumnUtils::extract_nested_column_ids); }; + // The Iceberg schema-mapping root is a StructNode whose registered children are the real + // table columns. When present, resolve each column by name through it so the column-id set + // stays consistent with the schema-mapping decision (BY_ID or BY_NAME/name-mapping); + // otherwise fall back to matching by Iceberg field id. + const auto* struct_node = + dynamic_cast(table_info_node.get()); + for (const auto* slot : tuple_descriptor->slots()) { - // 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()); - if (it == iceberg_id_to_orc_type_map.end()) { + const orc::Type* orc_field = nullptr; + if (struct_node != nullptr) { + // Synthesized/metadata slots (e.g. the TopN global row-id or the $row_id column) are + // never registered as children, so check membership before querying: calling + // children_column_exists() on an unregistered name DCHECK-aborts in debug builds and + // throws std::out_of_range from .at() in release builds. + if (struct_node->get_children().contains(slot->col_name()) && + struct_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 = + struct_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) { continue; } - const orc::Type* orc_field = it->second; if ((slot->col_type() != TYPE_STRUCT && slot->col_type() != TYPE_ARRAY && slot->col_type() != TYPE_MAP)) { diff --git a/be/src/format/table/iceberg_reader.h b/be/src/format/table/iceberg_reader.h index 205e46a3a1bf7b..9a237848b522f7 100644 --- a/be/src/format/table/iceberg_reader.h +++ b/be/src/format/table/iceberg_reader.h @@ -105,8 +105,9 @@ class IcebergParquetReader final : public IcebergReaderMixin { this->get_state(), this->_meta_cache); } - static ColumnIdResult _create_column_ids(const FieldDescriptor* field_desc, - const TupleDescriptor* tuple_descriptor); + static ColumnIdResult _create_column_ids( + const FieldDescriptor* field_desc, const TupleDescriptor* tuple_descriptor, + const std::shared_ptr& table_info_node = nullptr); private: Status _read_position_delete_file(const TFileRangeDesc* delete_range, @@ -151,8 +152,9 @@ class IcebergOrcReader final : public IcebergReaderMixin { this->get_io_ctx(), this->_meta_cache); } - static ColumnIdResult _create_column_ids(const orc::Type* orc_type, - const TupleDescriptor* tuple_descriptor); + static ColumnIdResult _create_column_ids( + const orc::Type* orc_type, const TupleDescriptor* tuple_descriptor, + const std::shared_ptr& table_info_node = nullptr); static const std::string ICEBERG_ORC_ATTRIBUTE; diff --git a/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp b/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp index e32153d1ef7f74..9807569cef1ccb 100644 --- a/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp +++ b/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp @@ -28,6 +28,7 @@ #include #include +#include "common/consts.h" #include "common/object_pool.h" #include "core/block/block.h" #include "core/block/column_with_type_and_name.h" @@ -1166,4 +1167,75 @@ TEST_F(IcebergReaderCreateColumnIdsTest, test_create_column_ids_6) { } } +// Regression: a synthesized/metadata slot (e.g. the TopN global row-id column) is projected but +// is never serialized into the Iceberg schema tree, so it is absent from the schema-mapping +// StructNode. Before the get_children().contains() guard, _create_column_ids() called +// StructNode::children_column_exists() on that unregistered name, which hits +// DCHECK(children.contains(name)) and aborts in debug/ASAN builds (and throws std::out_of_range +// from .at() in release builds). These tests core without the guard and pass with it: the +// synthesized slot is skipped and only the real column contributes a column id. +TEST_F(IcebergReaderCreateColumnIdsTest, parquet_synthesized_slot_is_skipped_not_crash) { + // Physical Parquet schema: a single real column "id" (Iceberg field id 1). + FieldDescriptor field_desc; + FieldSchema id_field; + id_field.name = "id"; + id_field.data_type = + DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + id_field.field_id = 1; + field_desc._fields.emplace_back(id_field); + + // Schema-mapping node registers only the real table column "id" (mapped to file column "id"). + // The synthesized global row-id column is intentionally NOT registered as a child. + auto struct_node = std::make_shared(); + struct_node->add_children("id", "id", TableSchemaChangeHelper::ConstNode::get_instance()); + std::shared_ptr table_info_node = struct_node; + + // Projected tuple: the real column plus a synthesized global-row-id slot absent from the node. + SlotDescriptor id_slot; + id_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + id_slot._col_name = "id"; + + SlotDescriptor row_id_slot; + row_id_slot._type = + DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + row_id_slot._col_name = BeConsts::GLOBAL_ROWID_COL; + + TupleDescriptor tuple_desc; + tuple_desc.add_slot(&id_slot); + tuple_desc.add_slot(&row_id_slot); + + const ColumnIdResult result = + IcebergParquetReader::_create_column_ids(&field_desc, &tuple_desc, table_info_node); + EXPECT_EQ(result.column_ids, (std::set {1})); + EXPECT_TRUE(result.filter_column_ids.empty()); +} + +TEST_F(IcebergReaderCreateColumnIdsTest, orc_synthesized_slot_is_skipped_not_crash) { + // Physical ORC schema: a single real column "id" carrying Iceberg field id 1. + std::unique_ptr orc_type(orc::Type::buildTypeFromString("struct")); + orc_type->getSubtype(0)->setAttribute(IcebergOrcReader::ICEBERG_ORC_ATTRIBUTE, "1"); + + auto struct_node = std::make_shared(); + struct_node->add_children("id", "id", TableSchemaChangeHelper::ConstNode::get_instance()); + std::shared_ptr table_info_node = struct_node; + + SlotDescriptor id_slot; + id_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + id_slot._col_name = "id"; + + SlotDescriptor row_id_slot; + row_id_slot._type = + DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + row_id_slot._col_name = BeConsts::GLOBAL_ROWID_COL; + + TupleDescriptor tuple_desc; + tuple_desc.add_slot(&id_slot); + tuple_desc.add_slot(&row_id_slot); + + const ColumnIdResult result = + IcebergOrcReader::_create_column_ids(orc_type.get(), &tuple_desc, table_info_node); + EXPECT_EQ(result.column_ids, (std::set {1})); + EXPECT_TRUE(result.filter_column_ids.empty()); +} + } // namespace doris \ No newline at end of file diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index 3c3541280755de..c9b4ebcbd93553 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -1166,7 +1166,7 @@ public static double getHotValueThreshold() { "FileScanNode 扫描数据的最大并发,默认为 16", "The max threads to read data of FileScanNode, default 16"}) public int maxFileScannersConcurrency = 16; - @VarAttrDef.VarAttr(name = ENABLE_FILE_SCANNER_V2, needForward = true, description = { + @VarAttrDef.VarAttr(name = ENABLE_FILE_SCANNER_V2, needForward = true, fuzzy = true, description = { "开启后 FileScanNode 会在支持的查询场景使用 FileScannerV2,默认开启", "When enabled, FileScanNode uses FileScannerV2 for supported query scans. Enabled by default."}) public boolean enableFileScannerV2 = true; @@ -3991,6 +3991,9 @@ private void setFuzzyForCatalog(Random random) { if (!Config.fuzzy_test_type.equals("external")) { return; } + // file scanner + this.enableFileScannerV2 = random.nextBoolean(); + // parquet this.enableParquetFilterByMinMax = random.nextBoolean(); this.enableParquetFilterByBloomFilter = random.nextBoolean(); From c2ea0ff890fee3d86d9e28a1d8ed9fc479cacf44 Mon Sep 17 00:00:00 2001 From: daidai Date: Wed, 15 Jul 2026 16:26:56 +0800 Subject: [PATCH 4/4] fix review --- .../iceberg_reader_create_column_ids_test.cpp | 85 ++++++++++++------- .../org/apache/doris/qe/SessionVariable.java | 7 +- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp b/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp index 9807569cef1ccb..c15cb8931b7011 100644 --- a/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp +++ b/be/test/format/table/iceberg/iceberg_reader_create_column_ids_test.cpp @@ -1167,33 +1167,46 @@ TEST_F(IcebergReaderCreateColumnIdsTest, test_create_column_ids_6) { } } -// Regression: a synthesized/metadata slot (e.g. the TopN global row-id column) is projected but -// is never serialized into the Iceberg schema tree, so it is absent from the schema-mapping -// StructNode. Before the get_children().contains() guard, _create_column_ids() called -// StructNode::children_column_exists() on that unregistered name, which hits -// DCHECK(children.contains(name)) and aborts in debug/ASAN builds (and throws std::out_of_range -// from .at() in release builds). These tests core without the guard and pass with it: the -// synthesized slot is skipped and only the real column contributes a column id. -TEST_F(IcebergReaderCreateColumnIdsTest, parquet_synthesized_slot_is_skipped_not_crash) { - // Physical Parquet schema: a single real column "id" (Iceberg field id 1). +// Regression coverage for _create_column_ids() driven by the schema-mapping StructNode: +// 1. Synthesized/metadata slots (e.g. the TopN global row-id column) are never serialized into +// the Iceberg schema tree, so they are absent from the node. Before the +// get_children().contains() guard, StructNode::children_column_exists() was called on such an +// unregistered name and hit DCHECK(children.contains(name)) -> abort in debug/ASAN builds +// (and threw std::out_of_range from .at() in release). The projected row-id slot reproduces +// that; with the guard it is skipped instead. +// 2. A projected column must resolve to its physical file column BY NAME through the node, not by +// Iceberg field id and not by its own (table) name, so partial-id / name-mapping files stay +// correct. Table column "a" maps to physical "legacy_a", while an unrelated "stale" column +// carries a field id that collides with the projected slot's default field id (0). The result +// must be "legacy_a"'s column id, never "stale"'s -- a regression to id-only or identity-name +// binding would produce a different set and fail here. +TEST_F(IcebergReaderCreateColumnIdsTest, parquet_name_mapped_and_synthesized_slots) { + // Physical Parquet schema (BY_NAME / partial-id file): + // legacy_a: id-less real data column -> column id 1 + // stale: unrelated column carrying stale id 0 -> column id 2 FieldDescriptor field_desc; - FieldSchema id_field; - id_field.name = "id"; - id_field.data_type = + FieldSchema legacy_a; + legacy_a.name = "legacy_a"; + legacy_a.data_type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); - id_field.field_id = 1; - field_desc._fields.emplace_back(id_field); + legacy_a.field_id = -1; + field_desc._fields.emplace_back(legacy_a); + FieldSchema stale; + stale.name = "stale"; + stale.data_type = + DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + stale.field_id = 0; // collides with the projected slot's default col_unique_id (0) + field_desc._fields.emplace_back(stale); - // Schema-mapping node registers only the real table column "id" (mapped to file column "id"). - // The synthesized global row-id column is intentionally NOT registered as a child. + // Table column "a" resolves BY NAME to physical "legacy_a"; the synthesized global row-id + // column is intentionally NOT registered. auto struct_node = std::make_shared(); - struct_node->add_children("id", "id", TableSchemaChangeHelper::ConstNode::get_instance()); + struct_node->add_children("a", "legacy_a", TableSchemaChangeHelper::ConstNode::get_instance()); std::shared_ptr table_info_node = struct_node; - // Projected tuple: the real column plus a synthesized global-row-id slot absent from the node. - SlotDescriptor id_slot; - id_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); - id_slot._col_name = "id"; + SlotDescriptor a_slot; + a_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + a_slot._col_name = "a"; SlotDescriptor row_id_slot; row_id_slot._type = @@ -1201,27 +1214,33 @@ TEST_F(IcebergReaderCreateColumnIdsTest, parquet_synthesized_slot_is_skipped_not row_id_slot._col_name = BeConsts::GLOBAL_ROWID_COL; TupleDescriptor tuple_desc; - tuple_desc.add_slot(&id_slot); + tuple_desc.add_slot(&a_slot); tuple_desc.add_slot(&row_id_slot); + // No abort on the synthesized slot; "a" resolves BY NAME to "legacy_a" (column id 1), never to + // "stale" (column id 2) through the colliding field id. const ColumnIdResult result = IcebergParquetReader::_create_column_ids(&field_desc, &tuple_desc, table_info_node); EXPECT_EQ(result.column_ids, (std::set {1})); EXPECT_TRUE(result.filter_column_ids.empty()); } -TEST_F(IcebergReaderCreateColumnIdsTest, orc_synthesized_slot_is_skipped_not_crash) { - // Physical ORC schema: a single real column "id" carrying Iceberg field id 1. - std::unique_ptr orc_type(orc::Type::buildTypeFromString("struct")); - orc_type->getSubtype(0)->setAttribute(IcebergOrcReader::ICEBERG_ORC_ATTRIBUTE, "1"); +TEST_F(IcebergReaderCreateColumnIdsTest, orc_name_mapped_and_synthesized_slots) { + // Physical ORC schema (BY_NAME): legacy_a -> column id 1, stale -> column id 2. "stale" carries + // an iceberg.id attribute colliding with the projected slot's default field id (0). + std::unique_ptr orc_type( + orc::Type::buildTypeFromString("struct")); + orc_type->getSubtype(1)->setAttribute(IcebergOrcReader::ICEBERG_ORC_ATTRIBUTE, "0"); + // Table column "a" resolves BY NAME to physical "legacy_a"; the synthesized global row-id + // column is intentionally NOT registered. auto struct_node = std::make_shared(); - struct_node->add_children("id", "id", TableSchemaChangeHelper::ConstNode::get_instance()); + struct_node->add_children("a", "legacy_a", TableSchemaChangeHelper::ConstNode::get_instance()); std::shared_ptr table_info_node = struct_node; - SlotDescriptor id_slot; - id_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); - id_slot._col_name = "id"; + SlotDescriptor a_slot; + a_slot._type = DataTypeFactory::instance().create_data_type(PrimitiveType::TYPE_BIGINT, true); + a_slot._col_name = "a"; SlotDescriptor row_id_slot; row_id_slot._type = @@ -1229,13 +1248,15 @@ TEST_F(IcebergReaderCreateColumnIdsTest, orc_synthesized_slot_is_skipped_not_cra row_id_slot._col_name = BeConsts::GLOBAL_ROWID_COL; TupleDescriptor tuple_desc; - tuple_desc.add_slot(&id_slot); + tuple_desc.add_slot(&a_slot); tuple_desc.add_slot(&row_id_slot); + // No abort on the synthesized slot; "a" resolves BY NAME to "legacy_a" (column id 1), never to + // "stale" (column id 2) through the colliding field id. const ColumnIdResult result = IcebergOrcReader::_create_column_ids(orc_type.get(), &tuple_desc, table_info_node); EXPECT_EQ(result.column_ids, (std::set {1})); EXPECT_TRUE(result.filter_column_ids.empty()); } -} // namespace doris \ No newline at end of file +} // namespace doris diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index c9b4ebcbd93553..aa70a4f6ea6052 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -3841,6 +3841,10 @@ public void initFuzzyModeVariables() { this.enableLocalExchange = random.nextBoolean(); this.enableSharedExchangeSinkBuffer = random.nextBoolean(); this.useSerialExchange = random.nextBoolean(); + // Randomize the external file scanner engine (FileScannerV2 vs the legacy V1 path). Kept + // here rather than in setFuzzyForCatalog() so it also runs in the external regression + // pipeline, which enables fuzzy sessions with fuzzy_test_type=p1 (not "external"). + this.enableFileScannerV2 = random.nextBoolean(); this.disableStreamPreaggregations = random.nextBoolean(); this.enableStreamingAggHashJoinForcePassthrough = random.nextBoolean(); this.enableLocalExchangeBeforeAgg = random.nextBoolean(); @@ -3991,9 +3995,6 @@ private void setFuzzyForCatalog(Random random) { if (!Config.fuzzy_test_type.equals("external")) { return; } - // file scanner - this.enableFileScannerV2 = random.nextBoolean(); - // parquet this.enableParquetFilterByMinMax = random.nextBoolean(); this.enableParquetFilterByBloomFilter = random.nextBoolean();