github-actions[bot] commented on code in PR #66310:
URL: https://github.com/apache/doris/pull/66310#discussion_r3685790223
##########
be/src/format_v2/table_reader.h:
##########
@@ -1339,32 +1401,107 @@ class TableReader {
return Status::OK();
}
if (mapping.default_expr != nullptr) {
- if (current_block->rows() == rows) {
- ColumnWithTypeAndName result;
- RETURN_IF_ERROR(_execute_default_expr_without_root_type_check(
- mapping.default_expr, current_block, &result));
- ColumnPtr result_column = result.column;
- RETURN_IF_ERROR(_align_column_nullability(&result_column,
mapping.table_type));
- *column = _detach_column(std::move(result_column));
- } else {
- DORIS_CHECK(mapping.constant_index.has_value());
- Block eval_block;
-
eval_block.insert({mapping.table_type->create_column_const_with_default_value(rows),
- mapping.table_type,
"__table_reader_const_rows"});
- ColumnWithTypeAndName result;
- RETURN_IF_ERROR(_execute_default_expr_without_root_type_check(
- mapping.default_expr, &eval_block, &result));
- ColumnPtr result_column = result.column;
- RETURN_IF_ERROR(_align_column_nullability(&result_column,
mapping.table_type));
- *column = _detach_column(std::move(result_column));
- }
- return Status::OK();
+ return _materialize_default_mapping_column(mapping, current_block,
rows, column);
}
ColumnPtr result_column =
mapping.table_type->create_column_const_with_default_value(rows);
*column = _detach_column(std::move(result_column));
return Status::OK();
}
+ Status _materialize_default_mapping_column(const ColumnMapping& mapping,
Block* current_block,
+ size_t rows, ColumnPtr* column)
{
+ DORIS_CHECK(mapping.default_expr != nullptr);
+ DORIS_CHECK(mapping.table_type != nullptr);
+ DORIS_CHECK(current_block != nullptr);
+ DORIS_CHECK(column != nullptr);
+ if (current_block->rows() == rows) {
+ ColumnWithTypeAndName result;
+
RETURN_IF_ERROR(_execute_default_expr_without_root_type_check(mapping.default_expr,
+
current_block, &result));
+ ColumnPtr result_column = result.column;
+ RETURN_IF_ERROR(_align_column_nullability(&result_column,
mapping.table_type));
+ *column = _detach_column(std::move(result_column));
+ return Status::OK();
+ }
+
+ DORIS_CHECK(mapping.constant_index.has_value());
+ Block eval_block;
+
eval_block.insert({mapping.table_type->create_column_const_with_default_value(rows),
+ mapping.table_type, "__table_reader_const_rows"});
+ ColumnWithTypeAndName result;
+
RETURN_IF_ERROR(_execute_default_expr_without_root_type_check(mapping.default_expr,
+
&eval_block, &result));
+ ColumnPtr result_column = result.column;
+ RETURN_IF_ERROR(_align_column_nullability(&result_column,
mapping.table_type));
+ *column = _detach_column(std::move(result_column));
+ return Status::OK();
+ }
+
+ Status _materialize_variant_path_mapping_column(const ColumnMapping&
mapping,
+ Block* current_block,
size_t rows,
+ ColumnPtr* column,
+ bool
take_projection_result) {
+ DORIS_CHECK(!mapping.column_paths.empty());
+ DORIS_CHECK(mapping.resolved_variant_path != nullptr);
+ DORIS_CHECK(mapping.table_type != nullptr);
+ DORIS_CHECK(current_block != nullptr);
+ DORIS_CHECK(column != nullptr);
+
+ ColumnPtr root_column;
+ if (mapping.projection != nullptr) {
+ int result_id;
+ auto status = mapping.projection->execute(current_block,
&result_id);
+ if (!status.ok()) {
+ return Status::InternalError(
+ "Failed to read Variant root carrier for Path Slot
'{}' "
+ "(global_index={}, rows={}): {}, mapping={}",
+ mapping.table_column_name,
mapping.global_index.value(), rows,
+ status.to_string(), mapping.debug_string());
+ }
+ root_column = take_projection_result
+ ?
_take_and_detach_block_column(current_block, result_id)
+ :
current_block->get_by_position(result_id).column;
+ } else if (mapping.default_expr != nullptr) {
+ // Schema-evolution defaults describe the missing root column.
Apply the same path
+ // extraction after evaluating that root value instead of exposing
the whole default.
+ RETURN_IF_ERROR(_materialize_default_mapping_column(mapping,
current_block, rows,
+ &root_column));
+ } else {
+ // A missing root without a default makes every requested subpath
SQL NULL.
+ *column = _detach_column(
+
mapping.table_type->create_column_const_with_default_value(rows));
+ return Status::OK();
+ }
+
+ const ColumnPtr materialized =
root_column->convert_to_full_column_if_const();
+ const IColumn* physical = materialized.get();
+ std::span<const uint8_t> outer_nulls;
+ if (const auto* nullable =
check_and_get_column<ColumnNullable>(physical)) {
+ outer_nulls = nullable->get_null_map_data();
+ physical = &nullable->get_nested_column();
+ }
+ const auto* variant = check_and_get_column<ColumnVariantV2>(physical);
+ if (variant == nullptr) {
+ return Status::NotSupported(
+ "Variant Path Slot '{}' requires a ColumnVariantV2 root
carrier, got {}. "
+ "Legacy ColumnVariant path extraction is not equivalent
because it also needs "
+ "sparse/document fallback semantics",
+ mapping.table_column_name, materialized->get_name());
+ }
+ if (variant->size() != rows) {
+ return Status::InternalError(
+ "Variant root carrier for Path Slot '{}' has {} rows,
expected {}",
+ mapping.table_column_name, variant->size(), rows);
+ }
+
+ ColumnPtr result;
+ RETURN_IF_ERROR(extract_variant_element_v2(*variant,
*mapping.resolved_variant_path,
Review Comment:
[P1] Size batches from the full fallback carrier
Current production readers ignore `variant_paths`, so a Path-only request
still materializes the complete Variant root here and then returns only the
much smaller extracted leaf. `FileScannerV2` starts with a 32-row probe but
updates `AdaptiveBlockSizePredictor` from the post-finalize table block; with a
1 MiB root and a scalar path, that probe reads about 32 MiB yet reports only
scalar bytes/row, allowing the next request to jump to the 4096-row cap and
approach 4 GiB. Wide missing-root/default carriers have the same blind spot.
This can OOM even though the requested Path Slot is narrow. Until readers
advertise real physical path pruning, please cap these scans conservatively or
feed the predictor the pre-extraction carrier bytes, and add scanner-level
coverage for large-root/tiny-path reads and defaults.
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -58,6 +60,67 @@ namespace doris::format {
namespace {
+std::string mapping_mode_to_string(TableColumnMappingMode mode);
+
+bool is_variant_v2_type(const DataTypePtr& type) {
+ return type != nullptr &&
+ dynamic_cast<const DataTypeVariantV2*>(remove_nullable(type).get())
!= nullptr;
+}
+
+Status resolve_variant_path(const std::vector<std::string>& column_paths,
+ std::shared_ptr<const
ResolvedVariantElementV2Path>* resolved_path) {
+ DORIS_CHECK(!column_paths.empty());
+ DORIS_CHECK(resolved_path != nullptr);
+ std::vector<VariantElementV2PathSegment> segments;
+ segments.reserve(column_paths.size());
+ for (const auto& key : column_paths) {
+ // TSlotDescriptor.column_paths is an already-tokenized list. Treat
every entry as one
+ // object key; joining on '.' would turn a literal key such as "a.b"
into two selectors.
+ // Array indexes need a typed transport and are intentionally outside
this contract.
+ segments.push_back(
+ VariantElementV2PathSegment::object_key(StringRef(key.data(),
key.size())));
+ }
+ std::unique_ptr<ResolvedVariantElementV2Path> candidate;
+ RETURN_IF_ERROR(resolve_variant_element_v2_path(segments, &candidate));
+ *resolved_path = std::move(candidate);
+ return Status::OK();
+}
+
+Status initialize_variant_path_mapping(const ColumnDefinition& table_column,
+ TableColumnMappingMode mode,
ColumnMapping* mapping) {
+ DORIS_CHECK(mapping != nullptr);
+ mapping->column_paths = table_column.column_paths;
+ if (mapping->column_paths.empty()) {
+ return Status::OK();
+ }
+ if (mode != TableColumnMappingMode::BY_FIELD_ID) {
Review Comment:
[P1] Preserve Path Slots on idless Iceberg files
`IcebergTableReader::mapping_mode()` deliberately selects `BY_NAME` for
legacy files without physical field IDs, but this returns `NotSupported` before
`_find_file_field()` and before the missing-root initial/default/NULL branches.
Once the future FE rewrite emits a Path Slot for a table that still contains an
older idless data file, `v['metric']['x']` therefore fails on that split even
if `v` is safely name-mappable, or did not exist yet and should produce its
Iceberg initial default/SQL NULL. Please retain a safe root-name/name-mapping
identity alongside the authoritative field ID and support the idless per-file
mode, with Iceberg reader tests for both an existing root and a missing root
with a default.
--
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]
