github-actions[bot] commented on code in PR #66575:
URL: https://github.com/apache/doris/pull/66575#discussion_r3746703168
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java:
##########
@@ -92,21 +92,20 @@ public Void visitLogicalGenerate(LogicalGenerate<? extends
Plan> generate, State
for (Expression child : function.children()) {
exprCollector.collect(child);
}
+ } else if (function.arity() == 1 &&
function.child(0).getDataType().isVariantType()) {
Review Comment:
[P1] Preserve each Variant container in multi-argument explode
This new guard only covers arity one. For a query that reads `col1['k']`
from `explode(v1, v2)`, the multi-argument branch below still propagates `k`
into Variant child `v1`, so Parquet can retain only that shredded leaf even
though the table function must cast the whole `v1` value to an array. If the
array is in root residual bytes, `get_value_ref()` then tries to materialize
the incomplete Variant and fails with `A projected Parquet Variant can only
serve its validated shredded leaves`. Please stop path propagation at each
Variant child in the multi-argument branch too, and add direct/nested/outer
coverage. This is the arity-greater-than-one case not covered by the existing
unary thread.
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -1819,12 +1819,11 @@ static bool build_variant_leaf_path_projection(const
ColumnMapping& mapping,
std::ranges::all_of(value.substr(digits_begin),
[](unsigned char c) { return
std::isdigit(c); });
};
- if (path.size() != 1 || path[0].empty() || path[0] == "NULL" ||
- path[0].find('.') != std::string::npos || is_numeric_selector(path[0])
||
- !mapping.file_local_id.has_value()) {
- // Thrift currently carries access paths as strings without
segment-kind or escaping
- // metadata. Signed numeric tokens are therefore also ambiguous
between an array selector
- // and an object key, so only a single unambiguous key can be mapped
losslessly to a leaf.
+ if (path.empty() || !mapping.file_local_id.has_value() ||
Review Comment:
[P1] Do not leaf-project unfused external Variant paths
This accepts multi-segment legacy paths, but external file scans still
execute `payload['profile']['address']['zip']` as separate binary `element_at`
calls (the fused Variant subpath slots are OLAP-only). The scan can therefore
retain only the terminal `zip` leaf, while the first call asks the shredded
state for the intermediate `profile` object. That projected state cannot return
a STRUCT terminal, falls back to its encoded root, and fails with `A projected
Parquet Variant can only serve its validated shredded leaves`. Please keep
multi-segment external paths on the full projection until execution has a fused
path consumer, or make projected intermediate objects safe, and add an
end-to-end deep-path scan with the leaf counter positive.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -535,14 +711,26 @@ Status plan_parquet_row_groups(const
NativeParquetMetadata& metadata,
std::vector<int> scan_range_selected;
RETURN_IF_ERROR(detail::select_native_row_groups_by_scan_range(
metadata.to_thrift(), scan_range, &row_group_first_rows,
&scan_range_selected));
- std::vector<int> metadata_selected;
- RETURN_IF_ERROR(select_row_groups_by_metadata(
- metadata.to_thrift(), file_schema, request, &scan_range_selected,
&metadata_selected,
- enable_bloom_filter, &plan->pruning_stats, timezone,
runtime_state, file_context,
- column_reader_profile, ParquetMetadataProbeMode::FOOTER_ONLY));
RETURN_IF_ERROR(build_native_row_group_read_plans(metadata, file_schema,
request,
- metadata_selected,
row_group_first_rows, plan,
- timezone, runtime_state,
file_context));
+ scan_range_selected,
row_group_first_rows,
+ plan, timezone,
runtime_state, file_context));
+ std::vector<RowGroupReadPlan> metadata_selected_plans;
+ metadata_selected_plans.reserve(plan->row_groups.size());
+ for (auto& row_group_plan : plan->row_groups) {
Review Comment:
[P2] Avoid rebuilding all requested leaves for every row group
This new loop constructs a complete `unordered_set` of physical leaf IDs for
every candidate row group before the first batch. On a wide non-Variant `SELECT
*` with no conjuncts, `physical_leaf_column_ids_for_row_group()` still walks
every projected leaf, while the footer-only probe cannot prune and never uses
the set; the same set is then rebuilt when each row group reaches the deferred
probe. That makes open/LIMIT latency O(row groups x projected leaves) for
ordinary scans. Please cache the immutable logical leaf set and apply only the
compact per-group fallback delta, keep footer selection batched, or defer this
construction until it is actually needed. This is the all-scan transient cost
introduced by the compact-delta replacement, not the retained Variant plan
copies from the earlier thread.
--
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]
