github-actions[bot] commented on code in PR #66310:
URL: https://github.com/apache/doris/pull/66310#discussion_r3685171032
##########
be/src/format_v2/table_reader.h:
##########
@@ -1339,32 +1346,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
Review Comment:
[P1] Avoid copying the shared Variant root before path extraction
When the outputs contain the full root followed by a Path Slot for the same
local position
(for example `v, v.metric.x`), `finalize_chunk()` materializes `v` first with
`take_projection_result=false`. `_detach_column()` then calls
`IColumn::mutate()` while the file
block still owns that `ColumnVariantV2`, so its metadata, metadata-id, and
value buffers are
recursively copied. This Path Slot subsequently takes the original carrier
only to read it.
The extra full-root copy is output-order-dependent and can restore the
multi-GB/OOM case that
the existing last-projection detach optimization was designed to avoid.
Please
materialize/extract all Path Slots that share a local position before
transferring its
full-root output, while preserving final output order, and add a
root-plus-path ownership 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]
