Gabriel39 commented on code in PR #66206:
URL: https://github.com/apache/doris/pull/66206#discussion_r3680421431
##########
be/src/core/column/variant_v2/column_variant_v2.cpp:
##########
@@ -497,6 +554,9 @@ void ColumnVariantV2::for_each_subcolumn(ColumnCallback
callback) const {
}
void ColumnVariantV2::mutate_subcolumns() {
+ if (_shredded) {
+ ensure_encoded();
Review Comment:
Fixed. Output detachment now preserves the immutable shredded state instead
of forcing canonical encoding. The end-to-end leaf projection path remains
shredded until the Variant element consumer reads it.
##########
be/src/core/column/variant_v2/column_variant_v2.cpp:
##########
@@ -724,6 +792,20 @@ void ColumnVariantV2::insert_range_from( //
NOLINT(readability-function-size)
return;
}
+ if (!_shredded && !_typed && empty() && _metadatas->empty() &&
source._shredded && start == 0 &&
+ length == source.size()) {
+ _shredded = source._shredded;
+ _check_invariants();
+ return;
+ }
+ if (_shredded) {
+ ensure_encoded();
+ }
+ if (source._shredded) {
+ insert_range_from(source._shredded->materialized_column(), start,
length);
Review Comment:
Fixed. Shredded states now implement native range and index selection, so
proper subset cuts stay shredded. The widened-batch coverage exercises this
path.
##########
be/src/format_v2/parquet/native_schema_desc.cpp:
##########
@@ -55,6 +56,180 @@ static bool is_map_node(const tparquet::SchemaElement&
schema) {
(schema.__isset.logicalType && schema.logicalType.__isset.MAP);
}
+static bool is_variant_node(const tparquet::SchemaElement& schema) {
+ return schema.__isset.logicalType && schema.logicalType.__isset.VARIANT;
+}
+
+class ScopedBoolOverride {
+public:
+ ScopedBoolOverride(bool& target, bool value) : _target(target),
_original(target) {
+ _target = value;
+ }
+ ~ScopedBoolOverride() { _target = _original; }
+
+private:
+ bool& _target;
+ bool _original;
+};
+
+static Status validate_variant_layout(const tparquet::SchemaElement&
group_schema,
+ const NativeFieldSchema& group_field) {
+ const auto& annotation = group_schema.logicalType.VARIANT;
+ if (annotation.__isset.specification_version &&
annotation.specification_version != 1) {
+ return Status::NotSupported("Parquet Variant specification version {}
is not supported",
+ annotation.specification_version);
+ }
+ if (group_field.children.size() < 2 || group_field.children.size() > 3) {
+ return Status::Corruption(
+ "Parquet Variant {} must contain metadata, value, and optional
typed_value",
+ group_schema.name);
+ }
+
+ const NativeFieldSchema* metadata = nullptr;
+ const NativeFieldSchema* value = nullptr;
+ const NativeFieldSchema* typed_value = nullptr;
+ for (const auto& child : group_field.children) {
+ const NativeFieldSchema** target = nullptr;
+ if (child.name == "metadata") {
+ target = &metadata;
+ } else if (child.name == "value") {
+ target = &value;
+ } else if (child.name == "typed_value") {
+ target = &typed_value;
+ } else {
+ return Status::Corruption("Parquet Variant {} has unexpected child
{}",
+ group_schema.name, child.name);
+ }
+ if (*target != nullptr) {
+ return Status::Corruption("Parquet Variant {} has duplicate child
{}",
+ group_schema.name, child.name);
+ }
+ *target = &child;
+ }
+ if (metadata == nullptr || value == nullptr) {
+ return Status::Corruption("Parquet Variant {} requires metadata and
value children",
+ group_schema.name);
+ }
+ if (!metadata->children.empty() || metadata->physical_type !=
tparquet::Type::BYTE_ARRAY ||
+ metadata->parquet_schema.repetition_type !=
tparquet::FieldRepetitionType::REQUIRED) {
+ return Status::Corruption("Parquet Variant {} metadata must be a
required BYTE_ARRAY",
+ group_schema.name);
+ }
+ const auto expected_value_repetition = typed_value == nullptr
+ ?
tparquet::FieldRepetitionType::REQUIRED
+ :
tparquet::FieldRepetitionType::OPTIONAL;
+ // SQL nullability belongs to the outer Variant group. Only shredding
makes value optional,
+ // because typed_value may carry all or part of the logical value instead.
+ if (!value->children.empty() || value->physical_type !=
tparquet::Type::BYTE_ARRAY ||
+ value->parquet_schema.repetition_type != expected_value_repetition) {
+ return Status::Corruption("Parquet Variant {} value must be a {}
BYTE_ARRAY",
+ group_schema.name,
+ typed_value == nullptr ? "required" :
"optional");
+ }
+ if (typed_value != nullptr &&
+ typed_value->parquet_schema.repetition_type !=
tparquet::FieldRepetitionType::OPTIONAL) {
+ return Status::Corruption("Parquet Variant {} typed_value must be
optional",
+ group_schema.name);
+ }
+
+ std::function<Status(const NativeFieldSchema&)> validate_typed_value;
+ std::function<Status(const NativeFieldSchema&)> validate_wrapper;
+ validate_wrapper = [&](const NativeFieldSchema& wrapper) -> Status {
+ if (!wrapper.parquet_schema.__isset.repetition_type ||
+ wrapper.parquet_schema.repetition_type !=
tparquet::FieldRepetitionType::REQUIRED) {
+ return Status::Corruption("Parquet Variant shredded wrapper {}
must be required",
+ wrapper.name);
+ }
+ const NativeFieldSchema* fallback = nullptr;
+ const NativeFieldSchema* typed = nullptr;
+ for (const auto& child : wrapper.children) {
+ if (child.name == "value") {
+ fallback = &child;
+ } else if (child.name == "typed_value") {
+ typed = &child;
+ } else {
+ return Status::Corruption("Parquet Variant wrapper {} has
unexpected child {}",
+ wrapper.name, child.name);
+ }
+ }
+ if (fallback == nullptr || typed == nullptr ||
!fallback->children.empty() ||
Review Comment:
Fixed. Schema validation now accepts the context-valid omitted
value/typed_value children, validates every present child, and recurses only
through present typed values. Coverage includes value-only object fields and
typed-only/value-only array elements.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -1200,6 +1200,11 @@ private List<Split> doGetSplits(int numBackends) throws
UserException {
return doGetSystemTableSplits();
}
+ boolean projectsVariant = !isTableLevelCountStarPushdown()
+ && desc.getSlots().stream()
+ .anyMatch(slot ->
IcebergUtils.containsVariant(slot.getColumn().getType()));
Review Comment:
Fixed. The smooth-upgrade capability gate now uses each materialized slot
effective type, so a non-Variant sibling projection is allowed while semantic
Variant projections remain gated. Added FE coverage.
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -627,8 +672,15 @@ Status FileScannerV2::_create_table_reader_for_format(
Status FileScannerV2::_prepare_table_reader_split(const TFileRangeDesc& range,
std::map<std::string, Field>
partition_values) {
+ const auto format_type = get_range_format_type(*_params, range);
+ const bool projects_variant =
+ !_local_state->is_count_star_pushdown() &&
+ std::ranges::any_of(_projected_columns, [](const
format::ColumnDefinition& column) {
+ return contains_variant_type(column.type);
+ });
+ RETURN_IF_ERROR(validate_iceberg_variant_file_format(range, format_type,
projects_variant));
Review Comment:
Fixed. Unsupported-format rejection now occurs only after per-file mapping
proves that the surviving split physically supplies a requested Variant.
Evolved ORC files synthesize NULL, and mixed ORC/Parquet scans read the Parquet
Variant. The regression covers both cases.
##########
be/src/core/column/variant_v2/column_variant_v2.cpp:
##########
@@ -724,6 +794,37 @@ void ColumnVariantV2::insert_range_from( //
NOLINT(readability-function-size)
return;
}
+ if (!_shredded && !_typed && empty() && _metadatas->empty() &&
source._shredded) {
+ // IColumn::cut() inserts into an empty clone. Select the physical
tree directly because an
+ // incomplete leaf projection cannot be reconstructed merely to copy a
row range.
+ _shredded = start == 0 && length == source.size()
+ ? source._shredded
+ : source._shredded->select_range(start, length);
+ _check_invariants();
+ return;
+ }
+ if (_shredded && source._shredded) {
+ auto selected_source = start == 0 && length == source.size()
+ ? source._shredded
+ : source._shredded->select_range(start,
length);
+ if (!_shredded.unique()) {
Review Comment:
Fixed. Replaced shared_ptr::unique() with the repository-compatible
use_count() ownership check while preserving the copy-on-write invariant. The
rebased BE build passes locally; macOS CI has been re-triggered.
##########
be/src/core/column/variant_v2/column_variant_v2.cpp:
##########
@@ -724,6 +794,37 @@ void ColumnVariantV2::insert_range_from( //
NOLINT(readability-function-size)
return;
}
+ if (!_shredded && !_typed && empty() && _metadatas->empty() &&
source._shredded) {
+ // IColumn::cut() inserts into an empty clone. Select the physical
tree directly because an
+ // incomplete leaf projection cannot be reconstructed merely to copy a
row range.
+ _shredded = start == 0 && length == source.size()
+ ? source._shredded
+ : source._shredded->select_range(start, length);
+ _check_invariants();
+ return;
+ }
+ if (_shredded && source._shredded) {
+ auto selected_source = start == 0 && length == source.size()
+ ? source._shredded
+ : source._shredded->select_range(start,
length);
+ if (!_shredded.unique()) {
+ _shredded = _shredded->select_range(0, size());
+ }
+ // A partial physical projection has no metadata/value pair to encode.
Preserve that
+ // invariant by merging compatible scanner batches before the
canonical fallback below.
+ if (_shredded->try_append(*selected_source)) {
Review Comment:
Fixed. FileScannerV2 now projects each physical block before reading and
padding another split whenever the output contains Variant, so complete,
partial, and file-local shredding states are never merged raw. Added a two-file
complete-plus-leaf regression; test_iceberg_variant_read passes locally on
freshly rebuilt binaries (1 suite, 0 failures).
--
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]
