github-actions[bot] commented on code in PR #66941:
URL: https://github.com/apache/doris/pull/66941#discussion_r3820916467
##########
be/src/format_v2/parquet/parquet_reader.cpp:
##########
@@ -141,26 +141,124 @@ bool variant_leaf_projection_is_safe_for_row_group_impl(
} // namespace
-bool detail::variant_leaf_projection_is_safe_for_row_group(
+bool detail::variant_residual_columns_are_prunable_for_row_group(
const tparquet::RowGroup& row_group, const ParquetColumnSchema& schema,
const format::LocalColumnIndex& projection) {
- return variant_leaf_projection_is_safe_for_row_group_impl(row_group,
schema, projection);
+ return variant_residual_columns_are_prunable_impl(row_group, schema,
projection);
+}
+
+namespace {
+
+void remove_child_projection(format::LocalColumnIndex* projection, int32_t
local_id) {
+ std::erase_if(projection->children, [local_id](const
format::LocalColumnIndex& child) {
+ return child.local_id() == local_id;
+ });
+}
+
+void prune_variant_residual_columns_impl(const ParquetColumnSchema& schema,
+ format::LocalColumnIndex* projection)
{
+ 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 &&
+ typed_value->kind == ParquetColumnSchemaKind::PRIMITIVE) {
+ // Terminal wrapper: statistics proved its residual is entirely NULL
for this row group,
+ // so the shredded leaf alone answers every row.
+ remove_child_projection(projection, value->local_id);
+ return;
+ }
+ if (schema.kind == ParquetColumnSchemaKind::VARIANT) {
+ // The root dictionary is only needed to decode a residual.
+ if (const auto* metadata = schema_child_by_name(schema, "metadata");
metadata != nullptr) {
+ remove_child_projection(projection, metadata->local_id);
+ }
+ }
+ for (auto& child_projection : projection->children) {
+ const auto* child = projected_schema_child(schema,
child_projection.local_id());
+ if (child != nullptr) {
+ prune_variant_residual_columns_impl(*child, &child_projection);
+ }
+ }
+}
+
+} // namespace
+
+namespace {
+
+// A leaf projection can only answer rows whose value sits outside the
shredded leaf if it actually
+// reads the residual beside that leaf, and the root dictionary needed to
decode it. A projection
+// that omits either cannot serve those rows, and nothing else would notice:
the leaf would report
+// them as absent.
+bool variant_projection_carries_residuals_impl(const ParquetColumnSchema&
schema,
+ const format::LocalColumnIndex&
projection) {
+ const auto* value = schema_child_by_name(schema, "value");
+ const auto* typed_value = schema_child_by_name(schema, "typed_value");
+ const auto projects = [&projection](int32_t local_id) {
+ return std::ranges::any_of(projection.children, [local_id](const auto&
child) {
+ return child.local_id() == local_id;
+ });
+ };
+ if (value != nullptr && typed_value != nullptr) {
+ if (typed_value->kind == ParquetColumnSchemaKind::PRIMITIVE) {
+ return projects(value->local_id);
+ }
+ if (schema.kind == ParquetColumnSchemaKind::VARIANT) {
+ const auto* metadata = schema_child_by_name(schema, "metadata");
+ if (metadata == nullptr || !projects(metadata->local_id)) {
+ return false;
+ }
+ }
+ }
+ for (const auto& child_projection : projection.children) {
+ const auto* child = projected_schema_child(schema,
child_projection.local_id());
+ if (child != nullptr &&
+ !variant_projection_carries_residuals_impl(*child,
child_projection)) {
+ return false;
+ }
+ }
+ return true;
Review Comment:
[P1] Retain ancestor carriers needed for structure validation
This returns true for a deep residual-carrying projection even though the
mapper includes `value` only beside the terminal primitive, not beside the root
or intermediate object `typed_value`s. On the newly retained path (a row group
with any terminal fallback), one of those wrappers can therefore contain a
STRUCT `typed_value` plus a non-object residual: the partial request drops that
invalid carrier and the one-segment element chain serializes the already-pruned
prefix, while canonical materialization throws `object typed_value has
non-object residual value`. Please project and validate ancestor residual
carriers, or keep the complete fallback unless their null statistics make
omission safe, and add an end-to-end deep malformed-carrier test.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -479,54 +481,35 @@ std::unordered_set<int> request_leaf_column_ids(
return leaf_column_ids;
}
-void collect_full_variant_leaf_delta(const ParquetColumnSchema& schema,
- const format::LocalColumnIndex&
projection,
- std::span<const size_t> full_ordinals,
- size_t* candidate_ordinal,
- std::unordered_set<int>* leaf_column_ids)
{
- DORIS_CHECK(candidate_ordinal != nullptr && leaf_column_ids != nullptr);
- if (!format::is_partial_projection(&projection)) {
- return;
- }
- if (schema.kind == ParquetColumnSchemaKind::VARIANT) {
- if (std::ranges::binary_search(full_ordinals, *candidate_ordinal)) {
- collect_all_leaf_column_ids(schema, leaf_column_ids);
- }
- ++*candidate_ordinal;
- return;
- }
- for (const auto& child_projection : projection.children) {
- const auto* child_schema = projection_schema_child(schema,
child_projection.local_id());
- DORIS_CHECK(child_schema != nullptr);
- collect_full_variant_leaf_delta(*child_schema, child_projection,
full_ordinals,
- candidate_ordinal, leaf_column_ids);
- }
+void materialize_row_group_projection(
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request, const RowGroupReadPlan&
row_group_plan,
+ format::FileScanRequest* physical_request);
+
+std::unordered_set<int> request_leaf_column_ids(
+ const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
+ const format::FileScanRequest& request) {
+#ifdef BE_TEST
+ detail::physical_leaf_set_builds.fetch_add(1, std::memory_order_relaxed);
+#endif
+ return collect_request_leaf_column_ids(file_schema, request);
}
std::optional<std::unordered_set<int>> row_group_leaf_column_ids(
const std::vector<std::unique_ptr<ParquetColumnSchema>>& file_schema,
const format::FileScanRequest& request, const RowGroupReadPlan&
row_group_plan,
const std::unordered_set<int>& request_leaf_ids) {
- if (row_group_plan.full_variant_projection_ordinals.empty()) {
+ if (!row_group_plan.has_row_group_physical_projection()) {
return std::nullopt;
}
- auto leaf_column_ids = request_leaf_ids;
- size_t candidate_ordinal = 0;
- const auto collect_projection = [&](const format::LocalColumnIndex&
projection) {
- 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] == nullptr ||
!file_schema[local_id]->contains_variant) {
- return;
- }
- collect_full_variant_leaf_delta(*file_schema[local_id], projection,
-
row_group_plan.full_variant_projection_ordinals,
- &candidate_ordinal, &leaf_column_ids);
- };
- for (const auto& projection : request.predicate_columns) {
- collect_projection(projection);
- }
- for (const auto& projection : request.non_predicate_columns) {
- collect_projection(projection);
+ // Pruning removes columns, so this set cannot be derived by adding to the
request-level set:
+ // the same Variant can appear pruned under a predicate projection and
unpruned under the
+ // deferred output projection at once. Rebuild it from the rewritten
projections instead.
+ format::FileScanRequest physical_request;
Review Comment:
[P2] Reuse the request leaf set for row-group deltas
Any row group whose Variant residual can be pruned now copies both complete
projection vectors and walks every projected column to rebuild this set. That
happens for every candidate during footer pruning, again for deferred
dictionary/Bloom/page-index planning, and again after runtime-filter
replanning. Thus the common fully-shredded case adds `O(row_groups *
projected_columns)` eager allocation/hash/traversal work to wide scans,
although each row-group decision changes only a few Variant leaves. Please
derive a small delta from `request_leaf_ids` or cache sets by decision vectors;
the current test counter does not include these
`collect_request_leaf_column_ids()` calls.
##########
be/src/format_v2/parquet/reader/variant_column_reader.cpp:
##########
@@ -739,6 +747,226 @@ ColumnPtr normalize_materialized_path(const
ColumnVariantV2& materialized,
return ColumnNullable::create(std::move(values), std::move(nulls));
}
+// Column-level access to one optional Parquet column. Caching the null map
beside the values lets
+// the row loops resolve a path without repeating the ColumnNullable cast for
every row.
+struct NullableColumnView {
+ const IColumn* values = nullptr;
+ const uint8_t* nulls = nullptr;
+
+ bool present() const { return values != nullptr; }
+ bool is_null(size_t row) const {
+ return values == nullptr || (nulls != nullptr && nulls[row] != 0);
+ }
+};
+
+NullableColumnView view_column(const ColumnPtr& column) {
+ if (!column) {
+ return {};
+ }
+ if (const auto* nullable = check_and_get_column<ColumnNullable>(*column)) {
+ return {.values = &nullable->get_nested_column(),
+ .nulls = nullable->get_null_map_data().data()};
+ }
+ return {.values = column.get(), .nulls = nullptr};
+}
+
+// One `value`/`typed_value` pair along a shredded object path. Level 0 is the
Variant root and
+// level i + 1 belongs to path[i], so the residual at level i encodes the
value of that path prefix
+// for every row whose typed value beside it is null.
+struct ShreddedPathLevel {
+ ColumnPtr residual;
+ ColumnPtr typed;
+ NullableColumnView residual_view;
+ NullableColumnView typed_view;
+};
+
+// A residual can only supply the requested path for rows whose typed value is
null. Shredding keeps
+// an object's residual keys disjoint from its shredded fields, so a present
typed value always owns
+// the path and the residual beside it holds unrelated keys.
+bool residual_supplies_any_row(const ShreddedPathLevel& level, size_t rows) {
+ if (!level.residual_view.present()) {
+ return false;
+ }
+ for (size_t row = 0; row < rows; ++row) {
+ if (level.typed_view.is_null(row) &&
!level.residual_view.is_null(row)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+// Identifies the metadata dictionary of every row once per decoded batch.
Iceberg normally repeats
+// a single dictionary across a batch, so the list stays at one entry. Sharing
this across the paths
+// of one batch is what keeps a wide projection from re-scanning the
dictionary column per path.
+struct VariantMetadataIndex {
+ static constexpr uint32_t MISSING_DICTIONARY =
std::numeric_limits<uint32_t>::max();
+
+ DorisVector<StringRef> dictionaries;
+ DorisVector<uint32_t> row_dictionaries;
+};
+
+std::shared_ptr<const VariantMetadataIndex> build_variant_metadata_index(
+ const NullableColumnView& metadata, size_t rows) {
+ auto index = std::make_shared<VariantMetadataIndex>();
+ index->row_dictionaries.assign(rows,
VariantMetadataIndex::MISSING_DICTIONARY);
+ for (size_t row = 0; row < rows; ++row) {
+ if (metadata.is_null(row)) {
+ continue;
+ }
+ const StringRef bytes = metadata.values->get_data_at(row);
+ uint32_t dictionary = 0;
+ while (dictionary < index->dictionaries.size() &&
+ index->dictionaries[dictionary] != bytes) {
+ ++dictionary;
+ }
+ if (dictionary == index->dictionaries.size()) {
+ index->dictionaries.push_back(bytes);
+ }
+ index->row_dictionaries[row] = dictionary;
+ }
+ return index;
+}
+
+// Seeks object paths inside encoded residual bytes. Keys resolve to a
dictionary field id once per
+// distinct dictionary instead of once per row, and the dictionary index is
built only when a path
+// still has keys to resolve - a residual that already is the requested value
needs no dictionary.
+class VariantResidualSeeker {
+public:
+ using MetadataIndexProvider = std::function<const VariantMetadataIndex&()>;
+
+ VariantResidualSeeker(const NullableColumnView& metadata, size_t
path_length,
+ MetadataIndexProvider provider)
+ : _metadata(metadata), _path_length(path_length),
_provider(std::move(provider)) {}
+
+ // Resolves path[path_offset..] inside `value`. The offset keeps the cache
keyed by absolute
+ // path position, because different rows can enter the residual at
different depths.
+ bool seek(size_t row, StringRef value, size_t path_offset,
+ std::span<const VariantShreddedPathSegment> path, VariantRef*
output) {
+ DORIS_CHECK(output != nullptr);
+ if (path.empty()) {
+ // The residual already is the requested value; only its
dictionary travels with it.
+ if (_metadata.is_null(row)) {
+ return false;
+ }
+ const StringRef bytes = _metadata.values->get_data_at(row);
+ *output = VariantRef {.metadata = {.data = bytes.data, .size =
bytes.size},
+ .value = value};
+ return true;
+ }
+
+ const VariantMetadataIndex& index = _index == nullptr ?
_resolve_index() : *_index;
+ const uint32_t dictionary = index.row_dictionaries[row];
+ if (dictionary == VariantMetadataIndex::MISSING_DICTIONARY) {
Review Comment:
[P1] Preserve corruption errors on residual path misses
This turns a missing dictionary into an ordinary path miss, so a non-null
Variant row with null root metadata is emitted as SQL `NULL`. The same bypass
applies to malformed `sorted_strings` metadata: `find_key()` can miss before
`VariantMetadataRef::validate()` runs, and the miss branch validates only the
residual object's container. `encode_variant_range()` currently rejects null
metadata and validates every dictionary, so `v['key']` now changes
malformed-file behavior from an explicit corruption error to a plausible
result. Please validate each distinct dictionary (and reject missing metadata
for non-outer-null rows) before returning a miss, with differential
malformed-metadata tests.
##########
be/src/format_v2/parquet/reader/variant_column_reader.cpp:
##########
@@ -879,58 +1153,88 @@ class ParquetVariantShreddedState final : public
VariantShreddedState {
const ParquetColumnSchema* wrapper_schema = nullptr;
ColumnPtr wrapper = struct_child(*typed_schema, typed, key,
&wrapper_schema);
if (!wrapper) {
- return path_miss();
+ return seek_residual();
+ }
+ ColumnPtr residual = struct_child(*wrapper_schema, wrapper,
"value", nullptr);
+ typed = struct_child(*wrapper_schema, wrapper, "typed_value",
&typed_schema);
+ if (!typed) {
+ // The wrapper stores this field unshredded. Its residual
holds the whole value, so
+ // the remainder of the path resolves inside those bytes.
+ add_level(std::move(residual), nullptr);
+ return seek_residual();
+ }
+ add_level(std::move(residual), typed);
+
+ if (position + 1 < path.size()) {
+ // Every intermediate typed_value must be an object. A legacy
untyped path
+ // produced through an array/explode operation cannot cross a
repeated node, so
+ // those rows resolve from the residual beside it instead.
+ if (typed_schema->kind != ParquetColumnSchemaKind::STRUCT) {
+ levels.pop_back();
+ return seek_residual();
+ }
+ continue;
}
- if (ColumnPtr residual = struct_child(*wrapper_schema, wrapper,
"value", nullptr);
- static_cast<bool>(residual) && has_present_value(residual)) {
- // A residual value can contribute data to the same logical
object. Reconstructing
- // is required in that case; returning only the typed leaf
would drop information.
-
update_counter(_profile.variant_direct_leaf_residual_fallbacks, 1);
+ if (typed_schema->kind != ParquetColumnSchemaKind::PRIMITIVE ||
+ check_and_get_column<ColumnNullable>(*typed) == nullptr) {
+
update_counter(_profile.variant_direct_leaf_unsupported_fallbacks, 1);
+ if (!_complete) {
+ // Binary element_at evaluates complex prefixes before the
validated leaf.
+ // Serialize only retained descendants so projected-out
fields stay hidden.
+ if (auto normalized = find_normalized_value(path);
normalized.has_value()) {
+ return VariantShreddedTypedValue {
+ .column = nullptr, .type = nullptr,
.normalized = *normalized};
+ }
+ }
return std::nullopt;
}
- typed = struct_child(*wrapper_schema, wrapper, "typed_value",
&typed_schema);
- if (!typed) {
- return path_miss();
- }
- if (position + 1 == path.size()) {
- if (typed_schema->kind != ParquetColumnSchemaKind::PRIMITIVE ||
- check_and_get_column<ColumnNullable>(*typed) == nullptr) {
-
update_counter(_profile.variant_direct_leaf_unsupported_fallbacks, 1);
+ const bool needs_row_merge =
Review Comment:
[P1] Reject conflicting primitive carriers before direct handoff
`residual_supplies_any_row()` deliberately ignores `value` when
`typed_value` is present, so a terminal primitive wrapper with both fields
non-null leaves `needs_row_merge == false` and is returned from the typed leaf.
The canonical path rejects exactly this row in `append_typed_value()` (`scalar
typed_value cannot have residual value bytes`), as required by the shredding
rules. A normal one-segment `v['a']` access therefore silently accepts a
malformed row that full materialization rejects. Please validate the terminal
carrier pair before selecting the direct path and add a direct-vs-canonical
corruption test.
##########
be/src/format_v2/parquet/reader/variant_column_reader.cpp:
##########
@@ -739,6 +747,226 @@ ColumnPtr normalize_materialized_path(const
ColumnVariantV2& materialized,
return ColumnNullable::create(std::move(values), std::move(nulls));
}
+// Column-level access to one optional Parquet column. Caching the null map
beside the values lets
+// the row loops resolve a path without repeating the ColumnNullable cast for
every row.
+struct NullableColumnView {
+ const IColumn* values = nullptr;
+ const uint8_t* nulls = nullptr;
+
+ bool present() const { return values != nullptr; }
+ bool is_null(size_t row) const {
+ return values == nullptr || (nulls != nullptr && nulls[row] != 0);
+ }
+};
+
+NullableColumnView view_column(const ColumnPtr& column) {
+ if (!column) {
+ return {};
+ }
+ if (const auto* nullable = check_and_get_column<ColumnNullable>(*column)) {
+ return {.values = &nullable->get_nested_column(),
+ .nulls = nullable->get_null_map_data().data()};
+ }
+ return {.values = column.get(), .nulls = nullptr};
+}
+
+// One `value`/`typed_value` pair along a shredded object path. Level 0 is the
Variant root and
+// level i + 1 belongs to path[i], so the residual at level i encodes the
value of that path prefix
+// for every row whose typed value beside it is null.
+struct ShreddedPathLevel {
+ ColumnPtr residual;
+ ColumnPtr typed;
+ NullableColumnView residual_view;
+ NullableColumnView typed_view;
+};
+
+// A residual can only supply the requested path for rows whose typed value is
null. Shredding keeps
+// an object's residual keys disjoint from its shredded fields, so a present
typed value always owns
+// the path and the residual beside it holds unrelated keys.
+bool residual_supplies_any_row(const ShreddedPathLevel& level, size_t rows) {
+ if (!level.residual_view.present()) {
+ return false;
+ }
+ for (size_t row = 0; row < rows; ++row) {
+ if (level.typed_view.is_null(row) &&
!level.residual_view.is_null(row)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+// Identifies the metadata dictionary of every row once per decoded batch.
Iceberg normally repeats
+// a single dictionary across a batch, so the list stays at one entry. Sharing
this across the paths
+// of one batch is what keeps a wide projection from re-scanning the
dictionary column per path.
+struct VariantMetadataIndex {
+ static constexpr uint32_t MISSING_DICTIONARY =
std::numeric_limits<uint32_t>::max();
+
+ DorisVector<StringRef> dictionaries;
+ DorisVector<uint32_t> row_dictionaries;
+};
+
+std::shared_ptr<const VariantMetadataIndex> build_variant_metadata_index(
+ const NullableColumnView& metadata, size_t rows) {
+ auto index = std::make_shared<VariantMetadataIndex>();
+ index->row_dictionaries.assign(rows,
VariantMetadataIndex::MISSING_DICTIONARY);
+ for (size_t row = 0; row < rows; ++row) {
+ if (metadata.is_null(row)) {
+ continue;
+ }
+ const StringRef bytes = metadata.values->get_data_at(row);
+ uint32_t dictionary = 0;
+ while (dictionary < index->dictionaries.size() &&
Review Comment:
[P1] Avoid quadratic metadata dictionary indexing
Every row linearly compares its metadata bytes with every dictionary already
seen. Variant metadata is per row and valid files can use a distinct dictionary
for each row (the added two-dictionary test already relies on heterogeneity),
so a 65,535-row batch can perform about two billion byte comparisons before
seeking one key. This makes the new residual fast path pathological on valid
high-cardinality data. Please use a content-hashed dictionary index, validate
each newly inserted dictionary once, and cover a production-sized
high-cardinality batch.
##########
be/src/format_v2/parquet/reader/variant_column_reader.cpp:
##########
@@ -668,6 +669,13 @@ bool supports_direct_typed_variant_state(const
ParquetColumnSchema& schema) {
case TYPE_DECIMAL128I:
case TYPE_DATEV2:
return true;
+ case TYPE_STRING:
+ case TYPE_CHAR:
+ case TYPE_VARCHAR:
+ // BYTE_ARRAY carries strings, raw binary and UUID alike; only the
UTF-8 annotation makes
+ // the value a Variant string. Temporal identities stay excluded
because the typed state
+ // cannot record a timestamp's unit or its UTC adjustment.
+ return schema.type_descriptor.is_string_annotation &&
!schema.type_descriptor.is_uuid;
Review Comment:
[P1] Validate annotated STRING bytes before publishing typed state
This newly treats Parquet STRING leaves as safe for direct typed handoff,
but that bypasses the UTF-8 check performed by the canonical builder through
`VariantScalarRef::string()`. `ColumnVariantV2::create_typed()` checks only the
column/type shape, and the typed STRING cast returns the physical bytes
directly. A shredded STRING containing `0xff` can therefore be returned by
`CAST(v['a'] AS STRING)` while full Variant materialization rejects the same
row. Please validate non-null payloads before publishing this state (or keep
STRING on the normalized path) and add a malformed-UTF-8 differential test.
##########
be/src/format_v2/parquet/reader/variant_column_reader.cpp:
##########
@@ -879,58 +1153,88 @@ class ParquetVariantShreddedState final : public
VariantShreddedState {
const ParquetColumnSchema* wrapper_schema = nullptr;
ColumnPtr wrapper = struct_child(*typed_schema, typed, key,
&wrapper_schema);
if (!wrapper) {
- return path_miss();
+ return seek_residual();
+ }
+ ColumnPtr residual = struct_child(*wrapper_schema, wrapper,
"value", nullptr);
+ typed = struct_child(*wrapper_schema, wrapper, "typed_value",
&typed_schema);
+ if (!typed) {
+ // The wrapper stores this field unshredded. Its residual
holds the whole value, so
+ // the remainder of the path resolves inside those bytes.
+ add_level(std::move(residual), nullptr);
+ return seek_residual();
+ }
+ add_level(std::move(residual), typed);
+
+ if (position + 1 < path.size()) {
+ // Every intermediate typed_value must be an object. A legacy
untyped path
+ // produced through an array/explode operation cannot cross a
repeated node, so
+ // those rows resolve from the residual beside it instead.
+ if (typed_schema->kind != ParquetColumnSchemaKind::STRUCT) {
+ levels.pop_back();
+ return seek_residual();
+ }
+ continue;
}
- if (ColumnPtr residual = struct_child(*wrapper_schema, wrapper,
"value", nullptr);
- static_cast<bool>(residual) && has_present_value(residual)) {
- // A residual value can contribute data to the same logical
object. Reconstructing
- // is required in that case; returning only the typed leaf
would drop information.
-
update_counter(_profile.variant_direct_leaf_residual_fallbacks, 1);
+ if (typed_schema->kind != ParquetColumnSchemaKind::PRIMITIVE ||
+ check_and_get_column<ColumnNullable>(*typed) == nullptr) {
+
update_counter(_profile.variant_direct_leaf_unsupported_fallbacks, 1);
+ if (!_complete) {
+ // Binary element_at evaluates complex prefixes before the
validated leaf.
+ // Serialize only retained descendants so projected-out
fields stay hidden.
+ if (auto normalized = find_normalized_value(path);
normalized.has_value()) {
+ return VariantShreddedTypedValue {
+ .column = nullptr, .type = nullptr,
.normalized = *normalized};
+ }
+ }
return std::nullopt;
}
- typed = struct_child(*wrapper_schema, wrapper, "typed_value",
&typed_schema);
- if (!typed) {
- return path_miss();
- }
- if (position + 1 == path.size()) {
- if (typed_schema->kind != ParquetColumnSchemaKind::PRIMITIVE ||
- check_and_get_column<ColumnNullable>(*typed) == nullptr) {
-
update_counter(_profile.variant_direct_leaf_unsupported_fallbacks, 1);
+ const bool needs_row_merge =
+ std::ranges::any_of(levels, [rows](const
ShreddedPathLevel& level) {
+ return residual_supplies_any_row(level, rows);
+ });
+ if (needs_row_merge) {
+ // Decoding a residual needs the root dictionary. A state that
projected the
+ // metadata away cannot serve those rows, so it keeps the
complete fallback.
+ if (!metadata.present()) {
+
update_counter(_profile.variant_direct_leaf_residual_fallbacks, 1);
if (!_complete) {
- // Binary element_at evaluates complex prefixes before
the validated leaf.
- // Serialize only retained descendants so
projected-out fields stay hidden.
+ // A projected state cannot rebuild its root, so
serialize the retained
+ // descendants instead of demanding the complete
Variant.
if (auto normalized = find_normalized_value(path);
normalized.has_value()) {
return VariantShreddedTypedValue {
.column = nullptr, .type = nullptr,
.normalized = *normalized};
}
}
return std::nullopt;
}
- if (!supports_direct_typed_variant_state(*typed_schema)) {
- if (_complete) {
-
update_counter(_profile.variant_direct_leaf_unsupported_fallbacks, 1);
- return std::nullopt;
- }
- // A partial projection cannot reconstruct its root.
Normalize only the exact
- // requested leaf so Parquet annotations survive
heterogeneous file schemas.
- update_counter(_profile.variant_direct_leaf_rows,
- static_cast<int64_t>(typed->size()));
- return VariantShreddedTypedValue {
- .column = nullptr,
- .type = nullptr,
- .normalized =
normalize_projected_primitive_leaf(*typed_schema, typed)};
- }
- update_counter(_profile.variant_direct_leaf_rows,
- static_cast<int64_t>(typed->size()));
- return VariantShreddedTypedValue {.column = std::move(typed),
- .type =
remove_nullable(typed_schema->type),
- .normalized = nullptr};
+ update_counter(_profile.variant_direct_leaf_rows,
static_cast<int64_t>(rows));
+
update_counter(_profile.variant_direct_leaf_residual_merged_rows,
+ static_cast<int64_t>(rows));
+ return VariantShreddedTypedValue {.column = nullptr,
Review Comment:
[P2] Preserve normalized matches across composite states
This exact per-path normalized result is lost when incompatible pruned and
residual-retained states are combined by the ColumnVariantV2
insertion/selection or merging-exchange paths. The resulting
`CompositeVariantShreddedState` collects the direct and normalized matches,
treats them as non-homogeneous, discards both, and calls each segment's
`find_normalized_value()` again. The residual-bearing segment then
serializes/materializes the retained root before extracting this same path, so
residual rows are resolved twice and wide roots are encoded despite this
optimization. Please concatenate the already-collected per-segment matches
(normalizing only typed matches as needed) and add a mixed pruned/residual
composite test.
--
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]
