github-actions[bot] commented on code in PR #66575:
URL: https://github.com/apache/doris/pull/66575#discussion_r3746370021
##########
be/src/format_v2/parquet/parquet_reader.cpp:
##########
@@ -72,104 +72,143 @@ const ParquetColumnSchema* schema_child_by_name(const
ParquetColumnSchema& schem
return child_it == schema.children.end() ? nullptr : child_it->get();
}
-bool collect_variant_residual_leaf_ids(const ParquetColumnSchema& schema,
- const format::LocalColumnIndex&
projection,
- std::vector<int>* residual_leaf_ids) {
+bool collect_variant_terminal_fallback_leaf_ids(const ParquetColumnSchema&
schema,
+ const
format::LocalColumnIndex& projection,
+ std::vector<int>*
residual_leaf_ids) {
DORIS_CHECK(residual_leaf_ids != nullptr);
const auto* value = schema_child_by_name(schema, "value");
const auto* typed_value = schema_child_by_name(schema, "typed_value");
- if (value != nullptr && typed_value != nullptr) {
- if (value->kind != ParquetColumnSchemaKind::PRIMITIVE ||
value->leaf_column_id < 0) {
+ if (value != nullptr && value->kind == ParquetColumnSchemaKind::PRIMITIVE
&&
+ typed_value != nullptr) {
+ if (value->leaf_column_id < 0) {
return false;
}
+ const auto typed_projection_it =
+ std::ranges::find_if(projection.children, [&](const auto&
child_projection) {
+ return child_projection.local_id() ==
typed_value->local_id;
+ });
+ if (typed_projection_it == projection.children.end()) {
+ return false;
+ }
+ if (typed_value->kind != ParquetColumnSchemaKind::PRIMITIVE) {
+ return collect_variant_terminal_fallback_leaf_ids(*typed_value,
*typed_projection_it,
+
residual_leaf_ids);
+ }
+ // Object residual keys are disjoint from shredded keys. Only the
fallback paired with the
Review Comment:
[P1] Preserve the root fallback for paths propagated through explode
`AccessPathPlanCollector` encodes `select x['k'] ... explode(v)` as the
legacy path `v/k`, so the scan no longer knows that the generator must consume
`v` itself as an array. A valid heterogeneous Variant row group can have object
`typed_value.k` in its shared schema while array-valued rows live in the root
residual `value`; `k.value` is all NULL for those rows, so this terminal-only
check accepts the partial leaf and drops the arrays. `explode` then
casts/materializes the whole input Variant as ARRAY, but a partial Parquet
Variant cannot materialize its root. Please preserve container provenance (or
force full projection for generator-derived paths) and cover a mixed row group
with root residual arrays plus shredded object `k`.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -252,6 +252,75 @@ std::vector<format::LocalColumnIndex>
physical_non_predicate_columns(
return columns;
}
+struct VariantRowGroupProjectionCounts {
+ size_t leaf = 0;
+ size_t full = 0;
+};
+
+VariantRowGroupProjectionCounts finalize_variant_projections_for_row_group(
+ const tparquet::RowGroup& row_group,
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ std::vector<format::LocalColumnIndex>* projections) {
+ DORIS_CHECK(projections != nullptr);
+ VariantRowGroupProjectionCounts counts;
+ for (auto& projection : *projections) {
+ const int32_t local_id = projection.local_id();
+ if (local_id < 0 || local_id >=
static_cast<int32_t>(file_schema.size()) ||
+ !file_schema[local_id]->contains_variant) {
+ continue;
+ }
+ counts.leaf += detail::finalize_variant_leaf_projection_for_row_group(
+ row_group, *file_schema[local_id], &projection, &counts.full);
+ }
+ return counts;
+}
+
+void prepare_row_group_physical_projection(
+ const tparquet::RowGroup& row_group,
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request, RowGroupReadPlan*
row_group_plan) {
+ DORIS_CHECK(row_group_plan != nullptr);
+ const auto may_project_variant_leaf = [&](const auto& projections) {
+ return std::ranges::any_of(projections, [&](const auto& projection) {
+ const int32_t local_id = projection.local_id();
+ return local_id >= 0 && local_id <
static_cast<int32_t>(file_schema.size()) &&
+ file_schema[local_id] != nullptr &&
file_schema[local_id]->contains_variant &&
+ format::is_partial_projection(&projection);
+ });
+ };
+ if (!may_project_variant_leaf(request.predicate_columns) &&
+ !may_project_variant_leaf(request.non_predicate_columns)) {
+ row_group_plan->physical_predicate_columns = {};
+ row_group_plan->physical_non_predicate_columns = {};
+ row_group_plan->has_row_group_physical_projection = false;
+ row_group_plan->variant_leaf_projection_columns = 0;
+ row_group_plan->variant_full_projection_columns = 0;
+ return;
+ }
+ row_group_plan->physical_predicate_columns = request.predicate_columns;
Review Comment:
[P2] Avoid copying the complete projection into every row-group plan
As soon as any partial top-level projection belongs to a schema containing
Variant, this copies every predicate and output `LocalColumnIndex` tree into
every candidate row group. `physical_request_for_row_group()` then clones the
full `FileScanRequest` again for footer/deferred probes, and
`ParquetReader::open()` retains each surviving projection-bearing plan both in
`_state->scan_plan` and in the scheduler. For S surviving groups and P
wide/deep projection nodes, that adds O(S x P) persistent planning state
(roughly twice), plus repeated request-copy CPU before any batch is read.
Please represent the row-group leaf/full decision as a compact delta or shared
immutable projection shape, pass request views to metadata helpers, and avoid
retaining two deep plan copies.
--
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]
