github-actions[bot] commented on code in PR #63192:
URL: https://github.com/apache/doris/pull/63192#discussion_r3252759432


##########
be/src/format/parquet/parquet_variant_reader.cpp:
##########
@@ -0,0 +1,558 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "format/parquet/parquet_variant_reader.h"
+
+#include <algorithm>
+#include <cmath>
+#include <cstring>
+#include <iomanip>
+#include <limits>
+#include <sstream>
+#include <string_view>
+#include <vector>
+
+namespace doris::parquet {
+
+namespace {
+
+struct VariantMetadata {
+    std::vector<std::string> dictionary;
+};
+
+uint64_t read_unsigned_le(const uint8_t* ptr, int size) {
+    uint64_t value = 0;
+    for (int i = 0; i < size; ++i) {
+        value |= static_cast<uint64_t>(ptr[i]) << (i * 8);
+    }
+    return value;
+}
+
+int64_t read_signed_le(const uint8_t* ptr, int size) {
+    uint64_t value = read_unsigned_le(ptr, size);
+    if (size < 8) {
+        uint64_t sign_bit = uint64_t {1} << (size * 8 - 1);
+        if ((value & sign_bit) != 0) {
+            uint64_t mask = ~((uint64_t {1} << (size * 8)) - 1);
+            value |= mask;
+        }
+    }
+    return static_cast<int64_t>(value);
+}
+
+__int128 read_signed_int128_le(const uint8_t* ptr) {
+    unsigned __int128 unsigned_value = 0;
+    for (int i = 15; i >= 0; --i) {
+        unsigned_value <<= 8;
+        unsigned_value |= ptr[i];
+    }
+    static constexpr unsigned __int128 sign_bit = static_cast<unsigned 
__int128>(1) << 127;
+    if ((unsigned_value & sign_bit) == 0) {
+        return static_cast<__int128>(unsigned_value);
+    }
+    static constexpr __int128 signed_half_range = static_cast<__int128>(1) << 
126;
+    return (static_cast<__int128>(unsigned_value & (sign_bit - 1)) - 
signed_half_range) -
+           signed_half_range;
+}
+
+Status require_available(const uint8_t* ptr, const uint8_t* end, size_t size,
+                         std::string_view context) {
+    if (ptr > end || size > static_cast<size_t>(end - ptr)) {
+        return Status::Corruption("Invalid Parquet VARIANT {} encoding", 
context);
+    }
+    return Status::OK();
+}
+
+Status require_available_entries(const uint8_t* ptr, const uint8_t* end, 
uint64_t entries,
+                                 size_t entry_size, std::string_view context) {
+    if (entries > std::numeric_limits<size_t>::max() / entry_size) {
+        return Status::Corruption("Invalid Parquet VARIANT {} encoding", 
context);
+    }
+    return require_available(ptr, end, static_cast<size_t>(entries) * 
entry_size, context);
+}
+
+Status validate_array_field_offsets(const std::vector<uint64_t>& 
field_offsets, uint64_t total_size,
+                                    std::string_view context) {
+    if (field_offsets.empty() || field_offsets.front() != 0) {
+        return Status::Corruption("Invalid Parquet VARIANT {} field offsets", 
context);
+    }
+    for (size_t i = 0; i < field_offsets.size(); ++i) {
+        if (field_offsets[i] > total_size) {
+            return Status::Corruption("Invalid Parquet VARIANT {} field offset 
{}", context,
+                                      field_offsets[i]);
+        }
+        if (i > 0 && field_offsets[i] < field_offsets[i - 1]) {
+            return Status::Corruption("Invalid Parquet VARIANT {} field 
offsets", context);
+        }
+    }
+    return Status::OK();
+}
+
+Status compute_object_field_ends(const std::vector<uint64_t>& field_offsets, 
uint64_t total_size,
+                                 std::vector<uint64_t>* field_ends) {
+    if (field_offsets.empty()) {
+        return Status::Corruption("Invalid Parquet VARIANT object field 
offsets");
+    }
+    size_t num_elements = field_offsets.size() - 1;
+    if (num_elements == 0) {
+        if (total_size != 0) {
+            return Status::Corruption("Invalid Parquet VARIANT object field 
offsets");
+        }
+        return Status::OK();
+    }
+
+    std::vector<std::pair<uint64_t, size_t>> physical_offsets;
+    physical_offsets.reserve(num_elements);
+    for (size_t i = 0; i < num_elements; ++i) {
+        if (field_offsets[i] >= total_size) {
+            return Status::Corruption("Invalid Parquet VARIANT object field 
offset {}",
+                                      field_offsets[i]);
+        }
+        physical_offsets.emplace_back(field_offsets[i], i);
+    }
+    std::sort(physical_offsets.begin(), physical_offsets.end());
+    if (physical_offsets.front().first != 0) {
+        return Status::Corruption("Invalid Parquet VARIANT object field 
offsets");
+    }
+
+    field_ends->assign(num_elements, 0);
+    for (size_t i = 0; i < physical_offsets.size(); ++i) {
+        if (i > 0 && physical_offsets[i].first == physical_offsets[i - 
1].first) {
+            return Status::Corruption("Invalid Parquet VARIANT object field 
offsets");
+        }
+        uint64_t child_end =
+                i + 1 < physical_offsets.size() ? physical_offsets[i + 
1].first : total_size;
+        (*field_ends)[physical_offsets[i].second] = child_end;
+    }
+    return Status::OK();
+}
+
+void append_json_string(std::string_view value, std::string* json) {
+    json->push_back('"');
+    static constexpr char hex[] = "0123456789abcdef";
+    for (unsigned char c : value) {
+        switch (c) {
+        case '"':
+            json->append("\\\"");
+            break;
+        case '\\':
+            json->append("\\\\");
+            break;
+        case '\b':
+            json->append("\\b");
+            break;
+        case '\f':
+            json->append("\\f");
+            break;
+        case '\n':
+            json->append("\\n");
+            break;
+        case '\r':
+            json->append("\\r");
+            break;
+        case '\t':
+            json->append("\\t");
+            break;
+        default:
+            if (c < 0x20) {
+                json->append("\\u00");
+                json->push_back(hex[c >> 4]);
+                json->push_back(hex[c & 0x0f]);
+            } else {
+                json->push_back(static_cast<char>(c));
+            }
+            break;
+        }
+    }
+    json->push_back('"');
+}
+
+template <typename T>
+Status append_floating_json(T value, std::string* json) {
+    if (!std::isfinite(value)) {
+        return Status::NotSupported(
+                "Parquet VARIANT non-finite floating point value is not 
supported");
+    }
+    std::ostringstream oss;
+    oss << std::setprecision(std::numeric_limits<T>::max_digits10) << value;
+    json->append(oss.str());
+    return Status::OK();
+}
+
+std::string int128_to_string(__int128 value) {
+    if (value == 0) {
+        return "0";
+    }
+    bool negative = value < 0;
+    unsigned __int128 unsigned_value = negative ? static_cast<unsigned 
__int128>(-(value + 1)) + 1
+                                                : static_cast<unsigned 
__int128>(value);
+    std::string digits;
+    while (unsigned_value > 0) {
+        digits.push_back(static_cast<char>('0' + unsigned_value % 10));
+        unsigned_value /= 10;
+    }
+    if (negative) {
+        digits.push_back('-');
+    }
+    std::reverse(digits.begin(), digits.end());
+    return digits;
+}
+
+void append_decimal_json(__int128 unscaled, int scale, std::string* json) {
+    std::string value = int128_to_string(unscaled);
+    bool negative = !value.empty() && value[0] == '-';
+    std::string digits = negative ? value.substr(1) : value;
+    if (scale == 0) {
+        json->append(value);
+        return;
+    }
+    if (scale > 0) {
+        if (digits.size() <= static_cast<size_t>(scale)) {
+            digits.insert(0, static_cast<size_t>(scale) + 1 - digits.size(), 
'0');
+        }
+        digits.insert(digits.end() - scale, '.');
+        if (negative) {
+            json->push_back('-');
+        }
+        json->append(digits);
+        return;
+    }
+    if (negative) {
+        json->push_back('-');
+    }
+    json->append(digits);
+    json->append(static_cast<size_t>(-scale), '0');
+}
+
+void append_uuid_json(const uint8_t* ptr, std::string* json) {
+    static constexpr char hex[] = "0123456789abcdef";
+    json->push_back('"');
+    for (int i = 0; i < 16; ++i) {
+        if (i == 4 || i == 6 || i == 8 || i == 10) {
+            json->push_back('-');
+        }
+        json->push_back(hex[ptr[i] >> 4]);
+        json->push_back(hex[ptr[i] & 0x0f]);
+    }
+    json->push_back('"');
+}
+
+Status decode_metadata(const StringRef& metadata, VariantMetadata* result) {
+    const auto* ptr = reinterpret_cast<const uint8_t*>(metadata.data);
+    const auto* end = ptr + metadata.size;
+    RETURN_IF_ERROR(require_available(ptr, end, 1, "metadata"));
+    uint8_t header = *ptr++;
+    uint8_t version = header & 0x0f;
+    if (version != 1) {
+        return Status::Corruption("Unsupported Parquet VARIANT metadata 
version {}", version);
+    }
+    const bool sorted_strings = (header & 0x10) != 0;
+    int offset_size = ((header >> 6) & 0x03) + 1;
+    RETURN_IF_ERROR(require_available(ptr, end, offset_size, "metadata 
dictionary size"));
+    uint64_t dictionary_size = read_unsigned_le(ptr, offset_size);
+    ptr += offset_size;
+
+    RETURN_IF_ERROR(require_available_entries(ptr, end, dictionary_size + 1, 
offset_size,
+                                              "metadata dictionary offsets"));
+    std::vector<uint64_t> offsets(dictionary_size + 1);
+    for (uint64_t i = 0; i <= dictionary_size; ++i) {
+        offsets[i] = read_unsigned_le(ptr, offset_size);
+        ptr += offset_size;
+        if (i > 0 && offsets[i] < offsets[i - 1]) {
+            return Status::Corruption("Invalid Parquet VARIANT metadata 
dictionary offsets");
+        }
+    }
+
+    RETURN_IF_ERROR(require_available(ptr, end, offsets.back(), "metadata 
dictionary bytes"));

Review Comment:
   `decode_metadata()` validates that the dictionary bytes are available, but 
it never verifies that the metadata buffer ends after `offsets.back()` bytes. A 
malformed metadata blob with a valid header/offset table/dictionary plus 
trailing garbage is accepted and used to decode values, unlike 
`decode_variant_to_json()` which rejects trailing bytes in the value buffer. 
Please reject `ptr + offsets.back() != end` and add a malformed-metadata unit 
case.



##########
be/src/format/table/hive/hive_parquet_nested_column_utils.cpp:
##########
@@ -18,20 +18,294 @@
 #include "format/table/hive/hive_parquet_nested_column_utils.h"
 
 #include <algorithm>
+#include <cctype>
 #include <memory>
 #include <set>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <vector>
 
+#include "core/data_type/data_type_nullable.h"
 #include "format/parquet/schema_desc.h"
 #include "format/table/table_schema_change_helper.h"
 
 namespace doris {
+namespace {
+
+void add_column_id_range(const FieldSchema& field_schema, std::set<uint64_t>& 
column_ids) {
+    const uint64_t start_id = field_schema.get_column_id();
+    const uint64_t max_column_id = field_schema.get_max_column_id();
+    for (uint64_t id = start_id; id <= max_column_id; ++id) {
+        column_ids.insert(id);
+    }
+}
+
+const FieldSchema* find_child_by_structural_name(const FieldSchema& 
field_schema,
+                                                 std::string_view name) {
+    std::string lower_name(name);
+    std::transform(lower_name.begin(), lower_name.end(), lower_name.begin(),
+                   [](unsigned char c) { return 
static_cast<char>(std::tolower(c)); });
+    for (const auto& child : field_schema.children) {
+        if (child.name == name || child.lower_case_name == lower_name) {
+            return &child;
+        }
+    }
+    return nullptr;
+}
+
+const FieldSchema* find_child_by_exact_name(const FieldSchema& field_schema,
+                                            std::string_view name) {
+    for (const auto& child : field_schema.children) {
+        if (child.name == name) {
+            return &child;
+        }
+    }
+    return nullptr;
+}
+
+void add_variant_metadata(const FieldSchema& variant_field, 
std::set<uint64_t>& column_ids) {
+    if (const auto* metadata = find_child_by_structural_name(variant_field, 
"metadata")) {
+        add_column_id_range(*metadata, column_ids);
+    }
+}
+
+void add_variant_value(const FieldSchema& variant_field, std::set<uint64_t>& 
column_ids) {
+    add_variant_metadata(variant_field, column_ids);
+    if (const auto* value = find_child_by_structural_name(variant_field, 
"value")) {
+        add_column_id_range(*value, column_ids);
+    }
+}
+
+struct VariantColumnIdExtractionResult {
+    bool has_child_columns = false;
+    bool needs_metadata = false;
+};
+
+bool is_shredded_variant_field(const FieldSchema& field_schema) {
+    bool has_value = false;
+    const FieldSchema* typed_value = nullptr;
+    for (const auto& child : field_schema.children) {
+        if (child.lower_case_name == "value") {
+            if (child.physical_type != tparquet::Type::BYTE_ARRAY) {
+                return false;
+            }
+            has_value = true;
+            continue;
+        }
+        if (child.lower_case_name == "typed_value") {
+            typed_value = &child;
+            continue;
+        }
+        return false;
+    }
+    if (has_value) {
+        return typed_value != nullptr;
+    }
+    if (typed_value == nullptr) {
+        return false;
+    }
+    const auto type = remove_nullable(typed_value->data_type);
+    return type->get_primitive_type() == TYPE_STRUCT || 
type->get_primitive_type() == TYPE_ARRAY;
+}
+
+bool add_shredded_variant_field_value(const FieldSchema& shredded_field,
+                                      std::set<uint64_t>& column_ids) {
+    if (const auto* value = find_child_by_structural_name(shredded_field, 
"value")) {
+        add_column_id_range(*value, column_ids);
+        return true;
+    }
+    return false;
+}
+
+bool contains_inherited_metadata_value(const FieldSchema& field_schema) {
+    if (is_shredded_variant_field(field_schema) &&
+        find_child_by_structural_name(field_schema, "value") != nullptr) {
+        return true;
+    }
+    return std::any_of(
+            field_schema.children.begin(), field_schema.children.end(),
+            [](const FieldSchema& child) { return 
contains_inherited_metadata_value(child); });
+}
+
+VariantColumnIdExtractionResult extract_variant_typed_nested_column_ids(
+        const FieldSchema& field_schema, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids);
+
+VariantColumnIdExtractionResult extract_shredded_variant_field_ids(
+        const FieldSchema& shredded_field, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    const auto* typed_value = find_child_by_structural_name(shredded_field, 
"typed_value");
+    VariantColumnIdExtractionResult result;
+
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(shredded_field, column_ids);
+            result.has_child_columns = true;
+            result.needs_metadata |= 
contains_inherited_metadata_value(shredded_field);
+            continue;
+        }
+
+        bool has_selected_columns = 
add_shredded_variant_field_value(shredded_field, column_ids);
+        result.needs_metadata |= has_selected_columns;
+        if (typed_value != nullptr) {
+            if (const auto* typed_child = 
find_child_by_exact_name(*typed_value, path[0])) {
+                if (path.size() == 1) {
+                    add_column_id_range(*typed_child, column_ids);
+                    result.needs_metadata |= 
contains_inherited_metadata_value(*typed_child);
+                    column_ids.insert(typed_value->get_column_id());
+                    has_selected_columns = true;
+                } else {
+                    std::vector<std::vector<std::string>> child_paths {
+                            std::vector<std::string>(path.begin() + 1, 
path.end())};
+                    auto child_result = 
extract_variant_typed_nested_column_ids(
+                            *typed_child, child_paths, column_ids);
+                    if (child_result.has_child_columns) {
+                        column_ids.insert(typed_value->get_column_id());
+                        result.needs_metadata |= child_result.needs_metadata;
+                        has_selected_columns = true;
+                    }
+                }
+            }
+        }
+        result.has_child_columns |= has_selected_columns;
+    }
+
+    if (result.has_child_columns) {
+        column_ids.insert(shredded_field.get_column_id());
+    }
+    return result;
+}
+
+VariantColumnIdExtractionResult extract_variant_nested_column_ids(
+        const FieldSchema& variant_field, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    const auto* typed_value = find_child_by_structural_name(variant_field, 
"typed_value");
+    VariantColumnIdExtractionResult result;
+
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(variant_field, column_ids);
+            result.has_child_columns = true;
+            continue;
+        }
+
+        VariantColumnIdExtractionResult typed_result;
+        if (typed_value != nullptr) {
+            if (const auto* typed_child = 
find_child_by_exact_name(*typed_value, path[0])) {
+                if (path.size() == 1) {
+                    add_column_id_range(*typed_child, column_ids);
+                    typed_result.has_child_columns = true;
+                    typed_result.needs_metadata = 
contains_inherited_metadata_value(*typed_child);
+                } else {
+                    std::vector<std::vector<std::string>> child_paths {
+                            std::vector<std::string>(path.begin() + 1, 
path.end())};
+                    typed_result = 
extract_variant_typed_nested_column_ids(*typed_child,
+                                                                           
child_paths, column_ids);
+                }
+                if (typed_result.has_child_columns) {
+                    column_ids.insert(typed_value->get_column_id());
+                    if (typed_result.needs_metadata) {
+                        add_variant_metadata(variant_field, column_ids);
+                    }
+                }
+            }
+        }
+
+        if (!typed_result.has_child_columns) {
+            // For shredded object fields, the selected field's residual 
fallback is
+            // typed_value.<field>.value. The top-level value stores only 
fields that
+            // are not represented in typed_value, so it is only needed when 
the
+            // selected path is not shredded.
+            add_variant_value(variant_field, column_ids);
+        }
+        result.has_child_columns = true;
+    }
+
+    if (result.has_child_columns) {
+        column_ids.insert(variant_field.get_column_id());
+    }
+    return result;
+}
+
+VariantColumnIdExtractionResult extract_variant_typed_nested_column_ids(
+        const FieldSchema& field_schema, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    if (field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_VARIANT) {
+        return extract_variant_nested_column_ids(field_schema, paths, 
column_ids);
+    }
+    if (is_shredded_variant_field(field_schema)) {
+        return extract_shredded_variant_field_ids(field_schema, paths, 
column_ids);
+    }
+
+    VariantColumnIdExtractionResult result;
+    std::unordered_map<std::string, std::vector<std::vector<std::string>>> 
child_paths_by_name;
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(field_schema, column_ids);
+            result.has_child_columns = true;
+            result.needs_metadata |= 
contains_inherited_metadata_value(field_schema);
+            continue;
+        }
+        std::vector<std::string> remaining;
+        if (path.size() > 1) {
+            remaining.assign(path.begin() + 1, path.end());
+        }
+        child_paths_by_name[path[0]].push_back(std::move(remaining));
+    }
+
+    for (uint64_t i = 0; i < field_schema.children.size(); ++i) {
+        const auto& child = field_schema.children[i];
+        std::string child_name;
+
+        const bool is_list =
+                field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_ARRAY;
+        const bool is_map = field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_MAP;
+        if (is_list) {
+            child_name = "*";
+        } else if (is_map) {
+            child_name = i == 0 ? "KEYS" : "VALUES";
+        } else {
+            child_name = child.name;
+        }
+
+        auto child_paths_it = child_paths_by_name.find(child_name);

Review Comment:
   VARIANT array element subpaths do not match the typed LIST child here. FE 
records integer VARIANT subscripts literally, so `v['items'][1]['n']` reaches 
this helper as a path like `[items, 1, n]`; after entering the shredded `items` 
field, `child_paths_by_name` is keyed by `"1"`, but array traversal below only 
looks for the synthetic `"*"` child name. No typed leaf under 
`items.typed_value.list.element...` is selected, and the caller falls back to 
top-level `v.value`, which is absent for typed-only array shredding. This can 
return missing/null for a value stored only in the typed array shard. Please 
normalize VARIANT integer array subscripts to the array wildcard before 
recursing, and add coverage for a subpath inside a shredded VARIANT array 
element.



##########
be/src/format/parquet/schema_desc.cpp:
##########
@@ -446,6 +463,36 @@ Status FieldDescriptor::parse_group_field(const 
std::vector<tparquet::SchemaElem
     return Status::OK();
 }
 
+Status FieldDescriptor::parse_variant_field(const 
std::vector<tparquet::SchemaElement>& t_schemas,
+                                            size_t curr_pos, FieldSchema* 
variant_field) {
+    RETURN_IF_ERROR(parse_struct_field(t_schemas, curr_pos, variant_field));
+
+    bool has_metadata = false;
+    bool metadata_required = false;
+    bool has_value = false;
+    for (const auto& child : variant_field->children) {
+        if (child.lower_case_name == "metadata") {
+            has_metadata = child.physical_type == tparquet::Type::BYTE_ARRAY;
+            metadata_required = !child.data_type->is_nullable();

Review Comment:
   This makes the schema parser reject typed-only shredded VARIANT files where 
the top-level `value` field is omitted. The shredding spec defines `value` and 
`typed_value` as optional fields used together, and this PR's reader/test 
helpers already handle layouts like `required group v (VARIANT) { required 
binary metadata; optional group typed_value { optional int64 metric; } }`. With 
the current `!has_value` check, such a file fails during schema parsing before 
`VariantColumnReader` can use the typed-only path, so `SELECT v` or 
`v['metric']` cannot read a valid typed-only VARIANT file. Please allow 
`metadata + typed_value` without top-level `value` and add schema/parser 
coverage for that layout.



##########
be/src/format/table/iceberg/iceberg_parquet_nested_column_utils.cpp:
##########
@@ -18,21 +18,295 @@
 #include "format/table/iceberg/iceberg_parquet_nested_column_utils.h"
 
 #include <algorithm>
+#include <cctype>
 #include <iostream>
 #include <memory>
 #include <set>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <vector>
 
+#include "core/data_type/data_type_nullable.h"
 #include "format/parquet/schema_desc.h"
 #include "format/table/table_schema_change_helper.h"
 
 namespace doris {
+namespace {
+
+void add_column_id_range(const FieldSchema& field_schema, std::set<uint64_t>& 
column_ids) {
+    const uint64_t start_id = field_schema.get_column_id();
+    const uint64_t max_column_id = field_schema.get_max_column_id();
+    for (uint64_t id = start_id; id <= max_column_id; ++id) {
+        column_ids.insert(id);
+    }
+}
+
+const FieldSchema* find_child_by_structural_name(const FieldSchema& 
field_schema,
+                                                 std::string_view name) {
+    std::string lower_name(name);
+    std::transform(lower_name.begin(), lower_name.end(), lower_name.begin(),
+                   [](unsigned char c) { return 
static_cast<char>(std::tolower(c)); });
+    for (const auto& child : field_schema.children) {
+        if (child.name == name || child.lower_case_name == lower_name) {
+            return &child;
+        }
+    }
+    return nullptr;
+}
+
+const FieldSchema* find_child_by_exact_name(const FieldSchema& field_schema,
+                                            std::string_view name) {
+    for (const auto& child : field_schema.children) {
+        if (child.name == name) {
+            return &child;
+        }
+    }
+    return nullptr;
+}
+
+void add_variant_metadata(const FieldSchema& variant_field, 
std::set<uint64_t>& column_ids) {
+    if (const auto* metadata = find_child_by_structural_name(variant_field, 
"metadata")) {
+        add_column_id_range(*metadata, column_ids);
+    }
+}
+
+void add_variant_value(const FieldSchema& variant_field, std::set<uint64_t>& 
column_ids) {
+    add_variant_metadata(variant_field, column_ids);
+    if (const auto* value = find_child_by_structural_name(variant_field, 
"value")) {
+        add_column_id_range(*value, column_ids);
+    }
+}
+
+struct VariantColumnIdExtractionResult {
+    bool has_child_columns = false;
+    bool needs_metadata = false;
+};
+
+bool is_shredded_variant_field(const FieldSchema& field_schema) {
+    bool has_value = false;
+    const FieldSchema* typed_value = nullptr;
+    for (const auto& child : field_schema.children) {
+        if (child.lower_case_name == "value") {
+            if (child.physical_type != tparquet::Type::BYTE_ARRAY) {
+                return false;
+            }
+            has_value = true;
+            continue;
+        }
+        if (child.lower_case_name == "typed_value") {
+            typed_value = &child;
+            continue;
+        }
+        return false;
+    }
+    if (has_value) {
+        return typed_value != nullptr;
+    }
+    if (typed_value == nullptr) {
+        return false;
+    }
+    const auto type = remove_nullable(typed_value->data_type);
+    return type->get_primitive_type() == TYPE_STRUCT || 
type->get_primitive_type() == TYPE_ARRAY;
+}
+
+bool add_shredded_variant_field_value(const FieldSchema& shredded_field,
+                                      std::set<uint64_t>& column_ids) {
+    if (const auto* value = find_child_by_structural_name(shredded_field, 
"value")) {
+        add_column_id_range(*value, column_ids);
+        return true;
+    }
+    return false;
+}
+
+bool contains_inherited_metadata_value(const FieldSchema& field_schema) {
+    if (is_shredded_variant_field(field_schema) &&
+        find_child_by_structural_name(field_schema, "value") != nullptr) {
+        return true;
+    }
+    return std::any_of(
+            field_schema.children.begin(), field_schema.children.end(),
+            [](const FieldSchema& child) { return 
contains_inherited_metadata_value(child); });
+}
+
+VariantColumnIdExtractionResult extract_variant_typed_nested_column_ids(
+        const FieldSchema& field_schema, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids);
+
+VariantColumnIdExtractionResult extract_shredded_variant_field_ids(
+        const FieldSchema& shredded_field, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    const auto* typed_value = find_child_by_structural_name(shredded_field, 
"typed_value");
+    VariantColumnIdExtractionResult result;
+
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(shredded_field, column_ids);
+            result.has_child_columns = true;
+            result.needs_metadata |= 
contains_inherited_metadata_value(shredded_field);
+            continue;
+        }
+
+        bool has_selected_columns = 
add_shredded_variant_field_value(shredded_field, column_ids);
+        result.needs_metadata |= has_selected_columns;
+        if (typed_value != nullptr) {
+            if (const auto* typed_child = 
find_child_by_exact_name(*typed_value, path[0])) {
+                if (path.size() == 1) {
+                    add_column_id_range(*typed_child, column_ids);
+                    result.needs_metadata |= 
contains_inherited_metadata_value(*typed_child);
+                    column_ids.insert(typed_value->get_column_id());
+                    has_selected_columns = true;
+                } else {
+                    std::vector<std::vector<std::string>> child_paths {
+                            std::vector<std::string>(path.begin() + 1, 
path.end())};
+                    auto child_result = 
extract_variant_typed_nested_column_ids(
+                            *typed_child, child_paths, column_ids);
+                    if (child_result.has_child_columns) {
+                        column_ids.insert(typed_value->get_column_id());
+                        result.needs_metadata |= child_result.needs_metadata;
+                        has_selected_columns = true;
+                    }
+                }
+            }
+        }
+        result.has_child_columns |= has_selected_columns;
+    }
+
+    if (result.has_child_columns) {
+        column_ids.insert(shredded_field.get_column_id());
+    }
+    return result;
+}
+
+VariantColumnIdExtractionResult extract_variant_nested_column_ids(
+        const FieldSchema& variant_field, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    const auto* typed_value = find_child_by_structural_name(variant_field, 
"typed_value");
+    VariantColumnIdExtractionResult result;
+
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(variant_field, column_ids);
+            result.has_child_columns = true;
+            continue;
+        }
+
+        VariantColumnIdExtractionResult typed_result;
+        if (typed_value != nullptr) {
+            if (const auto* typed_child = 
find_child_by_exact_name(*typed_value, path[0])) {
+                if (path.size() == 1) {
+                    add_column_id_range(*typed_child, column_ids);
+                    typed_result.has_child_columns = true;
+                    typed_result.needs_metadata = 
contains_inherited_metadata_value(*typed_child);
+                } else {
+                    std::vector<std::vector<std::string>> child_paths {
+                            std::vector<std::string>(path.begin() + 1, 
path.end())};
+                    typed_result = 
extract_variant_typed_nested_column_ids(*typed_child,
+                                                                           
child_paths, column_ids);
+                }
+                if (typed_result.has_child_columns) {
+                    column_ids.insert(typed_value->get_column_id());
+                    if (typed_result.needs_metadata) {
+                        add_variant_metadata(variant_field, column_ids);
+                    }
+                }
+            }
+        }
+
+        if (!typed_result.has_child_columns) {
+            // For shredded object fields, the selected field's residual 
fallback is
+            // typed_value.<field>.value. The top-level value stores only 
fields that
+            // are not represented in typed_value, so it is only needed when 
the
+            // selected path is not shredded.
+            add_variant_value(variant_field, column_ids);
+        }
+        result.has_child_columns = true;
+    }
+
+    if (result.has_child_columns) {
+        column_ids.insert(variant_field.get_column_id());
+    }
+    return result;
+}
+
+VariantColumnIdExtractionResult extract_variant_typed_nested_column_ids(
+        const FieldSchema& field_schema, const 
std::vector<std::vector<std::string>>& paths,
+        std::set<uint64_t>& column_ids) {
+    if (field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_VARIANT) {
+        return extract_variant_nested_column_ids(field_schema, paths, 
column_ids);
+    }
+    if (is_shredded_variant_field(field_schema)) {
+        return extract_shredded_variant_field_ids(field_schema, paths, 
column_ids);
+    }
+
+    VariantColumnIdExtractionResult result;
+    std::unordered_map<std::string, std::vector<std::vector<std::string>>> 
child_paths_by_name;
+    for (const auto& path : paths) {
+        if (path.empty()) {
+            add_column_id_range(field_schema, column_ids);
+            result.has_child_columns = true;
+            result.needs_metadata |= 
contains_inherited_metadata_value(field_schema);
+            continue;
+        }
+        std::vector<std::string> remaining;
+        if (path.size() > 1) {
+            remaining.assign(path.begin() + 1, path.end());
+        }
+        child_paths_by_name[path[0]].push_back(std::move(remaining));
+    }
+
+    for (uint64_t i = 0; i < field_schema.children.size(); ++i) {
+        const auto& child = field_schema.children[i];
+        std::string child_name;
+
+        const bool is_list =
+                field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_ARRAY;
+        const bool is_map = field_schema.data_type->get_primitive_type() == 
PrimitiveType::TYPE_MAP;
+        if (is_list) {
+            child_name = "*";
+        } else if (is_map) {
+            child_name = i == 0 ? "KEYS" : "VALUES";
+        } else {
+            child_name = child.name;
+        }
+
+        auto child_paths_it = child_paths_by_name.find(child_name);

Review Comment:
   The Iceberg duplicate has the same VARIANT array element pruning gap as the 
Hive/local helper. `SlotTypeReplacer` intentionally leaves VARIANT subpath 
components as data keys, so a query like `v['items'][1]['n']` reaches this 
typed-array recursion with `"1"` rather than the array wildcard; the code below 
only matches `"*"`, selects no typed array leaf, and then falls back to 
top-level `v.value`. For typed-only Iceberg VARIANT array shredding, that 
silently misses data stored under 
`v.typed_value.items.typed_value.list.element...`. Please apply the same 
integer-subscript-to-wildcard handling here and cover the Iceberg path.



-- 
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]


Reply via email to