github-actions[bot] commented on code in PR #63001:
URL: https://github.com/apache/doris/pull/63001#discussion_r3275183781
##########
be/src/information_schema/schema_scanner.cpp:
##########
@@ -457,8 +473,8 @@ std::string SchemaScanner::get_db_from_full_name(const
std::string& full_name) {
Status SchemaScanner::insert_block_column(TCell cell, int col_index, Block*
block,
PrimitiveType type) {
MutableColumnPtr mutable_col_ptr;
- mutable_col_ptr =
std::move(*block->get_by_position(col_index).column).assume_mutable();
Review Comment:
`insert_block_column()` has the same live-slot move gap: unsupported `type`
returns an error before the column is restored, and the string/date/numeric
insertions can throw after the block slot has been consumed. That leaves the
caller's reusable output block with a null column on a normal non-OK return.
Please mutate this slot through a scoped guard or restore it on every exit
before returning an error.
##########
be/src/information_schema/schema_scanner.cpp:
##########
@@ -298,11 +314,10 @@ void SchemaScanner::_init_block(Block* src_block) {
Status SchemaScanner::fill_dest_column_for_range(Block* block, size_t pos,
const std::vector<void*>&
datas) {
const ColumnDesc& col_desc = _columns[pos];
- MutableColumnPtr column_ptr;
- column_ptr =
std::move(*block->get_by_position(pos).column).assume_mutable();
Review Comment:
This moves the live destination slot out of `block` before the row appends
and type dispatch are guaranteed to finish. The default branch below returns
`Status::InternalError` before the new `replace_by_position()`, and the
individual `insert_*` calls can also throw/OOM; either path leaves
`block->get_by_position(pos).column` moved-from. This is a distinct
information-schema fill path from the existing meta-scanner/schema-scan
ownership threads. Please use `block->mutate_column_scoped(pos)` or another
restore guard and release/commit only after all error-producing work succeeds.
##########
be/src/exec/operator/schema_scan_operator.cpp:
##########
@@ -256,10 +257,16 @@ Status SchemaScanOperatorX::get_block(RuntimeState*
state, Block* block, bool* e
if (src_block.rows()) {
// block->check_number_of_rows();
for (int i = 0; i < _slot_num; ++i) {
- MutableColumnPtr column_ptr =
std::move(*block->get_by_position(i).column).mutate();
- column_ptr->insert_range_from(
-
*src_block.safe_get_by_position(_slot_offsets[i]).column, 0,
- src_block.rows());
+ MutableColumnPtr column_ptr =
Review Comment:
This schema-scan source path still steals each live output column before
`make_nullable()`, the nullability check, and `insert_range_from()` are
guaranteed to complete. If any append/allocation throws, the pipeline output
block retains a moved-from slot because `replace_by_position()` is skipped.
This is a separate caller from the already-commented
`SchemaScanner::get_next_block`/meta-scanner paths; please use
`mutate_column_scoped(i)` or defer consuming the slot until the replacement
column is fully built.
##########
be/src/information_schema/schema_scanner_helper.cpp:
##########
@@ -31,29 +32,31 @@ namespace doris {
void SchemaScannerHelper::insert_string_value(int col_index, std::string_view
str_val,
Block* block) {
- MutableColumnPtr mutable_col_ptr;
- mutable_col_ptr =
block->get_by_position(col_index).column->assume_mutable();
+ MutableColumnPtr mutable_col_ptr =
+
IColumn::mutate(std::move(block->get_by_position(col_index).column));
auto* nullable_column =
assert_cast<ColumnNullable*>(mutable_col_ptr.get());
IColumn* col_ptr = &nullable_column->get_nested_column();
assert_cast<ColumnString*>(col_ptr)->insert_data(str_val.data(),
str_val.size());
nullable_column->push_false_to_nullmap(1);
+ block->replace_by_position(col_index, std::move(mutable_col_ptr));
}
void SchemaScannerHelper::insert_datetime_value(int col_index, const
std::vector<void*>& datas,
Block* block) {
- MutableColumnPtr mutable_col_ptr;
- mutable_col_ptr =
block->get_by_position(col_index).column->assume_mutable();
+ MutableColumnPtr mutable_col_ptr =
+
IColumn::mutate(std::move(block->get_by_position(col_index).column));
Review Comment:
All of these helper insertors now move the nullable column out of the
caller's live block and only put it back after appending nested data and the
null-map bit. Any allocation failure from `insert_data()`/`insert_value()` or
`push_false_to_nullmap()` leaves the block slot empty. Please use the new
single-column scoped mutation helper here so the original owner is restored
during unwinding.
--
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]
