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


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java:
##########
@@ -230,11 +231,15 @@ public Void visitLogicalGenerate(LogicalGenerate<? 
extends Plan> generate, State
     public Void visitLogicalProject(LogicalProject<? extends Plan> project, 
StatementContext context) {
         AccessPathExpressionCollector exprCollector
                 = new AccessPathExpressionCollector(context, 
allSlotToAccessPaths, false);

Review Comment:
   Recording a root VARIANT path only when the project output is a bare slot 
still misses full-column uses inside expressions. For example, `SELECT cast(v 
AS string) FROM tbl WHERE cast(v['metric'] AS bigint) > 0` reaches this project 
with `Alias(Cast(Slot v))`, so `collectWholeVariantOutput` is not called; 
`AccessPathExpressionCollector.visitCast()` then visits the direct 
`SlotReference v` with an empty builder and records no root `[v]` path, while 
the filter records only `[v, metric]`. The scan can therefore prune to the 
nested shard and evaluate the full `cast(v AS string)` on a partial VARIANT. 
This is distinct from the existing plain `SELECT v` thread because the full 
VARIANT requirement is inside a non-slot expression. Please make direct VARIANT 
slot references in full-value expressions record a root path, with coverage for 
cast/order/filter/group expressions combined with nested VARIANT access.



##########
be/src/format/parquet/parquet_nested_column_utils.cpp:
##########
@@ -0,0 +1,479 @@
+// 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_nested_column_utils.h"
+
+#include <algorithm>
+#include <cctype>
+#include <string_view>
+#include <unordered_map>
+#include <utility>
+
+#include "core/data_type/data_type_nullable.h"
+#include "format/parquet/schema_desc.h"
+
+namespace doris {
+namespace {
+
+enum class NestedPathMode {
+    NAME,
+    FIELD_ID,
+};
+
+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 true;
+    }
+    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 is_variant_array_subscript(std::string_view path) {
+    return !path.empty() &&
+           std::all_of(path.begin(), path.end(), [](unsigned char c) { return 
std::isdigit(c); });
+}
+
+bool is_terminal_variant_meta_component(std::string_view path) {
+    return path == "NULL" || path == "OFFSET";
+}
+
+const std::vector<std::string>& effective_variant_path(const 
std::vector<std::string>& raw_path,
+                                                       
std::vector<std::string>& stripped_path) {
+    if (!raw_path.empty() && 
is_terminal_variant_meta_component(raw_path.back())) {
+        stripped_path.assign(raw_path.begin(), raw_path.end() - 1);
+        return stripped_path;
+    }
+    return raw_path;
+}
+
+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& raw_path : paths) {
+        std::vector<std::string> stripped_path;
+        const auto& path = effective_variant_path(raw_path, stripped_path);
+        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) {
+            const auto typed_value_type = 
remove_nullable(typed_value->data_type);
+            if (typed_value_type->get_primitive_type() != TYPE_STRUCT) {
+                auto child_result =
+                        extract_variant_typed_nested_column_ids(*typed_value, 
{path}, 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;
+                }
+            } else 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& raw_path : paths) {
+        std::vector<std::string> stripped_path;
+        const auto& path = effective_variant_path(raw_path, stripped_path);
+        if (path.empty()) {
+            add_column_id_range(variant_field, column_ids);
+            result.has_child_columns = true;
+            continue;
+        }
+
+        VariantColumnIdExtractionResult typed_result;
+        if (typed_value != nullptr) {
+            const auto typed_value_type = 
remove_nullable(typed_value->data_type);
+            if (typed_value_type->get_primitive_type() != TYPE_STRUCT) {
+                typed_result =
+                        extract_variant_typed_nested_column_ids(*typed_value, 
{path}, column_ids);
+            } else 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) {
+            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;
+        }
+        const auto field_type = 
remove_nullable(field_schema.data_type)->get_primitive_type();
+        const bool is_list = field_type == PrimitiveType::TYPE_ARRAY;
+        const bool is_map = field_type == PrimitiveType::TYPE_MAP;
+        std::vector<std::string> remaining;
+        std::string child_key;
+        if (is_list) {
+            child_key = "*";
+            if (!is_variant_array_subscript(path[0])) {
+                remaining.assign(path.begin(), path.end());
+            } else if (path.size() > 1) {
+                remaining.assign(path.begin() + 1, path.end());
+            }
+        } else if (is_map) {
+            child_paths_by_name["KEYS"].emplace_back();
+            child_key = "VALUES";
+            if (path.size() > 1) {
+                remaining.assign(path.begin() + 1, path.end());
+            }
+        } else {
+            child_key = path[0];
+            if (path.size() > 1) {
+                remaining.assign(path.begin() + 1, path.end());
+            }
+        }
+        child_paths_by_name[child_key].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;

Review Comment:
   This second pass loses the nullable unwrapping done above when grouping 
paths. `parse_list_field()` and `parse_map_field()` wrap optional LIST/MAP 
groups as `Nullable(Array)` / `Nullable(Map)`, so for a valid typed-only 
shredded field like optional `typed_value.items` queried as 
`v['items'][1]['n']`, line 282 groups the path under `"*"`, but this loop sees 
`TYPE_NULLABLE`, uses the physical child name instead of `"*"`, and never 
recurses into the element. The typed leaf is then not selected and typed-only 
files fall back to metadata/value, returning missing data. Please use 
`remove_nullable(field_schema.data_type)` here as well (and in the map check) 
and add coverage for optional typed LIST/MAP shards.



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